)]}'
{"nova/tests/unit/virt/test_hardware.py":[{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"dc846a4e8219afcb8e00f13715e33782b1ec9f08","unresolved":false,"context_lines":[{"line_number":2364,"context_line":"        got_topo \u003d objects.VirtCPUTopology(sockets\u003d1, cores\u003d4, threads\u003d1)"},{"line_number":2365,"context_line":"        self.assertEqualTopology(got_topo, inst_pin.cpu_topology)"},{"line_number":2366,"context_line":""},{"line_number":2367,"context_line":""},{"line_number":2368,"context_line":"    def test_get_pinning_host_siblings_instance_odd_fit_orphan_only(self):"},{"line_number":2369,"context_line":"        host_pin \u003d objects.NUMACell(id\u003d0, cpuset\u003dset([0, 1, 2, 3, 4, 5, 6, 7]),"},{"line_number":2370,"context_line":"                                    memory\u003d4096, memory_usage\u003d0,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_2f62bbad","line":2367,"updated":"2016-07-19 20:47:36.000000000","message":"Too many blank lines -- see PEP8 failure on previous patch set.","commit_id":"ee63d0a0c6c4a1d12044883bd50f770618a5e21b"}],"nova/virt/hardware.py":[{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"389fe1529465aca72cdfdb2476f50549a6b18211","unresolved":false,"context_lines":[{"line_number":791,"context_line":"                break"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":794,"context_line":"    else:  # PREFER"},{"line_number":795,"context_line":"        for threads_no, sibling_set in sorted("},{"line_number":796,"context_line":"                (t for t in sibling_sets.items()), reverse\u003dTrue):"},{"line_number":797,"context_line":"            pinning \u003d _get_pinning(threads_no, sibling_set,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fad55976_3109ef32","line":794,"updated":"2016-07-15 17:03:18.000000000","message":"The comment is wrong, \"else\" here doesn\u0027t necessarily means that the policy was set to \"prefer\".  It could just be that the user hasn\u0027t set any policy.\n\nActually I\u0027m not sure if we should have an \"else\" case here, all this code duplication seems not right.","commit_id":"79f4236556b049e728ba07db5c12d5d12e9fb7d1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b19ad99b6e9155e5d6770a6bc6cd85f2d23adeb4","unresolved":false,"context_lines":[{"line_number":791,"context_line":"                break"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":794,"context_line":"    else:  # PREFER"},{"line_number":795,"context_line":"        for threads_no, sibling_set in sorted("},{"line_number":796,"context_line":"                (t for t in sibling_sets.items()), reverse\u003dTrue):"},{"line_number":797,"context_line":"            pinning \u003d _get_pinning(threads_no, sibling_set,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_8a9c7ec1","line":794,"in_reply_to":"fad55976_3109ef32","updated":"2016-07-19 17:49:40.000000000","message":"Prefer is the implicit default, so this is correct (albeit implicit). I can expand to clarify this though.\n\nRE: duplication - see comment below","commit_id":"79f4236556b049e728ba07db5c12d5d12e9fb7d1"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"389fe1529465aca72cdfdb2476f50549a6b18211","unresolved":false,"context_lines":[{"line_number":799,"context_line":"            if pinning:"},{"line_number":800,"context_line":"                break"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"        # NOTE(sfinucan): If siblings weren\u0027t available, fall back to linear"},{"line_number":803,"context_line":"        # assignment across cores"},{"line_number":804,"context_line":"        if not pinning:"},{"line_number":805,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fad55976_b462edab","line":802,"updated":"2016-07-15 17:03:18.000000000","message":"If this patch is only about adding a fallback method, maybe this could be done at L810 instead and leave the conditional logic here untouched.","commit_id":"79f4236556b049e728ba07db5c12d5d12e9fb7d1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b19ad99b6e9155e5d6770a6bc6cd85f2d23adeb4","unresolved":false,"context_lines":[{"line_number":799,"context_line":"            if pinning:"},{"line_number":800,"context_line":"                break"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"        # NOTE(sfinucan): If siblings weren\u0027t available, fall back to linear"},{"line_number":803,"context_line":"        # assignment across cores"},{"line_number":804,"context_line":"        if not pinning:"},{"line_number":805,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_2a8d927b","line":802,"in_reply_to":"fad55976_b462edab","updated":"2016-07-19 17:49:40.000000000","message":"Yes, that\u0027s the way I had done this before. However, it made the code path even more complicated. I thought a little bit of duplication would be worth it for more readable code flow. Thoughts?","commit_id":"79f4236556b049e728ba07db5c12d5d12e9fb7d1"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"389fe1529465aca72cdfdb2476f50549a6b18211","unresolved":false,"context_lines":[{"line_number":807,"context_line":""},{"line_number":808,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"    if not pinning:"},{"line_number":811,"context_line":"        return"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"    topology \u003d objects.VirtCPUTopology(sockets\u003d1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fad55976_d4c68115","line":810,"updated":"2016-07-15 17:03:18.000000000","message":"Maybe something like:\n\nif (instance_cell.cpu_thread_policy not in   \n    [fields.CPUThreadAllocationPolicy.REQUIRE, \n     fields.CPUThreadAllocationPolicy.ISOLATE]):\n    pinning \u003d zip(sorted(instance_cell.cpuset), itertools.chain(*sibling_set))\n....","commit_id":"79f4236556b049e728ba07db5c12d5d12e9fb7d1"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4cf5944209ce713dde217f57e4e9516428e02af7","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        pinning \u003d _get_pinning(1,  # we only want to \"use\" one thread per core"},{"line_number":772,"context_line":"                               sibling_sets[threads_per_core],"},{"line_number":773,"context_line":"                               instance_cell.cpuset)"},{"line_number":774,"context_line":"    else:  # ISOLATE, PREFER (explicit, implicit)"},{"line_number":775,"context_line":"        # NOTE(ndipanov): We iterate over the sibling sets in descending order"},{"line_number":776,"context_line":"        # of cores that can be packed. This is an attempt to evenly distribute"},{"line_number":777,"context_line":"        # instances among physical cores"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_b06e0c94","line":774,"range":{"start_line":774,"start_character":13,"end_line":774,"end_character":20},"updated":"2016-07-19 22:14:33.000000000","message":"shouldn\u0027t that be REQUIRE instead of ISOLATE?","commit_id":"51eba92cfb57408378cc5689e300abe58593fe84"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef236b1ba9187cd8e78befba234091d785e423fe","unresolved":false,"context_lines":[{"line_number":771,"context_line":"        pinning \u003d _get_pinning(1,  # we only want to \"use\" one thread per core"},{"line_number":772,"context_line":"                               sibling_sets[threads_per_core],"},{"line_number":773,"context_line":"                               instance_cell.cpuset)"},{"line_number":774,"context_line":"    else:  # ISOLATE, PREFER (explicit, implicit)"},{"line_number":775,"context_line":"        # NOTE(ndipanov): We iterate over the sibling sets in descending order"},{"line_number":776,"context_line":"        # of cores that can be packed. This is an attempt to evenly distribute"},{"line_number":777,"context_line":"        # instances among physical cores"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_d07d801d","line":774,"range":{"start_line":774,"start_character":13,"end_line":774,"end_character":20},"in_reply_to":"dada55a8_b06e0c94","updated":"2016-07-19 22:16:16.000000000","message":"dammit","commit_id":"51eba92cfb57408378cc5689e300abe58593fe84"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"8d49251d9bef39c2ad32784f88150a242db7665d","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        # cores"},{"line_number":801,"context_line":"        if (instance_cell.cpu_thread_policy !\u003d"},{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a8ec9b2_64c5e53b","line":803,"range":{"start_line":803,"start_character":20,"end_line":803,"end_character":27},"updated":"2016-09-20 18:15:00.000000000","message":"It looks to me like it\u0027s possible for this to not be defined yet. If the conditional on line 790 is True for each iteration of the for loop above \u0027pinning\u0027 will not be set. I can\u0027t easily tell what would be required to make that happen, or if it\u0027s possible. But an easy fix would be to set \u0027pinning\u0027 to a default somewhere above.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"487dcd0718995e3c364db24a5d39703c87bc6488","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        # cores"},{"line_number":801,"context_line":"        if (instance_cell.cpu_thread_policy !\u003d"},{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a8ec9b2_9544ddb7","line":803,"range":{"start_line":803,"start_character":20,"end_line":803,"end_character":27},"in_reply_to":"7a8ec9b2_35cea983","updated":"2016-09-20 19:36:48.000000000","message":"Ah, right you are. I\u0027d still prefer to have pinning set to a sane default early on so we know it\u0027s defined. But not enough to hold up the bug fix.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b56a460c6c915c00e542ff94a89ff6b0f53be644","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        # cores"},{"line_number":801,"context_line":"        if (instance_cell.cpu_thread_policy !\u003d"},{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a8ec9b2_35cea983","line":803,"range":{"start_line":803,"start_character":20,"end_line":803,"end_character":27},"in_reply_to":"7a8ec9b2_64c5e53b","updated":"2016-09-20 19:28:40.000000000","message":"Hm, it looks like in order for it to bypass the pinning variable assignment on L793, instance._cell.cpu_thread_policy has to be REQUIRE and this only checks the pinning variable if instance_cell.cpu_thread_policy is not REQUIRE.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e570323eab3bb5cf775a44d6d40d2c7f4c85bb3e","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_7bd76423","line":805,"range":{"start_line":805,"start_character":43,"end_line":805,"end_character":54},"updated":"2016-09-09 21:29:32.000000000","message":"This is pretty subtle (to me). This will be the last sibling_set in sibling_sets, which will be the set including the non-siblings because of the reverse sort on L781. The sibling_set variable would also be undefined if sibling_sets were empty, but I see that this function _pack_instance_onto_cores isn\u0027t called in that case.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9bf137ceb111d64cc804349583308fcd92d97c42","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_e5e54a38","line":805,"range":{"start_line":805,"start_character":43,"end_line":805,"end_character":54},"in_reply_to":"9a89bdaa_7bd76423","updated":"2016-10-06 20:11:50.000000000","message":"Nit...might make sense to add a comment in the code including the above explanation.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f1ea5f9a557a15fcb93bcfb63474e1ca4cd4c51d","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                fields.CPUThreadAllocationPolicy.REQUIRE and"},{"line_number":803,"context_line":"                not pinning):"},{"line_number":804,"context_line":"            pinning \u003d zip(sorted(instance_cell.cpuset),"},{"line_number":805,"context_line":"                          itertools.chain(*sibling_set))"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        threads_no \u003d _threads(instance_cell, threads_no)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1a95cdbc_bf942f43","line":805,"range":{"start_line":805,"start_character":43,"end_line":805,"end_character":54},"in_reply_to":"9a89bdaa_7bd76423","updated":"2016-10-06 19:46:41.000000000","message":"This is definitely hard to read code, at least for me.","commit_id":"8361d8d6c315bb3ae71c3ff0147f7d5156bc46f3"}]}