)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check both source and dest compute libvirt versions for mdev lv"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since only libvirt 8.6.0 supports mdev live-migration, we need to verify"},{"line_number":10,"context_line":"the values of the hypervisor for both the source and the destination."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"If one of them are older, the conductor raises an exception that will"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"64fa16a5_9a2ace3c","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":24},"updated":"2024-01-22 15:44:57.000000000","message":"If there some specific needs for qemu as well ?\nIt seems you want a qemu 8.1 which is higher than the default min version.\nSo I think you should add it in this msg.","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5d228eaaa57e3566885d03e318afa034b07b0438","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check both source and dest compute libvirt versions for mdev lv"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since only libvirt 8.6.0 supports mdev live-migration, we need to verify"},{"line_number":10,"context_line":"the values of the hypervisor for both the source and the destination."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"If one of them are older, the conductor raises an exception that will"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"d35a0d7d_3f1ce432","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":24},"in_reply_to":"64fa16a5_9a2ace3c","updated":"2024-01-25 11:19:24.000000000","message":"Done","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"74dd70bf2ef156db276e05cdaeb21d03641c6595","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0a4ca9a3_5cee39a3","updated":"2023-12-22 08:28:20.000000000","message":"recheck unrelated volume attachment issue on osc-func-devstack job","commit_id":"bb87e7870fc0799a685aef57dec8388cc0afb026"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"973b7dd2_76dc4374","updated":"2024-01-22 15:44:57.000000000","message":"Just a -1 for minor things. Overall it looks good.\nI checked that dest is checked before source.","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f046b89c5154c86a12ff4b7f290de3f93c045aa5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f79f7058_c5da711e","updated":"2024-01-17 18:10:47.000000000","message":"recheck timeout for deleting image on nova-lvm","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8bca6b8fa08b0b43af7aff462aa111c2535db0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a0c8097f_c8d50b2d","updated":"2024-02-02 14:58:42.000000000","message":"Just nits, nothing critical","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1f186d926c5ead12f2dda2557bca92620d9d343","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"1a623e5a_9937a537","updated":"2024-02-14 11:23:33.000000000","message":"given dan is fine with this i dont think we need to respin so lets proceed with the patch as is","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dafce741510958ed1a5c25e70d99863a2fe3ef64","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b40e0c7d_99efd58a","updated":"2024-02-20 12:09:57.000000000","message":"recheck https://review.opendev.org/c/openstack/nova/+/909469 has merged so hopefully that helps stablise the nova-lvm job","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2eac3d19d4437512823cfe4f5bd12245b7637bf8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"86f20f3c_97a346e8","updated":"2024-02-16 09:49:55.000000000","message":"recheck nova-lvm gets me mad","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8535dae6bccb09caa68f89d57fb71f4e333280d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d74fc63b_c7739171","updated":"2024-02-23 11:22:05.000000000","message":"recheck nova-next is now fixed","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9ed91dc7a6475656da8443dc5f6a39f6b1952335","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e5367d13_38d31458","updated":"2024-02-15 13:06:47.000000000","message":"recheck post failure","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"}],"nova/tests/functional/libvirt/test_vgpu.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ab950d92be3cc04f5b1dacc581126e913cd1d407","unresolved":true,"context_lines":[{"line_number":546,"context_line":"            flavor_id\u003dself.flavor, networks\u003d\u0027auto\u0027, host\u003dself.src.host)"},{"line_number":547,"context_line":"        inst \u003d objects.Instance.get_by_uuid(self.context, self.server[\u0027id\u0027])"},{"line_number":548,"context_line":"        mdevs \u003d self.src.driver._get_all_assigned_mediated_devices("},{"line_number":549,"context_line":"                inst)"},{"line_number":550,"context_line":"        self.assertEqual(1, len(mdevs))"},{"line_number":551,"context_line":"        self._live_migrate(self.server, \u0027completed\u0027)"},{"line_number":552,"context_line":"        # FIXME(sbauza): The domain is fully copied to the destination so the"}],"source_content_type":"text/x-python","patch_set":6,"id":"91362fc7_36a9dffc","line":549,"updated":"2024-01-30 14:45:15.000000000","message":"Should be indented? Although it will probably fit on the line above...","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8de58ef7ad03a0cc04c2bfed5d40aac8d1dd3ace","unresolved":false,"context_lines":[{"line_number":546,"context_line":"            flavor_id\u003dself.flavor, networks\u003d\u0027auto\u0027, host\u003dself.src.host)"},{"line_number":547,"context_line":"        inst \u003d objects.Instance.get_by_uuid(self.context, self.server[\u0027id\u0027])"},{"line_number":548,"context_line":"        mdevs \u003d self.src.driver._get_all_assigned_mediated_devices("},{"line_number":549,"context_line":"                inst)"},{"line_number":550,"context_line":"        self.assertEqual(1, len(mdevs))"},{"line_number":551,"context_line":"        self._live_migrate(self.server, \u0027completed\u0027)"},{"line_number":552,"context_line":"        # FIXME(sbauza): The domain is fully copied to the destination so the"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ec69f18_b2e2bcc7","line":549,"in_reply_to":"91362fc7_36a9dffc","updated":"2024-01-30 18:17:51.000000000","message":"Done","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":true,"context_lines":[{"line_number":12007,"context_line":""},{"line_number":12008,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":12009,"context_line":"                       \u0027_host_can_support_mdev_live_migration\u0027)"},{"line_number":12010,"context_line":"    def test_assert_source_can_lv_mdevs_fails_due_to_host(self, mock_host):"},{"line_number":12011,"context_line":"        mock_host.return_value \u003d False"},{"line_number":12012,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":12013,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"68899802_9b5c40ec","line":12010,"range":{"start_line":12010,"start_character":53,"end_line":12010,"end_character":57},"updated":"2024-01-22 15:44:57.000000000","message":"I would use source here.","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5d228eaaa57e3566885d03e318afa034b07b0438","unresolved":false,"context_lines":[{"line_number":12007,"context_line":""},{"line_number":12008,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":12009,"context_line":"                       \u0027_host_can_support_mdev_live_migration\u0027)"},{"line_number":12010,"context_line":"    def test_assert_source_can_lv_mdevs_fails_due_to_host(self, mock_host):"},{"line_number":12011,"context_line":"        mock_host.return_value \u003d False"},{"line_number":12012,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":12013,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"e22c2357_5076e296","line":12010,"range":{"start_line":12010,"start_character":53,"end_line":12010,"end_character":57},"in_reply_to":"68899802_9b5c40ec","updated":"2024-01-25 11:19:24.000000000","message":"Done","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8bca6b8fa08b0b43af7aff462aa111c2535db0","unresolved":true,"context_lines":[{"line_number":11558,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_compare_cpu\u0027)"},{"line_number":11559,"context_line":"    def test_check_can_live_migrate_dest_mdev_lm("},{"line_number":11560,"context_line":"        self, mock_cpu, mock_test_file, mock_can_sup_mdev_lm,"},{"line_number":11561,"context_line":"    ):"},{"line_number":11562,"context_line":"        mock_can_sup_mdev_lm.return_value \u003d True"},{"line_number":11563,"context_line":"        instance_ref \u003d objects.Instance(**self.test_instance)"},{"line_number":11564,"context_line":"        instance_ref.numa_topology \u003d objects.InstanceNUMATopology("}],"source_content_type":"text/x-python","patch_set":7,"id":"1433e098_4e2d7f60","line":11561,"updated":"2024-02-02 14:58:42.000000000","message":"More wasted vertical space here, but I see you\u0027re keeping the pattern of wasting vertical space in the rest of this file, so I guess that\u0027s \"good\" :(","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8bca6b8fa08b0b43af7aff462aa111c2535db0","unresolved":true,"context_lines":[{"line_number":11589,"context_line":"        compute_info \u003d {\u0027cpu_info\u0027: \u0027asdf\u0027, \u0027disk_available_least\u0027: 1}"},{"line_number":11590,"context_line":"        result \u003d drvr.check_can_live_migrate_destination("},{"line_number":11591,"context_line":"            self.context, instance_ref, compute_info, compute_info)"},{"line_number":11592,"context_line":"        self.assertNotIn(\u0027dst_supports_mdev_live_migration\u0027, result)"},{"line_number":11593,"context_line":""},{"line_number":11594,"context_line":"    @mock.patch("},{"line_number":11595,"context_line":"        \u0027nova.network.neutron.API.has_port_binding_extension\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"97500a08_8ca1f33e","line":11592,"updated":"2024-02-02 14:58:42.000000000","message":"All of this is duplicated to run the above again with False? :)","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1f186d926c5ead12f2dda2557bca92620d9d343","unresolved":true,"context_lines":[{"line_number":11589,"context_line":"        compute_info \u003d {\u0027cpu_info\u0027: \u0027asdf\u0027, \u0027disk_available_least\u0027: 1}"},{"line_number":11590,"context_line":"        result \u003d drvr.check_can_live_migrate_destination("},{"line_number":11591,"context_line":"            self.context, instance_ref, compute_info, compute_info)"},{"line_number":11592,"context_line":"        self.assertNotIn(\u0027dst_supports_mdev_live_migration\u0027, result)"},{"line_number":11593,"context_line":""},{"line_number":11594,"context_line":"    @mock.patch("},{"line_number":11595,"context_line":"        \u0027nova.network.neutron.API.has_port_binding_extension\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"6a7aee88_9afdbb0f","line":11592,"in_reply_to":"97500a08_8ca1f33e","updated":"2024-02-14 11:23:33.000000000","message":"ya this can proablly be extracted to a shared test function and just pass that in\n\nor you ddt","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1f186d926c5ead12f2dda2557bca92620d9d343","unresolved":true,"context_lines":[{"line_number":11701,"context_line":""},{"line_number":11702,"context_line":"    @mock.patch.object(fakelibvirt.Connection, \u0027getLibVersion\u0027)"},{"line_number":11703,"context_line":"    @mock.patch.object(fakelibvirt.Connection, \u0027getVersion\u0027)"},{"line_number":11704,"context_line":"    def _test_host_can_support_mdev_lm(self, mock_getversion,"},{"line_number":11705,"context_line":"                                      mock_getlibversion,"},{"line_number":11706,"context_line":"                                      old_libvirt, old_qemu, expected):"},{"line_number":11707,"context_line":"        min_libvirt_ver \u003d versionutils.convert_version_to_int("}],"source_content_type":"text/x-python","patch_set":7,"id":"c8e6b019_b7c1b904","line":11704,"updated":"2024-02-14 11:23:33.000000000","message":"sylvain has extracted the common code here so i dont mind the duplication above as much.","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1f186d926c5ead12f2dda2557bca92620d9d343","unresolved":true,"context_lines":[{"line_number":12015,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":12016,"context_line":"                       \u0027_host_can_support_mdev_live_migration\u0027)"},{"line_number":12017,"context_line":"    def test_assert_source_can_lv_mdevs_fails_due_to_src(self, mock_host):"},{"line_number":12018,"context_line":"        mock_host.return_value \u003d False"},{"line_number":12019,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":12020,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":12021,"context_line":"        dest_check_data \u003d objects.LibvirtLiveMigrateData("}],"source_content_type":"text/x-python","patch_set":7,"id":"0a83d5b4_17a0cf04","line":12018,"updated":"2024-02-14 11:23:33.000000000","message":"you know you can just set this when you do the patch adn if you dont\ntest it or pass it to anything you can also do new\u003dmock.Mock(return_value\u003dtrue) without passing it as an arge to the test function.","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"576deea4e88bcaf1cf979cb94059e2977a79e600","unresolved":true,"context_lines":[{"line_number":8398,"context_line":"        return mediated_devices"},{"line_number":8399,"context_line":""},{"line_number":8400,"context_line":"    def _get_mdev_types_from_uuids(self, mdev_uuids):"},{"line_number":8401,"context_line":"        \"\"\"Returns a dict of mdev and its type from a list of mediated device"},{"line_number":8402,"context_line":"        UUIDs. If no mdevs are actually using those UUIDs, it returns an empty"},{"line_number":8403,"context_line":"        dict."},{"line_number":8404,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"6da6af52_ec083381","line":8401,"range":{"start_line":8401,"start_character":28,"end_line":8401,"end_character":47},"updated":"2023-12-21 12:40:37.000000000","message":"Nit: I\u0027d use plural, \"mdevs and their type\"","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ffedb9c2feca9dba2c0b7f41b3b216566f0ba3c","unresolved":false,"context_lines":[{"line_number":8398,"context_line":"        return mediated_devices"},{"line_number":8399,"context_line":""},{"line_number":8400,"context_line":"    def _get_mdev_types_from_uuids(self, mdev_uuids):"},{"line_number":8401,"context_line":"        \"\"\"Returns a dict of mdev and its type from a list of mediated device"},{"line_number":8402,"context_line":"        UUIDs. If no mdevs are actually using those UUIDs, it returns an empty"},{"line_number":8403,"context_line":"        dict."},{"line_number":8404,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ebf403a7_f9725dec","line":8401,"range":{"start_line":8401,"start_character":28,"end_line":8401,"end_character":47},"in_reply_to":"6da6af52_ec083381","updated":"2023-12-21 13:15:26.000000000","message":"Acknowledged","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"576deea4e88bcaf1cf979cb94059e2977a79e600","unresolved":true,"context_lines":[{"line_number":9763,"context_line":""},{"line_number":9764,"context_line":"        # Just flag the fact we can live-migrate mdevs even if we don\u0027t use"},{"line_number":9765,"context_line":"        # them so the source will know we can use this compute."},{"line_number":9766,"context_line":"        if self._host_can_support_mdev_live_migration():"},{"line_number":9767,"context_line":"            data.dst_supports_mdev_live_migration \u003d True"},{"line_number":9768,"context_line":""},{"line_number":9769,"context_line":"        return data"},{"line_number":9770,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"2d8c7614_554b07f8","line":9767,"range":{"start_line":9766,"start_character":0,"end_line":9767,"end_character":56},"updated":"2023-12-21 12:40:37.000000000","message":"Thanks for this; this sounds like a handy check to account for all the mdev-capable hosts.","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ffedb9c2feca9dba2c0b7f41b3b216566f0ba3c","unresolved":false,"context_lines":[{"line_number":9763,"context_line":""},{"line_number":9764,"context_line":"        # Just flag the fact we can live-migrate mdevs even if we don\u0027t use"},{"line_number":9765,"context_line":"        # them so the source will know we can use this compute."},{"line_number":9766,"context_line":"        if self._host_can_support_mdev_live_migration():"},{"line_number":9767,"context_line":"            data.dst_supports_mdev_live_migration \u003d True"},{"line_number":9768,"context_line":""},{"line_number":9769,"context_line":"        return data"},{"line_number":9770,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e4625924_bb5fd7b7","line":9767,"range":{"start_line":9766,"start_character":0,"end_line":9767,"end_character":56},"in_reply_to":"2d8c7614_554b07f8","updated":"2023-12-21 13:15:26.000000000","message":"Acknowledged","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"576deea4e88bcaf1cf979cb94059e2977a79e600","unresolved":true,"context_lines":[{"line_number":9988,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":9989,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":9990,"context_line":""},{"line_number":9991,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9992,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9993,"context_line":"        ):"},{"line_number":9994,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9995,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"1c9ec01f_b73d337d","line":9992,"range":{"start_line":9991,"start_character":0,"end_line":9992,"end_character":64},"updated":"2023-12-21 12:40:37.000000000","message":"I\u0027m curious as a newbie: Why do we need both ways to check (component versions and also at the destination object)?  Isn\u0027t doing just one thing robust enough?","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ffedb9c2feca9dba2c0b7f41b3b216566f0ba3c","unresolved":false,"context_lines":[{"line_number":9988,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":9989,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":9990,"context_line":""},{"line_number":9991,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9992,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9993,"context_line":"        ):"},{"line_number":9994,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9995,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"ac22eec3_4209e9d4","line":9992,"range":{"start_line":9991,"start_character":0,"end_line":9992,"end_character":64},"in_reply_to":"1c9ec01f_b73d337d","updated":"2023-12-21 13:15:26.000000000","message":"We need to make sure that both of the computes can support the right libvirt/qemu versions, hence why we pass this field back to the source.","commit_id":"d69c5adf2d7e8a7bfc3c1227cf6c4c3869f700db"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":true,"context_lines":[{"line_number":8409,"context_line":"        :returns: dict where key is the mdev UUID and the value is its type."},{"line_number":8410,"context_line":"        \"\"\""},{"line_number":8411,"context_line":"        host_mdevs \u003d self._get_mediated_devices()"},{"line_number":8412,"context_line":"        inst_dev_infos \u003d filter(lambda dev: dev[\u0027uuid\u0027] in mdev_uuids,"},{"line_number":8413,"context_line":"                                host_mdevs)"},{"line_number":8414,"context_line":"        return {mdev[\u0027uuid\u0027]: mdev[\u0027type\u0027] for mdev in inst_dev_infos}"},{"line_number":8415,"context_line":""},{"line_number":8416,"context_line":"    def _get_all_assigned_mediated_devices(self, instance\u003dNone):"},{"line_number":8417,"context_line":"        \"\"\"Lookup all instances from the host and return all the mediated"}],"source_content_type":"text/x-python","patch_set":5,"id":"82d160cd_bc1ce913","line":8414,"range":{"start_line":8412,"start_character":8,"end_line":8414,"end_character":70},"updated":"2024-01-22 15:44:57.000000000","message":"functional style yeah ! ;)","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5d228eaaa57e3566885d03e318afa034b07b0438","unresolved":false,"context_lines":[{"line_number":8409,"context_line":"        :returns: dict where key is the mdev UUID and the value is its type."},{"line_number":8410,"context_line":"        \"\"\""},{"line_number":8411,"context_line":"        host_mdevs \u003d self._get_mediated_devices()"},{"line_number":8412,"context_line":"        inst_dev_infos \u003d filter(lambda dev: dev[\u0027uuid\u0027] in mdev_uuids,"},{"line_number":8413,"context_line":"                                host_mdevs)"},{"line_number":8414,"context_line":"        return {mdev[\u0027uuid\u0027]: mdev[\u0027type\u0027] for mdev in inst_dev_infos}"},{"line_number":8415,"context_line":""},{"line_number":8416,"context_line":"    def _get_all_assigned_mediated_devices(self, instance\u003dNone):"},{"line_number":8417,"context_line":"        \"\"\"Lookup all instances from the host and return all the mediated"}],"source_content_type":"text/x-python","patch_set":5,"id":"a94f5674_518ae01d","line":8414,"range":{"start_line":8412,"start_character":8,"end_line":8414,"end_character":70},"in_reply_to":"82d160cd_bc1ce913","updated":"2024-01-25 11:19:24.000000000","message":"Acknowledged","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":true,"context_lines":[{"line_number":9991,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":9992,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":9993,"context_line":""},{"line_number":9994,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9995,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9996,"context_line":"        ):"},{"line_number":9997,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9998,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"},{"line_number":9999,"context_line":"                        \u0027service are too old than the supported ones : \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5bd355b9_fe891f67","line":9996,"range":{"start_line":9994,"start_character":8,"end_line":9996,"end_character":9},"updated":"2024-01-22 15:44:57.000000000","message":"nit, I would write it:\n\nif not (\u0027dst_supports_mdev_live_migration\u0027 in dest_check_data and\n            dest_check_data.dst_supports_mdev_live_migration\n        ):\n        \nEasier to read IMHO.","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5d228eaaa57e3566885d03e318afa034b07b0438","unresolved":false,"context_lines":[{"line_number":9991,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":9992,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":9993,"context_line":""},{"line_number":9994,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9995,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9996,"context_line":"        ):"},{"line_number":9997,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9998,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"},{"line_number":9999,"context_line":"                        \u0027service are too old than the supported ones : \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"e02cb8f6_946033d3","line":9996,"range":{"start_line":9994,"start_character":8,"end_line":9996,"end_character":9},"in_reply_to":"5bd355b9_fe891f67","updated":"2024-01-25 11:19:24.000000000","message":"Done","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7c555b08dd8ce1759cb0956dac1cf98b51e075fc","unresolved":true,"context_lines":[{"line_number":9994,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9995,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9996,"context_line":"        ):"},{"line_number":9997,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9998,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"},{"line_number":9999,"context_line":"                        \u0027service are too old than the supported ones : \u0027"},{"line_number":10000,"context_line":"                        \u0027(QEMU: %(qemu_v)s, libvirt: %(libv_v)s)\u0027 %"},{"line_number":10001,"context_line":"                        {\u0027instance_uuid\u0027: instance.uuid,"},{"line_number":10002,"context_line":"                         \u0027qemu_v\u0027: libvirt_utils.version_to_string("},{"line_number":10003,"context_line":"                           MIN_MDEV_LIVEMIG_QEMU_VERSION),"},{"line_number":10004,"context_line":"                         \u0027libv_v\u0027: libvirt_utils.version_to_string("},{"line_number":10005,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":10006,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":10007,"context_line":""},{"line_number":10008,"context_line":"    def _is_shared_block_storage(self, instance, dest_check_data,"},{"line_number":10009,"context_line":"                                 block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"b892f7f8_f8e2c8d1","line":10006,"range":{"start_line":9997,"start_character":12,"end_line":10006,"end_character":65},"updated":"2024-01-22 15:44:57.000000000","message":"I would factorize as it is exactly the same msg as above.\nAnd this file is already too long.","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5d228eaaa57e3566885d03e318afa034b07b0438","unresolved":false,"context_lines":[{"line_number":9994,"context_line":"        if (\u0027dst_supports_mdev_live_migration\u0027 not in dest_check_data or"},{"line_number":9995,"context_line":"            not dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9996,"context_line":"        ):"},{"line_number":9997,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9998,"context_line":"                        \u0027Either libvirt or QEMU version of the target compute \u0027"},{"line_number":9999,"context_line":"                        \u0027service are too old than the supported ones : \u0027"},{"line_number":10000,"context_line":"                        \u0027(QEMU: %(qemu_v)s, libvirt: %(libv_v)s)\u0027 %"},{"line_number":10001,"context_line":"                        {\u0027instance_uuid\u0027: instance.uuid,"},{"line_number":10002,"context_line":"                         \u0027qemu_v\u0027: libvirt_utils.version_to_string("},{"line_number":10003,"context_line":"                           MIN_MDEV_LIVEMIG_QEMU_VERSION),"},{"line_number":10004,"context_line":"                         \u0027libv_v\u0027: libvirt_utils.version_to_string("},{"line_number":10005,"context_line":"                           MIN_MDEV_LIVEMIG_LIBVIRT_VERSION)}))"},{"line_number":10006,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":10007,"context_line":""},{"line_number":10008,"context_line":"    def _is_shared_block_storage(self, instance, dest_check_data,"},{"line_number":10009,"context_line":"                                 block_device_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"d9727b11_6b856e69","line":10006,"range":{"start_line":9997,"start_character":12,"end_line":10006,"end_character":65},"in_reply_to":"b892f7f8_f8e2c8d1","updated":"2024-01-25 11:19:24.000000000","message":"Done","commit_id":"a5f9c1d6bce3686d83ced084e787a7dc1bb53e14"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ab950d92be3cc04f5b1dacc581126e913cd1d407","unresolved":true,"context_lines":[{"line_number":9767,"context_line":"        # Just flag the fact we can live-migrate mdevs even if we don\u0027t use"},{"line_number":9768,"context_line":"        # them so the source will know we can use this compute."},{"line_number":9769,"context_line":"        if self._host_can_support_mdev_live_migration():"},{"line_number":9770,"context_line":"            data.dst_supports_mdev_live_migration \u003d True"},{"line_number":9771,"context_line":""},{"line_number":9772,"context_line":"        return data"},{"line_number":9773,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"afa7c819_a17fc0ea","line":9770,"updated":"2024-01-30 14:45:15.000000000","message":"Okay, I guess you\u0027re setting these flags based on libvirt support as well, so maybe service version isn\u0027t enough. Just seems like we could end up with lots of these flags in that object. It also means we\u0027d be implying a min libvirt version by bumping that object to 2.0, which while probably reasonable, is somewhat mixing domains.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8bca6b8fa08b0b43af7aff462aa111c2535db0","unresolved":false,"context_lines":[{"line_number":9767,"context_line":"        # Just flag the fact we can live-migrate mdevs even if we don\u0027t use"},{"line_number":9768,"context_line":"        # them so the source will know we can use this compute."},{"line_number":9769,"context_line":"        if self._host_can_support_mdev_live_migration():"},{"line_number":9770,"context_line":"            data.dst_supports_mdev_live_migration \u003d True"},{"line_number":9771,"context_line":""},{"line_number":9772,"context_line":"        return data"},{"line_number":9773,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"f47a584a_0d8cce5b","line":9770,"in_reply_to":"4d8a0f90_3c7dc9c2","updated":"2024-02-02 14:58:42.000000000","message":"Yeah it just becomes a bit of a hidden signal when we start doing that. Sending the actual version numbers may have been better in the long run for all these features, but that ship has sailed.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8de58ef7ad03a0cc04c2bfed5d40aac8d1dd3ace","unresolved":true,"context_lines":[{"line_number":9767,"context_line":"        # Just flag the fact we can live-migrate mdevs even if we don\u0027t use"},{"line_number":9768,"context_line":"        # them so the source will know we can use this compute."},{"line_number":9769,"context_line":"        if self._host_can_support_mdev_live_migration():"},{"line_number":9770,"context_line":"            data.dst_supports_mdev_live_migration \u003d True"},{"line_number":9771,"context_line":""},{"line_number":9772,"context_line":"        return data"},{"line_number":9773,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4d8a0f90_3c7dc9c2","line":9770,"in_reply_to":"afa7c819_a17fc0ea","updated":"2024-01-30 18:17:51.000000000","message":"Yeah, the support envelope is not only \u0027is this compute newer or equal than Caracal ?\u0027 but rather \u0027does this compute support all the needed dependencies for a mdev live-migration ?\u0027, hence why we need it.\n\nThe fact that we could remove this field by a new 2.0 major version is because I think the minimum libvirt/qemu versions for the time when we create a major version would be supporting this feature.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ab950d92be3cc04f5b1dacc581126e913cd1d407","unresolved":true,"context_lines":[{"line_number":9970,"context_line":"            hv_type\u003dhost.HV_DRIVER_QEMU,"},{"line_number":9971,"context_line":"        )"},{"line_number":9972,"context_line":""},{"line_number":9973,"context_line":"    def _assert_source_can_live_migrate_mdevs(self, instance, dest_check_data):"},{"line_number":9974,"context_line":"        \"\"\"Check if the source can live migrate the instance by looking at the"},{"line_number":9975,"context_line":"        QEMU and libvirt versions but also at the destination object."},{"line_number":9976,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"2bad91f2_cdd1504c","line":9973,"updated":"2024-01-30 14:45:15.000000000","message":"Seems like maybe it would be good to have a unit test for this on its own, instead of just testing this as part of the larger ops. Not sure you\u0027re catching all three potential cases in the first `if..else(a and b)` with the composite tests.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8de58ef7ad03a0cc04c2bfed5d40aac8d1dd3ace","unresolved":false,"context_lines":[{"line_number":9970,"context_line":"            hv_type\u003dhost.HV_DRIVER_QEMU,"},{"line_number":9971,"context_line":"        )"},{"line_number":9972,"context_line":""},{"line_number":9973,"context_line":"    def _assert_source_can_live_migrate_mdevs(self, instance, dest_check_data):"},{"line_number":9974,"context_line":"        \"\"\"Check if the source can live migrate the instance by looking at the"},{"line_number":9975,"context_line":"        QEMU and libvirt versions but also at the destination object."},{"line_number":9976,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5974d476_3a869e84","line":9973,"in_reply_to":"2bad91f2_cdd1504c","updated":"2024-01-30 18:17:51.000000000","message":"Surely I can add specific UTs yeah but fwiw I was already testing both the src support and the dest support  with https://review.opendev.org/c/openstack/nova/+/904176/6/nova/tests/unit/virt/libvirt/test_driver.py#12010 and the below test.\n\nWill add specific tests.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ab950d92be3cc04f5b1dacc581126e913cd1d407","unresolved":true,"context_lines":[{"line_number":9986,"context_line":"        elif not ("},{"line_number":9987,"context_line":"            \u0027dst_supports_mdev_live_migration\u0027 in dest_check_data and"},{"line_number":9988,"context_line":"            dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9989,"context_line":"        ):"},{"line_number":9990,"context_line":"            failed \u003d \u0027target\u0027"},{"line_number":9991,"context_line":"        if failed:"},{"line_number":9992,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"184eb9ab_7335e361","line":9989,"updated":"2024-01-30 14:45:15.000000000","message":"Two wasted vertical lines here? Seems like it should fit.","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8de58ef7ad03a0cc04c2bfed5d40aac8d1dd3ace","unresolved":false,"context_lines":[{"line_number":9986,"context_line":"        elif not ("},{"line_number":9987,"context_line":"            \u0027dst_supports_mdev_live_migration\u0027 in dest_check_data and"},{"line_number":9988,"context_line":"            dest_check_data.dst_supports_mdev_live_migration"},{"line_number":9989,"context_line":"        ):"},{"line_number":9990,"context_line":"            failed \u003d \u0027target\u0027"},{"line_number":9991,"context_line":"        if failed:"},{"line_number":9992,"context_line":"            reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"0ef0ce30_66c96145","line":9989,"in_reply_to":"184eb9ab_7335e361","updated":"2024-01-30 18:17:51.000000000","message":"Done","commit_id":"dd534f44b55b9df5bd07d845e2aaacaaa2f5dc4f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3b8bca6b8fa08b0b43af7aff462aa111c2535db0","unresolved":true,"context_lines":[{"line_number":9967,"context_line":"            lv_ver\u003dMIN_MDEV_LIVEMIG_LIBVIRT_VERSION,"},{"line_number":9968,"context_line":"            hv_ver\u003dMIN_MDEV_LIVEMIG_QEMU_VERSION,"},{"line_number":9969,"context_line":"            hv_type\u003dhost.HV_DRIVER_QEMU,"},{"line_number":9970,"context_line":"        )"},{"line_number":9971,"context_line":""},{"line_number":9972,"context_line":"    def _assert_source_can_live_migrate_mdevs(self, instance, dest_check_data):"},{"line_number":9973,"context_line":"        \"\"\"Check if the source can live migrate the instance by looking at the"}],"source_content_type":"text/x-python","patch_set":7,"id":"cbcd5b72_d710f492","line":9970,"updated":"2024-02-02 14:58:42.000000000","message":":(","commit_id":"baa78326ddc3d120370134663afcf50cda607d56"}]}
