)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eaf748bf040f9c906e3abe232a199c2a3749f58b","unresolved":true,"context_lines":[{"line_number":14,"context_line":"VM configuration."},{"line_number":15,"context_line":"(\u003cvcpu cpuset\u003d\"0-1\"\u003e will be updated to \u003cvcpu cpuset\u003d\"2-3\"\u003e)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This update adds a new field, dst_cpu_shared_set_info, to the"},{"line_number":18,"context_line":"LibvirtLiveMigrateData object, which requires an increase in the"},{"line_number":19,"context_line":"object\u0027s version. As a result, this patch cannot be backported."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bfc62833_7c250f0a","line":17,"updated":"2024-04-16 17:31:16.000000000","message":"This is no longer true.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3310a7a63462d53b77609f18e75b0c8778435ebd","unresolved":true,"context_lines":[{"line_number":14,"context_line":"VM configuration."},{"line_number":15,"context_line":"(\u003cvcpu cpuset\u003d\"0-1\"\u003e will be updated to \u003cvcpu cpuset\u003d\"2-3\"\u003e)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This update adds a new field, dst_cpu_shared_set_info, to the"},{"line_number":18,"context_line":"LibvirtLiveMigrateData object, which requires an increase in the"},{"line_number":19,"context_line":"object\u0027s version. As a result, this patch cannot be backported."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ca70eb11_cdccded1","line":17,"in_reply_to":"130ded4b_9196a7b1","updated":"2024-04-24 14:52:51.000000000","message":"I think fixing commit messages is important, FWIW. The context of these two patches being together will be lost in a year when we\u0027re fixing a bug.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":14,"context_line":"VM configuration."},{"line_number":15,"context_line":"(\u003cvcpu cpuset\u003d\"0-1\"\u003e will be updated to \u003cvcpu cpuset\u003d\"2-3\"\u003e)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This update adds a new field, dst_cpu_shared_set_info, to the"},{"line_number":18,"context_line":"LibvirtLiveMigrateData object, which requires an increase in the"},{"line_number":19,"context_line":"object\u0027s version. As a result, this patch cannot be backported."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"130ded4b_9196a7b1","line":17,"in_reply_to":"bfc62833_7c250f0a","updated":"2024-04-24 14:34:40.000000000","message":"this is a nit but yes that is now in the previous patch","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eaf748bf040f9c906e3abe232a199c2a3749f58b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5ef8cd9f_09af7313","updated":"2024-04-16 17:31:16.000000000","message":"Small issue in the commit message, but no need to fix unless you respin for other reasons.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6082bd0f_13ab31fa","updated":"2024-04-24 14:34:40.000000000","message":"some nits but i think this is mergable\nmainly style/prefernce comments inlien","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0d8927f1dcbdd9e2d1145f77ad3c230b379fa6e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5e59cb4e_50c05456","updated":"2024-04-24 15:00:23.000000000","message":"Sean was +2 before I just fixed up the commit message issue and the boolean logic per the comments.","commit_id":"43dadaeb901f47f8d5ff9e818e62bdb32046527e"}],"nova/tests/functional/libvirt/test_live_migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":false,"context_lines":[{"line_number":263,"context_line":"        xml \u003d dom.XMLDesc(0)"},{"line_number":264,"context_line":"        # The destination should be updated to \"3-4\"."},{"line_number":265,"context_line":"        self.assertNotIn(\u0027\u003cvcpu cpuset\u003d\"0-1\"\u003e1\u003c/vcpu\u003e\u0027, xml)"},{"line_number":266,"context_line":"        self.assertIn(\u0027\u003cvcpu cpuset\u003d\"3-4\"\u003e1\u003c/vcpu\u003e\u0027, xml)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    def test_live_migration_to_no_cpu_shared_set(self):"},{"line_number":269,"context_line":"        \"\"\"Reproducer for bug 1869804 #2."}],"source_content_type":"text/x-python","patch_set":3,"id":"5bf31cfc_ea1fb3e9","line":266,"updated":"2024-04-24 14:34:40.000000000","message":"ack this asserts the bug is now adressed","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"}],"nova/tests/unit/virt/libvirt/test_migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    @mock.patch.object(migration, \u0027_update_serial_xml\u0027)"},{"line_number":77,"context_line":"    @mock.patch.object(migration, \u0027_update_volume_xml\u0027)"},{"line_number":78,"context_line":"    def _test_get_updated_guest_xml("},{"line_number":79,"context_line":"        self, data, check_cpu_shared_set, mock_volume, mock_serial,"},{"line_number":80,"context_line":"        mock_graphics, mock_perf_events_xml, mock_memory_backing,"},{"line_number":81,"context_line":"        mock_update_cpu_shared_set, mock_remove_cpu_shared_set, mock_tostring"},{"line_number":82,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":3,"id":"af027fe4_dcf2ea1a","line":79,"range":{"start_line":79,"start_character":20,"end_line":79,"end_character":40},"updated":"2024-04-24 14:34:40.000000000","message":"nit this is not realy \n\ncheck_cpu_shared_set\n\nthis is a addtional optional test predicate\n\nyou are usign it to assert if \n\n mock_update_cpu_shared_set and mock_remove_cpu_shared_set are called.\n \n you could have done this by having \n \n _test_get_updated_guest_xml return the mocks and then doing the assert yourself directly in the new fucntions.\n \n what you have done works but i would argue is less clean","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":120,"context_line":"            data.dst_cpu_shared_set_info \u003d None"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"},{"line_number":123,"context_line":"        self.assertRaises(ValueError, set_dst_cpu_shared_set_info)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def test_get_updated_guest_xml_dst_cpu_shared_set_info_set_to_empty(self):"},{"line_number":126,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":3,"id":"25b84311_d060d79e","line":123,"updated":"2024-04-24 14:34:40.000000000","message":"ok this is jsut checkign that it cant be None because we dont declare the filed as nullable.\n\nfine i dont think we need to test this but i dont think you need to remove this.\n\nwe dont really need to test the internals of oslo.versioned_object to this degree.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d526504b78f32bbed54d9f05a2c5e565824b9841","unresolved":true,"context_lines":[{"line_number":120,"context_line":"            data.dst_cpu_shared_set_info \u003d None"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"},{"line_number":123,"context_line":"        self.assertRaises(ValueError, set_dst_cpu_shared_set_info)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def test_get_updated_guest_xml_dst_cpu_shared_set_info_set_to_empty(self):"},{"line_number":126,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":3,"id":"0778c2f0_6b8eb415","line":123,"in_reply_to":"25b84311_d060d79e","updated":"2024-04-24 14:51:31.000000000","message":"Agree.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":314,"context_line":"              \u003cvcpu\u003e1\u003c/vcpu\u003e"},{"line_number":315,"context_line":"            \u003c/domain\u003e\"\"\")"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        self.assertXmlEqual(expected, result)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    def test_update_numa_xml(self):"},{"line_number":320,"context_line":"        doc \u003d etree.fromstring(\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"9d6b8923_2067c941","line":317,"updated":"2024-04-24 14:34:40.000000000","message":"+1","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":10011,"context_line":"            hardware.get_cpu_shared_set() or"},{"line_number":10012,"context_line":"            hardware.get_vcpu_pin_set() or"},{"line_number":10013,"context_line":"            set()"},{"line_number":10014,"context_line":"        )"},{"line_number":10015,"context_line":""},{"line_number":10016,"context_line":"        # NOTE(sean-k-mooney): The migrate_data vifs field is used to signal"},{"line_number":10017,"context_line":"        # that we are using the multiple port binding workflow so we can only"}],"source_content_type":"text/x-python","patch_set":3,"id":"160d1d3b_ad0b3259","line":10014,"updated":"2024-04-24 14:34:40.000000000","message":"this is what we were discussing in https://review.opendev.org/c/openstack/nova/+/903706/comment/efe9ffe9_eab8a1e0","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"639d8ccf48eb898784d712704da6a0dea0cca2fc","unresolved":true,"context_lines":[{"line_number":69,"context_line":"    # If \u0027dst_cpu_shared_set_info\u0027 is set, we are migrating a VM to a"},{"line_number":70,"context_line":"    # destination host, patched to fix bug 1869804."},{"line_number":71,"context_line":"    # Then, if dst_cpu_shared_set_info is None, it means that there is no"},{"line_number":72,"context_line":"    # cpu_shared_set configuration on the destination host."},{"line_number":73,"context_line":"    if ("},{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info is None"}],"source_content_type":"text/x-python","patch_set":1,"id":"affc9fbc_40470c1e","line":72,"updated":"2024-04-05 15:12:58.000000000","message":"I don\u0027t see where this can ever be set to none. Both of the hardware modules that provide the value for this field return lists, no?","commit_id":"21b60e6a26bdda0b77b861a217b11e30716cb326"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d526504b78f32bbed54d9f05a2c5e565824b9841","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    # If \u0027dst_cpu_shared_set_info\u0027 is set, we are migrating a VM to a"},{"line_number":70,"context_line":"    # destination host, patched to fix bug 1869804."},{"line_number":71,"context_line":"    # Then, if dst_cpu_shared_set_info is None, it means that there is no"},{"line_number":72,"context_line":"    # cpu_shared_set configuration on the destination host."},{"line_number":73,"context_line":"    if ("},{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info is None"}],"source_content_type":"text/x-python","patch_set":1,"id":"a27b44c5_fe856f0e","line":72,"in_reply_to":"96d783c6_f44a5aff","updated":"2024-04-24 14:51:31.000000000","message":"Done","commit_id":"21b60e6a26bdda0b77b861a217b11e30716cb326"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"43a6944842fa091ec0e086c4fe7203b20dc4e8b1","unresolved":true,"context_lines":[{"line_number":69,"context_line":"    # If \u0027dst_cpu_shared_set_info\u0027 is set, we are migrating a VM to a"},{"line_number":70,"context_line":"    # destination host, patched to fix bug 1869804."},{"line_number":71,"context_line":"    # Then, if dst_cpu_shared_set_info is None, it means that there is no"},{"line_number":72,"context_line":"    # cpu_shared_set configuration on the destination host."},{"line_number":73,"context_line":"    if ("},{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info is None"}],"source_content_type":"text/x-python","patch_set":1,"id":"96d783c6_f44a5aff","line":72,"in_reply_to":"affc9fbc_40470c1e","updated":"2024-04-08 14:21:15.000000000","message":"Please have a look at https://review.opendev.org/c/openstack/nova/+/903706/comment/efe9ffe9_eab8a1e0/.","commit_id":"21b60e6a26bdda0b77b861a217b11e30716cb326"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":70,"context_line":"    # destination host, patched to fix bug 1869804."},{"line_number":71,"context_line":"    # Then, if dst_cpu_shared_set_info is empty (set()), it means that there"},{"line_number":72,"context_line":"    # is no cpu_shared_set configuration on the destination host."},{"line_number":73,"context_line":"    if ("},{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info \u003d\u003d set()"},{"line_number":76,"context_line":"    ):"},{"line_number":77,"context_line":"        # There is no cpu_shared_set configured on destination host. So we"},{"line_number":78,"context_line":"        # need to remove the VM cpuset if any."},{"line_number":79,"context_line":"        xml_doc \u003d _remove_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"334ec593_91dd2a23","line":76,"range":{"start_line":73,"start_character":4,"end_line":76,"end_character":6},"updated":"2024-04-24 14:34:40.000000000","message":"i think this can be \n\nif (\n        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and\n         not migrate_data.dst_cpu_shared_set_info\n    ):","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"524c7d1bef26bd278015c80d882ea92911ffaf5b","unresolved":true,"context_lines":[{"line_number":70,"context_line":"    # destination host, patched to fix bug 1869804."},{"line_number":71,"context_line":"    # Then, if dst_cpu_shared_set_info is empty (set()), it means that there"},{"line_number":72,"context_line":"    # is no cpu_shared_set configuration on the destination host."},{"line_number":73,"context_line":"    if ("},{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info \u003d\u003d set()"},{"line_number":76,"context_line":"    ):"},{"line_number":77,"context_line":"        # There is no cpu_shared_set configured on destination host. So we"},{"line_number":78,"context_line":"        # need to remove the VM cpuset if any."},{"line_number":79,"context_line":"        xml_doc \u003d _remove_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fc16c8e4_33db4cf4","line":76,"range":{"start_line":73,"start_character":4,"end_line":76,"end_character":6},"in_reply_to":"334ec593_91dd2a23","updated":"2024-04-24 14:39:51.000000000","message":"Yeah, comparing to an empty set is less desirable than checking for falsiness, IMHO. I\u0027d prefer sean\u0027s version. Of course, I\u0027d prefer it on two lines instead of four but....","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f035e475485bfcc7dd9103bf1310240ec462a3fd","unresolved":true,"context_lines":[{"line_number":80,"context_line":"    if ("},{"line_number":81,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":82,"context_line":"        migrate_data.dst_cpu_shared_set_info !\u003d set()"},{"line_number":83,"context_line":"    ):"},{"line_number":84,"context_line":"        # There is cpu_shared_set configured on destination host. So we need"},{"line_number":85,"context_line":"        # to update the VM cpuset."},{"line_number":86,"context_line":"        xml_doc \u003d _update_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ba8de47_c58877c0","line":83,"updated":"2024-04-24 14:34:40.000000000","message":"and this could be\n\n if (\n        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and\n        migrate_data.dst_cpu_shared_set_info\n    ):\n    \nor \n\n elif (\n        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and\n        migrate_data.dst_cpu_shared_set_info\n    ):\n    \n    \nbut i guess what you have is fine\n\n\u003e\u003e\u003e bool(set([0]))\nTrue\n\nany non empty set is true\n\n\u003e\u003e\u003e bool(set())\nFalse","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"524c7d1bef26bd278015c80d882ea92911ffaf5b","unresolved":true,"context_lines":[{"line_number":80,"context_line":"    if ("},{"line_number":81,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":82,"context_line":"        migrate_data.dst_cpu_shared_set_info !\u003d set()"},{"line_number":83,"context_line":"    ):"},{"line_number":84,"context_line":"        # There is cpu_shared_set configured on destination host. So we need"},{"line_number":85,"context_line":"        # to update the VM cpuset."},{"line_number":86,"context_line":"        xml_doc \u003d _update_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"2957c850_2da75096","line":83,"in_reply_to":"7ba8de47_c58877c0","updated":"2024-04-24 14:39:51.000000000","message":"Again I agree, instantiating a whole set object and comparing the other set to it to determine if it\u0027s empty or not is really odd and more CPU-intensive than it needs to be. Why do this? Either use the inbuilt bool-ness or use `len()`, but the former is conventional python, IMHO.","commit_id":"4c290a541178da718a015384d6c17cf3fb0051f1"}]}