)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ba2c1d19c838b988d93e695f837b108741fcc8e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027cpuset_reserved\u0027 argument for the \u0027_numa_fit_instance_cell\u0027"},{"line_number":10,"context_line":"function is only ever non-zero if the \u0027isolate\u0027 CPU thread policy is in"},{"line_number":11,"context_line":"effect, and this thread policy can only be used if using the \u0027dedicated\u0027"},{"line_number":12,"context_line":"CPU policy is used. As such, we don\u0027t need to worry about overhead if"},{"line_number":13,"context_line":"either the \u0027share\u0027 CPU thread policy or the \u0027shared\u0027 CPU policy is in"},{"line_number":14,"context_line":"effect."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ff570b3c_5ef59e82","line":12,"range":{"start_line":11,"start_character":35,"end_line":12,"end_character":18},"updated":"2020-05-14 14:05:16.000000000","message":"I feel used.\n\nHow about just \"only be used when the CPU policy is dedicated\"?","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cede4195189dbbf4c86e15a439bcd7541b61f232","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The \u0027cpuset_reserved\u0027 argument for the \u0027_numa_fit_instance_cell\u0027"},{"line_number":10,"context_line":"function is only ever non-zero if the \u0027isolate\u0027 CPU thread policy is in"},{"line_number":11,"context_line":"effect, and this thread policy can only be used if using the \u0027dedicated\u0027"},{"line_number":12,"context_line":"CPU policy is used. As such, we don\u0027t need to worry about overhead if"},{"line_number":13,"context_line":"either the \u0027share\u0027 CPU thread policy or the \u0027shared\u0027 CPU policy is in"},{"line_number":14,"context_line":"effect."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"bf51134e_adc5ae5d","line":12,"range":{"start_line":11,"start_character":48,"end_line":12,"end_character":18},"updated":"2020-06-19 10:25:47.000000000","message":"if using the \u0027dedicated\u0027 CPU policy\nOR\nif the \u0027dedicated\u0027 CPU policy is used","commit_id":"c059f9a6bb5d2d45f419de05598c2ed8cdb980fd"}],"nova/virt/hardware.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ba2c1d19c838b988d93e695f837b108741fcc8e","unresolved":false,"context_lines":[{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"    # NOTE(stephenfin): As with memory, do not allow an instance to overcommit"},{"line_number":1126,"context_line":"    # against itself on any NUMA cell"},{"line_number":1127,"context_line":"    if instance_cell.cpu_policy \u003d\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1128,"context_line":"        required_cpus \u003d len(instance_cell.cpuset) + cpuset_reserved"},{"line_number":1129,"context_line":"        if required_cpus \u003e len(host_cell.pcpuset):"},{"line_number":1130,"context_line":"            LOG.debug(\u0027Not enough host cell CPUs to fit instance cell; \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff570b3c_9ea73650","line":1127,"updated":"2020-05-14 14:05:16.000000000","message":"OK - here\u0027s why I don\u0027t like this. Here\u0027s we\u0027re assuming that we\u0027re only passed a cpuset_reserved if the CPU policy is dedicated, but we\u0027re still using its value as its passed to us (even though we know it\u0027ll never be anything but 1). So we\u0027re both coupling to logic outside of this method, but still consuming information from the outside world.\n\nHow about we pass the thread policy into this _numa_fit_instance_cell() instead of cpuset_reserved, and work out the overhead in here? And get rid of L2091-2099.","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"09dfee100bae2e6562946a7f6993484ace94d32e","unresolved":false,"context_lines":[{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"    # NOTE(stephenfin): As with memory, do not allow an instance to overcommit"},{"line_number":1126,"context_line":"    # against itself on any NUMA cell"},{"line_number":1127,"context_line":"    if instance_cell.cpu_policy \u003d\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1128,"context_line":"        required_cpus \u003d len(instance_cell.cpuset) + cpuset_reserved"},{"line_number":1129,"context_line":"        if required_cpus \u003e len(host_cell.pcpuset):"},{"line_number":1130,"context_line":"            LOG.debug(\u0027Not enough host cell CPUs to fit instance cell; \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff570b3c_d936e8e6","line":1127,"in_reply_to":"ff570b3c_9ea73650","updated":"2020-05-14 14:12:04.000000000","message":"Yeahhhh, I\u0027m pretty sure I brought this up on the original review with Sahid years ago. Unfortunately we can\u0027t just pass in the thread value because we only add an additional core for guest NUMA node 0 (see the conditional on line 2092), meaning I\u0027d also have to pass in another field and it all gets even uglier.\n\nGiven that this whole thing goes away with [1], it probably makes sense to let this one be and review/iterate on that instead?\n\n[1] https://review.opendev.org/#/c/714703/","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0317bb7006e2509b0caa2e85f4b6afe9cc6baebe","unresolved":false,"context_lines":[{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"    # NOTE(stephenfin): As with memory, do not allow an instance to overcommit"},{"line_number":1126,"context_line":"    # against itself on any NUMA cell"},{"line_number":1127,"context_line":"    if instance_cell.cpu_policy \u003d\u003d fields.CPUAllocationPolicy.DEDICATED:"},{"line_number":1128,"context_line":"        required_cpus \u003d len(instance_cell.cpuset) + cpuset_reserved"},{"line_number":1129,"context_line":"        if required_cpus \u003e len(host_cell.pcpuset):"},{"line_number":1130,"context_line":"            LOG.debug(\u0027Not enough host cell CPUs to fit instance cell; \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff570b3c_b946944e","line":1127,"in_reply_to":"ff570b3c_d936e8e6","updated":"2020-05-14 14:15:16.000000000","message":"Egads, this dates back to Sahid\u0027s days. But fair point, no sense in arguing too much over this given the larger coming refactor.","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ba2c1d19c838b988d93e695f837b108741fcc8e","unresolved":false,"context_lines":[{"line_number":2097,"context_line":"                    # NUMA node associated to the guest NUMA node"},{"line_number":2098,"context_line":"                    # 0."},{"line_number":2099,"context_line":"                    cpuset_reserved \u003d 1"},{"line_number":2100,"context_line":"                got_cell \u003d _numa_fit_instance_cell("},{"line_number":2101,"context_line":"                    host_cell, instance_cell, limits, cpuset_reserved)"},{"line_number":2102,"context_line":"            except exception.MemoryPageSizeNotSupported:"},{"line_number":2103,"context_line":"                # This exception will been raised if instance cell\u0027s"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff570b3c_fe7912fc","line":2100,"updated":"2020-05-14 14:05:16.000000000","message":"_numa_fit_instance_cell() is only called here.","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ba2c1d19c838b988d93e695f837b108741fcc8e","unresolved":false,"context_lines":[{"line_number":2098,"context_line":"                    # 0."},{"line_number":2099,"context_line":"                    cpuset_reserved \u003d 1"},{"line_number":2100,"context_line":"                got_cell \u003d _numa_fit_instance_cell("},{"line_number":2101,"context_line":"                    host_cell, instance_cell, limits, cpuset_reserved)"},{"line_number":2102,"context_line":"            except exception.MemoryPageSizeNotSupported:"},{"line_number":2103,"context_line":"                # This exception will been raised if instance cell\u0027s"},{"line_number":2104,"context_line":"                # custom pagesize is not supported with host cell in"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff570b3c_3e5d4a7a","line":2101,"range":{"start_line":2101,"start_character":54,"end_line":2101,"end_character":69},"updated":"2020-05-14 14:05:16.000000000","message":"And this cpuset_reserved is only 1 if the \u0027if\u0027 on L2092 is True.","commit_id":"3823a5d0666206be7e708a1310a13f0330cb6ae7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cede4195189dbbf4c86e15a439bcd7541b61f232","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"                      \u0027required: %(required)d, actual: %(actual)d\u0027, {"},{"line_number":1145,"context_line":"                          \u0027required\u0027: len(instance_cell.cpuset),"},{"line_number":1146,"context_line":"                          \u0027actual\u0027: len(host_cell.cpuset),"},{"line_number":1147,"context_line":"                      })"},{"line_number":1148,"context_line":"            return"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if instance_cell.cpu_policy \u003d\u003d fields.CPUAllocationPolicy.DEDICATED:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_2d9d1e59","line":1147,"updated":"2020-06-19 10:25:47.000000000","message":"Hm the if and else branch was the same before?","commit_id":"c059f9a6bb5d2d45f419de05598c2ed8cdb980fd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6505e52f2d549ad9455ea1f8e558455783add28b","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"                      \u0027required: %(required)d, actual: %(actual)d\u0027, {"},{"line_number":1145,"context_line":"                          \u0027required\u0027: len(instance_cell.cpuset),"},{"line_number":1146,"context_line":"                          \u0027actual\u0027: len(host_cell.cpuset),"},{"line_number":1147,"context_line":"                      })"},{"line_number":1148,"context_line":"            return"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"    if instance_cell.cpu_policy \u003d\u003d fields.CPUAllocationPolicy.DEDICATED:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_e8c78d11","line":1147,"in_reply_to":"bf51134e_2d9d1e59","updated":"2020-06-19 16:46:52.000000000","message":"Yes, I\u0027m saying that the TODO and logging of cpuset_reserved were *never* necessary","commit_id":"c059f9a6bb5d2d45f419de05598c2ed8cdb980fd"}]}