)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6924c8504e52ad51cdc65bc01ab651492d820f15","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"370d38d8_6e59c333","updated":"2024-01-23 10:44:35.000000000","message":"Looks good to me, maybe just a variable to rename.","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"47dc2c3b9effdaad135822e4a3e5a4fa3c287ed6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"dbcb0774_6ffcece0","updated":"2024-01-19 09:16:41.000000000","message":"recheck again wrong image_ref for bfv instance","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"94db7a57cd5b4132d1d9bbdb6a68241840906da6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"4b50b049_2d95dc29","updated":"2024-01-17 23:18:16.000000000","message":"recheck rebuild bfv instance incorrect image_ref","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"8ebe689f_803ec249","updated":"2024-02-01 17:11:57.000000000","message":"Don\u0027t get me wrong, I\u0027ll provide a new revision for that patch because of the comment nitty modification (\"keyed by instance UUID\") but feel free to mark the new revision with a -1 if you do feel I\u0027m missing something.","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4b59be2a_8fd66761","updated":"2024-01-30 15:04:10.000000000","message":"I\u0027m going to call this a -1 because I don\u0027t think there is test coverage that proves my concern about allocating in `pre_check` doesn\u0027t leak.","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d9c3d00dca89de63c83c3681ffee1242a65ac0a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"10ce87b4_76b822c1","updated":"2024-01-29 13:27:23.000000000","message":"recheck fscking volume-related timeouts","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6df6bade954b70c503a8c95f3133d79b3b6b0d08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e3b93094_f25fcff0","updated":"2024-01-26 14:59:35.000000000","message":"recheck yay guest kernel panic is back","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"89e22e93_d3a35fd9","updated":"2024-02-02 10:10:38.000000000","message":"Damn, I was looking at the PS8...","commit_id":"82cfddc2c75390ddc8f542533f1515365fbdd498"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"42a2a65283ac9445ef68a28c7bb5072e974ecd2c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"abd172d6_a9c17db6","updated":"2024-02-02 09:51:15.000000000","message":"Doh stupid squash...","commit_id":"82cfddc2c75390ddc8f542533f1515365fbdd498"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a3858b19fc72725cf3cd33d5142e5d4082a62d19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"6f5ac28b_bf542aa3","updated":"2024-02-06 08:23:32.000000000","message":"recheck post failure again on slow ovs hybrid plug job","commit_id":"7c2041019678d8662459098a10b50eb83d5f0a41"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"83097f243ae0e3ed8c9c92a50583322294892add","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"16075a03_4fb1fa99","updated":"2024-02-05 17:49:31.000000000","message":"recheck post-failure","commit_id":"7c2041019678d8662459098a10b50eb83d5f0a41"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d69d7d087e267b857e5d0b1219424fc835240bca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"1fa7ae72_070acfcb","updated":"2024-02-16 08:57:15.000000000","message":"As I said on IRC, I\u0027ll also provide another top change for a periodic change verifying the dict.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4c7053ac8516b2607312ce61104a31ba5c93f628","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"34bcf366_4ac2bbd0","updated":"2024-02-15 17:33:15.000000000","message":"Waiting for a reply in IRC, but I thought I\u0027d commit this nit.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4c7053ac8516b2607312ce61104a31ba5c93f628","unresolved":true,"context_lines":[{"line_number":9211,"context_line":"                        has_vpmem \u003d True"},{"line_number":9212,"context_line":"                        break"},{"line_number":9213,"context_line":"            has_mdevs \u003d (True if migrate_data.obj_attr_is_set(\u0027target_mdevs\u0027)"},{"line_number":9214,"context_line":"                         else False)"},{"line_number":9215,"context_line":"            # No instance booting at source host, but instance dir"},{"line_number":9216,"context_line":"            # must be deleted for preparing next block migration"},{"line_number":9217,"context_line":"            # must be deleted for preparing next live migration w/o shared"}],"source_content_type":"text/x-python","patch_set":15,"id":"fb58f9ab_a9a4140f","line":9214,"updated":"2024-02-15 17:33:15.000000000","message":"I think this can just be:\n```\nhas_mdevs \u003d \u0027target_mdevs\u0027 in migrate_data\n```\n\nThe \"true if true else false\" statement here made me read it several times to see if you were inverting the logic or something.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"72ed9936d737d3302b0c8c245db416451f022fbf","unresolved":false,"context_lines":[{"line_number":9211,"context_line":"                        has_vpmem \u003d True"},{"line_number":9212,"context_line":"                        break"},{"line_number":9213,"context_line":"            has_mdevs \u003d (True if migrate_data.obj_attr_is_set(\u0027target_mdevs\u0027)"},{"line_number":9214,"context_line":"                         else False)"},{"line_number":9215,"context_line":"            # No instance booting at source host, but instance dir"},{"line_number":9216,"context_line":"            # must be deleted for preparing next block migration"},{"line_number":9217,"context_line":"            # must be deleted for preparing next live migration w/o shared"}],"source_content_type":"text/x-python","patch_set":15,"id":"53b4252c_2e94d428","line":9214,"in_reply_to":"4e03ac02_8ea0443d","updated":"2024-02-16 09:57:04.000000000","message":"Done","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d69d7d087e267b857e5d0b1219424fc835240bca","unresolved":true,"context_lines":[{"line_number":9211,"context_line":"                        has_vpmem \u003d True"},{"line_number":9212,"context_line":"                        break"},{"line_number":9213,"context_line":"            has_mdevs \u003d (True if migrate_data.obj_attr_is_set(\u0027target_mdevs\u0027)"},{"line_number":9214,"context_line":"                         else False)"},{"line_number":9215,"context_line":"            # No instance booting at source host, but instance dir"},{"line_number":9216,"context_line":"            # must be deleted for preparing next block migration"},{"line_number":9217,"context_line":"            # must be deleted for preparing next live migration w/o shared"}],"source_content_type":"text/x-python","patch_set":15,"id":"4e03ac02_8ea0443d","line":9214,"in_reply_to":"fb58f9ab_a9a4140f","updated":"2024-02-16 08:57:15.000000000","message":"You\u0027re right, I can fix it in a FUP.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"72ed9936d737d3302b0c8c245db416451f022fbf","unresolved":false,"context_lines":[{"line_number":11295,"context_line":"                revert \u003d (True if migration.source_compute \u003d\u003d CONF.host"},{"line_number":11296,"context_line":"                          else False)"},{"line_number":11297,"context_line":"                with instance.mutated_migration_context(revert\u003drevert):"},{"line_number":11298,"context_line":"                    self.driver.cleanup_lingering_instance_resources(instance)"},{"line_number":11299,"context_line":""},{"line_number":11300,"context_line":"                try:"},{"line_number":11301,"context_line":"                    migration.status \u003d \u0027failed\u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"03bffc6c_7afd0ec0","line":11298,"updated":"2024-02-16 09:57:04.000000000","message":"fwiw, this periodic checks a race condition, so I\u0027ll also add a check for the dicin the cleanup_lingering_i_r() method.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a17cd114b363124c6d8739e4824f5e7b479f708f","unresolved":false,"context_lines":[{"line_number":13489,"context_line":"            instance_relative_path\u003dFalse)"},{"line_number":13490,"context_line":"        self.assertRaises(exception.Invalid,"},{"line_number":13491,"context_line":"                          drvr.rollback_live_migration_at_destination,"},{"line_number":13492,"context_line":"                          \"context\", \"instance\", [], None, True, migrate_data)"},{"line_number":13493,"context_line":"        mock_exist.assert_called_once_with(fake_instance_path)"},{"line_number":13494,"context_line":"        mock_shutil.assert_called_once_with(fake_instance_path)"},{"line_number":13495,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3037616a_bf52d2f0","line":13492,"range":{"start_line":13492,"start_character":37,"end_line":13492,"end_character":47},"updated":"2024-01-17 08:32:39.000000000","message":"doh, fixed.","commit_id":"5d7aed0c119b15bf1ad1df62a99114578421dd15"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":true,"context_lines":[{"line_number":13529,"context_line":"        self.assertFalse(mock_shutil.called)"},{"line_number":13530,"context_line":"        # Assert we delete existing claimed mdevs for the instance"},{"line_number":13531,"context_line":"        self.assertEqual({uuids.other_inst: [uuids.mdev2]},"},{"line_number":13532,"context_line":"                         drvr.instance_claimed_mdevs)"},{"line_number":13533,"context_line":""},{"line_number":13534,"context_line":"    @mock.patch.object(fakelibvirt.Domain, \"XMLDesc\")"},{"line_number":13535,"context_line":"    def test_live_migration_copy_disk_paths_tunnelled(self, mock_xml):"}],"source_content_type":"text/x-python","patch_set":8,"id":"a979dbd9_912e1098","line":13532,"updated":"2024-02-01 17:11:57.000000000","message":"Dan, that\u0027s in this UT that we double-check that we delete the cached mdev if we rollback. Would you prefer some functional test that would ensure that the mdev is freed ? (not sure it\u0027s easy to write but I can try).\n\nI can also try to think hard about all the other corner cases where a migration can fail, but as I said, it looks like everything ends up into that libvirt method.\nhttps://specs.openstack.org/openstack/nova-specs/specs/2024.1/approved/libvirt-mdev-live-migrate.html#proposed-change (last paragraph)","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fb675cea209f587d606a8ee3b55cab2714121c31","unresolved":false,"context_lines":[{"line_number":13529,"context_line":"        self.assertFalse(mock_shutil.called)"},{"line_number":13530,"context_line":"        # Assert we delete existing claimed mdevs for the instance"},{"line_number":13531,"context_line":"        self.assertEqual({uuids.other_inst: [uuids.mdev2]},"},{"line_number":13532,"context_line":"                         drvr.instance_claimed_mdevs)"},{"line_number":13533,"context_line":""},{"line_number":13534,"context_line":"    @mock.patch.object(fakelibvirt.Domain, \"XMLDesc\")"},{"line_number":13535,"context_line":"    def test_live_migration_copy_disk_paths_tunnelled(self, mock_xml):"}],"source_content_type":"text/x-python","patch_set":8,"id":"be4c809d_bd1bb7ed","line":13532,"in_reply_to":"a979dbd9_912e1098","updated":"2024-02-01 17:50:43.000000000","message":"Should be addressed now with https://review.opendev.org/c/openstack/nova/+/904209/12/nova/tests/functional/libvirt/test_vgpu.py","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98920d182c285f014c897a27c55bd0dd2681df5d","unresolved":true,"context_lines":[{"line_number":13521,"context_line":""},{"line_number":13522,"context_line":"        mock_destroy.side_effect \u003d fake_destroy"},{"line_number":13523,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":13524,"context_line":"        self.assertEqual({}, drvr.instance_claimed_mdevs)"},{"line_number":13525,"context_line":""},{"line_number":13526,"context_line":"        instance \u003d objects.Instance(id\u003d1, uuid\u003duuids.instance)"},{"line_number":13527,"context_line":"        migrate_data \u003d objects.LibvirtLiveMigrateData("}],"source_content_type":"text/x-python","patch_set":15,"id":"f3d72951_df33f3a0","line":13524,"updated":"2024-02-14 12:08:20.000000000","message":"this is showing we have not claimed any devices before the migration which is fine.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98920d182c285f014c897a27c55bd0dd2681df5d","unresolved":true,"context_lines":[{"line_number":13529,"context_line":"            instance_relative_path\u003dFalse)"},{"line_number":13530,"context_line":"        drvr.instance_claimed_mdevs \u003d {instance.uuid: [uuids.mdev1],"},{"line_number":13531,"context_line":"                                       uuids.other_inst: [uuids.mdev2]}"},{"line_number":13532,"context_line":"        drvr.rollback_live_migration_at_destination(\"context\", instance, [],"},{"line_number":13533,"context_line":"                                                    None, True, migrate_data)"},{"line_number":13534,"context_line":"        mock_destroy.assert_called_once_with(\"context\", instance, [],"},{"line_number":13535,"context_line":"                                             None, True)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b68eb674_2ca4349b","line":13532,"updated":"2024-02-14 12:08:20.000000000","message":"then you fake the resevations of this insntance and another instnace","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98920d182c285f014c897a27c55bd0dd2681df5d","unresolved":true,"context_lines":[{"line_number":13539,"context_line":"        # Assert we delete existing claimed mdevs for the instance"},{"line_number":13540,"context_line":"        self.assertEqual({uuids.other_inst: [uuids.mdev2]},"},{"line_number":13541,"context_line":"                         drvr.instance_claimed_mdevs)"},{"line_number":13542,"context_line":""},{"line_number":13543,"context_line":"    @mock.patch.object(fakelibvirt.Domain, \"XMLDesc\")"},{"line_number":13544,"context_line":"    def test_live_migration_copy_disk_paths_tunnelled(self, mock_xml):"},{"line_number":13545,"context_line":"        self.flags(live_migration_tunnelled\u003dTrue, group\u003d\u0027libvirt\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"f7d04814_d0effa43","line":13542,"updated":"2024-02-14 12:08:20.000000000","message":"and finally show that only the claimis for the rolled back instance are cleanned up.\n\ni think this covers the leak that dan was concerned about on roolback.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98920d182c285f014c897a27c55bd0dd2681df5d","unresolved":true,"context_lines":[{"line_number":20346,"context_line":"        # Assert we delete existing claimed mdevs for the instance"},{"line_number":20347,"context_line":"        self.assertEqual({uuids.other_inst: [uuids.mdev2]},"},{"line_number":20348,"context_line":"                         drvr.instance_claimed_mdevs)"},{"line_number":20349,"context_line":"        vif \u003d network_model.VIF(id\u003duuids.port_id,"},{"line_number":20350,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_NORMAL)"},{"line_number":20351,"context_line":"        vif_direct \u003d network_model.VIF(id\u003duuids.port_id,"},{"line_number":20352,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fab7261_a9be549e","line":20349,"updated":"2024-02-14 12:08:20.000000000","message":"you also show that we clean this up in post migrate covering the happy path.","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c8d350a34265819f6ce9b7ebed1bdfe89c548c8b","unresolved":false,"context_lines":[{"line_number":20346,"context_line":"        # Assert we delete existing claimed mdevs for the instance"},{"line_number":20347,"context_line":"        self.assertEqual({uuids.other_inst: [uuids.mdev2]},"},{"line_number":20348,"context_line":"                         drvr.instance_claimed_mdevs)"},{"line_number":20349,"context_line":"        vif \u003d network_model.VIF(id\u003duuids.port_id,"},{"line_number":20350,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_NORMAL)"},{"line_number":20351,"context_line":"        vif_direct \u003d network_model.VIF(id\u003duuids.port_id,"},{"line_number":20352,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT)"}],"source_content_type":"text/x-python","patch_set":15,"id":"bcf04b94_3ef911b5","line":20349,"in_reply_to":"3fab7261_a9be549e","updated":"2024-02-20 12:13:00.000000000","message":"Acknowledged","commit_id":"1aac0ee0318ac3815aee889f22eadaf37bf1db2f"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6924c8504e52ad51cdc65bc01ab651492d820f15","unresolved":false,"context_lines":[{"line_number":9810,"context_line":"                raise exception.MigrationPreCheckError(reason)"},{"line_number":9811,"context_line":"            dst_mdevs \u003d self._allocate_mdevs(allocs)"},{"line_number":9812,"context_line":"            dst_mdev_types \u003d self._get_mdev_types_from_uuids(dst_mdevs)"},{"line_number":9813,"context_line":"            target_mdevs \u003d {}"},{"line_number":9814,"context_line":"            for src_mdev, src_type in src_mdev_types.items():"},{"line_number":9815,"context_line":"                for dst_mdev, dst_type in dst_mdev_types.items():"},{"line_number":9816,"context_line":"                    # we want to associate by 1:1 between dst and src mdevs"}],"source_content_type":"text/x-python","patch_set":5,"id":"a998ac86_64f3b6de","line":9813,"in_reply_to":"530ba8e1_76048ce1","updated":"2024-01-23 10:44:35.000000000","message":"😊","commit_id":"5d7aed0c119b15bf1ad1df62a99114578421dd15"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"02ae46fb112bad4b17c0cbf0c44d9242181c0fd3","unresolved":false,"context_lines":[{"line_number":9810,"context_line":"                raise exception.MigrationPreCheckError(reason)"},{"line_number":9811,"context_line":"            dst_mdevs \u003d self._allocate_mdevs(allocs)"},{"line_number":9812,"context_line":"            dst_mdev_types \u003d self._get_mdev_types_from_uuids(dst_mdevs)"},{"line_number":9813,"context_line":"            target_mdevs \u003d {}"},{"line_number":9814,"context_line":"            for src_mdev, src_type in src_mdev_types.items():"},{"line_number":9815,"context_line":"                for dst_mdev, dst_type in dst_mdev_types.items():"},{"line_number":9816,"context_line":"                    # we want to associate by 1:1 between dst and src mdevs"}],"source_content_type":"text/x-python","patch_set":5,"id":"530ba8e1_76048ce1","line":9813,"in_reply_to":"6d64f844_f9c12544","updated":"2024-01-17 09:30:26.000000000","message":"That\u0027s one good example of how much I hate static typing for Python.","commit_id":"5d7aed0c119b15bf1ad1df62a99114578421dd15"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6924c8504e52ad51cdc65bc01ab651492d820f15","unresolved":true,"context_lines":[{"line_number":543,"context_line":"        # This dict is for knowing which mdevs are already claimed by some"},{"line_number":544,"context_line":"        # instance. This is keyed by instance UUIDs and the value is a list"},{"line_number":545,"context_line":"        # of mediated device UUIDs."},{"line_number":546,"context_line":"        self.instance_claimed_mdevs \u003d {}"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"        # Handles ongoing device manipultion in libvirt where we wait for the"},{"line_number":549,"context_line":"        # events about success or failure."}],"source_content_type":"text/x-python","patch_set":8,"id":"04c69168_d820b35b","line":546,"updated":"2024-01-23 10:44:35.000000000","message":"I would call it instance**s**_claimed_mdevs.\nbecause it is not the mdevs on 1 instance.","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"180cf30eb10c2b284803c0fbe48b88ed7e494036","unresolved":true,"context_lines":[{"line_number":543,"context_line":"        # This dict is for knowing which mdevs are already claimed by some"},{"line_number":544,"context_line":"        # instance. This is keyed by instance UUIDs and the value is a list"},{"line_number":545,"context_line":"        # of mediated device UUIDs."},{"line_number":546,"context_line":"        self.instance_claimed_mdevs \u003d {}"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"        # Handles ongoing device manipultion in libvirt where we wait for the"},{"line_number":549,"context_line":"        # events about success or failure."}],"source_content_type":"text/x-python","patch_set":8,"id":"e91116b6_481d6fde","line":546,"in_reply_to":"04c69168_d820b35b","updated":"2024-01-25 13:32:38.000000000","message":"Nop, I disagree: this is a 1:N relationship as I wrote in the comment.\nFor example, a key/value is \n\n`inst1: [mdev1, mdev2]`","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fb675cea209f587d606a8ee3b55cab2714121c31","unresolved":false,"context_lines":[{"line_number":543,"context_line":"        # This dict is for knowing which mdevs are already claimed by some"},{"line_number":544,"context_line":"        # instance. This is keyed by instance UUIDs and the value is a list"},{"line_number":545,"context_line":"        # of mediated device UUIDs."},{"line_number":546,"context_line":"        self.instance_claimed_mdevs \u003d {}"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"        # Handles ongoing device manipultion in libvirt where we wait for the"},{"line_number":549,"context_line":"        # events about success or failure."}],"source_content_type":"text/x-python","patch_set":8,"id":"1b418acd_58e1c9fb","line":546,"in_reply_to":"8d7a7d9e_50dff6ca","updated":"2024-02-01 17:50:43.000000000","message":"Should be addressed now with https://review.opendev.org/c/openstack/nova/+/904209/12/nova/tests/functional/libvirt/test_vgpu.py","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":true,"context_lines":[{"line_number":543,"context_line":"        # This dict is for knowing which mdevs are already claimed by some"},{"line_number":544,"context_line":"        # instance. This is keyed by instance UUIDs and the value is a list"},{"line_number":545,"context_line":"        # of mediated device UUIDs."},{"line_number":546,"context_line":"        self.instance_claimed_mdevs \u003d {}"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"        # Handles ongoing device manipultion in libvirt where we wait for the"},{"line_number":549,"context_line":"        # events about success or failure."}],"source_content_type":"text/x-python","patch_set":8,"id":"8d7a7d9e_50dff6ca","line":546,"in_reply_to":"8e753060_d9401040","updated":"2024-02-01 17:11:57.000000000","message":"If the migration does abort, we cleanup the cruft by the rollback method below https://review.opendev.org/c/openstack/nova/+/904209/8/nova/virt/libvirt/driver.py#10937\n\nI tried to cover the failing scenarios in my spec, all of them lead to the same virt driver rollback_live_migration_at_destination() method eventually.","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":true,"context_lines":[{"line_number":543,"context_line":"        # This dict is for knowing which mdevs are already claimed by some"},{"line_number":544,"context_line":"        # instance. This is keyed by instance UUIDs and the value is a list"},{"line_number":545,"context_line":"        # of mediated device UUIDs."},{"line_number":546,"context_line":"        self.instance_claimed_mdevs \u003d {}"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"        # Handles ongoing device manipultion in libvirt where we wait for the"},{"line_number":549,"context_line":"        # events about success or failure."}],"source_content_type":"text/x-python","patch_set":8,"id":"8e753060_d9401040","line":546,"in_reply_to":"e91116b6_481d6fde","updated":"2024-01-30 15:04:10.000000000","message":"Change the comment above to say \"keyed by instance UUID\" then so it matches. But I agree with Sylvain, yep.\n\nAlthough I will say that this seems like it\u0027s begging to become cluttered with cruft if you don\u0027t clean it up 100% after each migration. We set this in `post_check...` but, will we ever get called if the migration doesn\u0027t proceed?","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6924c8504e52ad51cdc65bc01ab651492d820f15","unresolved":true,"context_lines":[{"line_number":9822,"context_line":"                        dst_mdev not in target_mdevs.values()"},{"line_number":9823,"context_line":"                    ):"},{"line_number":9824,"context_line":"                        target_mdevs[src_mdev] \u003d dst_mdev"},{"line_number":9825,"context_line":"                        continue"},{"line_number":9826,"context_line":"            if len(target_mdevs) !\u003d len(src_mdev_types):"},{"line_number":9827,"context_line":"                reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9828,"context_line":"                            \u0027Source mdevs %(src_mdevs)s are not \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"4103296b_d86c157d","line":9825,"range":{"start_line":9825,"start_character":24,"end_line":9825,"end_character":32},"updated":"2024-01-23 10:44:35.000000000","message":"good practice ++","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"180cf30eb10c2b284803c0fbe48b88ed7e494036","unresolved":false,"context_lines":[{"line_number":9822,"context_line":"                        dst_mdev not in target_mdevs.values()"},{"line_number":9823,"context_line":"                    ):"},{"line_number":9824,"context_line":"                        target_mdevs[src_mdev] \u003d dst_mdev"},{"line_number":9825,"context_line":"                        continue"},{"line_number":9826,"context_line":"            if len(target_mdevs) !\u003d len(src_mdev_types):"},{"line_number":9827,"context_line":"                reason \u003d (_(\u0027Unable to migrate %(instance_uuid)s: \u0027"},{"line_number":9828,"context_line":"                            \u0027Source mdevs %(src_mdevs)s are not \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"3500a325_e4dd4534","line":9825,"range":{"start_line":9825,"start_character":24,"end_line":9825,"end_character":32},"in_reply_to":"4103296b_d86c157d","updated":"2024-01-25 13:32:38.000000000","message":"Acknowledged","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":false,"context_lines":[{"line_number":10934,"context_line":"                                if \"source_vif\" in vif and vif.source_vif])"},{"line_number":10935,"context_line":"            self._reattach_instance_vifs(context, instance, network_info)"},{"line_number":10936,"context_line":""},{"line_number":10937,"context_line":"    def rollback_live_migration_at_destination(self, context, instance,"},{"line_number":10938,"context_line":"                                               network_info,"},{"line_number":10939,"context_line":"                                               block_device_info,"},{"line_number":10940,"context_line":"                                               destroy_disks\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":8,"id":"0fe9031f_450828e2","line":10937,"updated":"2024-02-01 17:11:57.000000000","message":"there, this method is called every time we rollback a migration.","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":true,"context_lines":[{"line_number":10967,"context_line":"                LOG.debug(\"Unclaiming mdevs %s from instance %s\","},{"line_number":10968,"context_line":"                    self.instance_claimed_mdevs.get(instance.uuid),"},{"line_number":10969,"context_line":"                    instance.uuid)"},{"line_number":10970,"context_line":"                del self.instance_claimed_mdevs[instance.uuid]"},{"line_number":10971,"context_line":""},{"line_number":10972,"context_line":"    def _pre_live_migration_plug_vifs(self, instance, network_info,"},{"line_number":10973,"context_line":"                                      migrate_data):"}],"source_content_type":"text/x-python","patch_set":8,"id":"2630a45c_50df32d9","line":10970,"updated":"2024-02-01 17:11:57.000000000","message":"nit: one could say a single dict.pop(instance.uuid, None) would suffice (and they would be right) but I think this LOG message is worthy to provide.","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":10967,"context_line":"                LOG.debug(\"Unclaiming mdevs %s from instance %s\","},{"line_number":10968,"context_line":"                    self.instance_claimed_mdevs.get(instance.uuid),"},{"line_number":10969,"context_line":"                    instance.uuid)"},{"line_number":10970,"context_line":"                del self.instance_claimed_mdevs[instance.uuid]"},{"line_number":10971,"context_line":""},{"line_number":10972,"context_line":"    def _pre_live_migration_plug_vifs(self, instance, network_info,"},{"line_number":10973,"context_line":"                                      migrate_data):"}],"source_content_type":"text/x-python","patch_set":8,"id":"e8a29a63_af14042e","line":10970,"in_reply_to":"2630a45c_50df32d9","updated":"2024-02-02 10:10:38.000000000","message":"Done","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba4a40a5aa377687aa2884b9db86c3e3b02a2256","unresolved":true,"context_lines":[{"line_number":11356,"context_line":"            LOG.debug(\"Unclaiming mdevs %s from instance %s\","},{"line_number":11357,"context_line":"                self.instance_claimed_mdevs.get(instance.uuid),"},{"line_number":11358,"context_line":"                instance.uuid)"},{"line_number":11359,"context_line":"            del self.instance_claimed_mdevs[instance.uuid]"},{"line_number":11360,"context_line":""},{"line_number":11361,"context_line":"    def _get_instance_disk_info_from_config(self, guest_config,"},{"line_number":11362,"context_line":"                                            block_device_info):"}],"source_content_type":"text/x-python","patch_set":8,"id":"0e7121be_d119d957","line":11359,"updated":"2024-02-01 17:11:57.000000000","message":"ditto here about an alternative .pop()\n\n(this is just about what people prefer to read)","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":11356,"context_line":"            LOG.debug(\"Unclaiming mdevs %s from instance %s\","},{"line_number":11357,"context_line":"                self.instance_claimed_mdevs.get(instance.uuid),"},{"line_number":11358,"context_line":"                instance.uuid)"},{"line_number":11359,"context_line":"            del self.instance_claimed_mdevs[instance.uuid]"},{"line_number":11360,"context_line":""},{"line_number":11361,"context_line":"    def _get_instance_disk_info_from_config(self, guest_config,"},{"line_number":11362,"context_line":"                                            block_device_info):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ee41b5e5_81d2ef98","line":11359,"in_reply_to":"0e7121be_d119d957","updated":"2024-02-02 10:10:38.000000000","message":"Done","commit_id":"318c1fc013e04f2b2b7f368a75850fa8eb342955"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":true,"context_lines":[{"line_number":9814,"context_line":"                           \u0027src_types\u0027: list(src_mdev_types.values()),"},{"line_number":9815,"context_line":"                           \u0027dest_types\u0027: self.supported_vgpu_types}))"},{"line_number":9816,"context_line":"                raise exception.MigrationPreCheckError(reason)"},{"line_number":9817,"context_line":"            dst_mdevs \u003d self._allocate_mdevs(allocs)"},{"line_number":9818,"context_line":"            dst_mdev_types \u003d self._get_mdev_types_from_uuids(dst_mdevs)"},{"line_number":9819,"context_line":"            target_mdevs: ty.Dict[str, str] \u003d {}"},{"line_number":9820,"context_line":"            for src_mdev, src_type in src_mdev_types.items():"}],"source_content_type":"text/x-python","patch_set":10,"id":"d9793b66_a2a91748","line":9817,"updated":"2024-01-30 15:04:10.000000000","message":"Hmm, we\u0027re allocating in `post_check...` which seems like an odd place to reserve something, in case the migration doesn\u0027t proceed. Are we sure we\u0027ll get called to clean this up if the migration doesn\u0027t happen? All these pre/post check/migrate/complete things are hard to track...","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":9814,"context_line":"                           \u0027src_types\u0027: list(src_mdev_types.values()),"},{"line_number":9815,"context_line":"                           \u0027dest_types\u0027: self.supported_vgpu_types}))"},{"line_number":9816,"context_line":"                raise exception.MigrationPreCheckError(reason)"},{"line_number":9817,"context_line":"            dst_mdevs \u003d self._allocate_mdevs(allocs)"},{"line_number":9818,"context_line":"            dst_mdev_types \u003d self._get_mdev_types_from_uuids(dst_mdevs)"},{"line_number":9819,"context_line":"            target_mdevs: ty.Dict[str, str] \u003d {}"},{"line_number":9820,"context_line":"            for src_mdev, src_type in src_mdev_types.items():"}],"source_content_type":"text/x-python","patch_set":10,"id":"a4202274_0afd78b3","line":9817,"in_reply_to":"d9793b66_a2a91748","updated":"2024-02-02 10:10:38.000000000","message":"Dunno why I wasn\u0027t able to see your comment (probably due to looking at PS8), but I think I can resolve this comment given now I provide a functional test for checking that.","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":true,"context_lines":[{"line_number":9823,"context_line":"                    if (src_type \u003d\u003d dst_type and"},{"line_number":9824,"context_line":"                        src_type not in target_mdevs and"},{"line_number":9825,"context_line":"                        dst_mdev not in target_mdevs.values()"},{"line_number":9826,"context_line":"                    ):"},{"line_number":9827,"context_line":"                        target_mdevs[src_mdev] \u003d dst_mdev"},{"line_number":9828,"context_line":"                        continue"},{"line_number":9829,"context_line":"            if len(target_mdevs) !\u003d len(src_mdev_types):"}],"source_content_type":"text/x-python","patch_set":10,"id":"af4d6d1a_4a9fe74a","line":9826,"updated":"2024-01-30 15:04:10.000000000","message":"Why waste the line? :(","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":9823,"context_line":"                    if (src_type \u003d\u003d dst_type and"},{"line_number":9824,"context_line":"                        src_type not in target_mdevs and"},{"line_number":9825,"context_line":"                        dst_mdev not in target_mdevs.values()"},{"line_number":9826,"context_line":"                    ):"},{"line_number":9827,"context_line":"                        target_mdevs[src_mdev] \u003d dst_mdev"},{"line_number":9828,"context_line":"                        continue"},{"line_number":9829,"context_line":"            if len(target_mdevs) !\u003d len(src_mdev_types):"}],"source_content_type":"text/x-python","patch_set":10,"id":"2189c239_b1d2ea03","line":9826,"in_reply_to":"af4d6d1a_4a9fe74a","updated":"2024-02-02 10:10:38.000000000","message":"Done","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":true,"context_lines":[{"line_number":10960,"context_line":"                    instance, migrate_data)"},{"line_number":10961,"context_line":"                if os.path.exists(instance_dir):"},{"line_number":10962,"context_line":"                    shutil.rmtree(instance_dir)"},{"line_number":10963,"context_line":"            if self.instance_claimed_mdevs.get(instance.uuid):"},{"line_number":10964,"context_line":"                # The live migration is aborted, we need to remove the reserved"},{"line_number":10965,"context_line":"                # values."},{"line_number":10966,"context_line":"                LOG.debug(\"Unclaiming mdevs %s from instance %s\","}],"source_content_type":"text/x-python","patch_set":10,"id":"14e613d9_5c85859f","line":10963,"updated":"2024-01-30 15:04:10.000000000","message":"Use `pop(instance.uuid, None)` here. Same behavior, but you don\u0027t have to worry about the `del` below and/or crashing on something before you clean up.","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":10960,"context_line":"                    instance, migrate_data)"},{"line_number":10961,"context_line":"                if os.path.exists(instance_dir):"},{"line_number":10962,"context_line":"                    shutil.rmtree(instance_dir)"},{"line_number":10963,"context_line":"            if self.instance_claimed_mdevs.get(instance.uuid):"},{"line_number":10964,"context_line":"                # The live migration is aborted, we need to remove the reserved"},{"line_number":10965,"context_line":"                # values."},{"line_number":10966,"context_line":"                LOG.debug(\"Unclaiming mdevs %s from instance %s\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9974cccb_137bb18c","line":10963,"in_reply_to":"14e613d9_5c85859f","updated":"2024-02-02 10:10:38.000000000","message":"Done","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73ab3458ab2f46da4741e1724902fe874d754d0b","unresolved":true,"context_lines":[{"line_number":11349,"context_line":"        \"\"\""},{"line_number":11350,"context_line":"        self._reattach_instance_vifs(context, instance, network_info)"},{"line_number":11351,"context_line":"        self._qemu_monitor_announce_self(instance)"},{"line_number":11352,"context_line":"        if self.instance_claimed_mdevs.get(instance.uuid):"},{"line_number":11353,"context_line":"            # The live migration is done, the related mdevs are now associated"},{"line_number":11354,"context_line":"            # to the domain XML so we can remove the reserved values."},{"line_number":11355,"context_line":"            LOG.debug(\"Unclaiming mdevs %s from instance %s\","}],"source_content_type":"text/x-python","patch_set":10,"id":"e345eb64_05c68c74","line":11352,"updated":"2024-01-30 15:04:10.000000000","message":"same.","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1a0a66533fb02b47e06ea9d9b93fcb40d510ad43","unresolved":false,"context_lines":[{"line_number":11349,"context_line":"        \"\"\""},{"line_number":11350,"context_line":"        self._reattach_instance_vifs(context, instance, network_info)"},{"line_number":11351,"context_line":"        self._qemu_monitor_announce_self(instance)"},{"line_number":11352,"context_line":"        if self.instance_claimed_mdevs.get(instance.uuid):"},{"line_number":11353,"context_line":"            # The live migration is done, the related mdevs are now associated"},{"line_number":11354,"context_line":"            # to the domain XML so we can remove the reserved values."},{"line_number":11355,"context_line":"            LOG.debug(\"Unclaiming mdevs %s from instance %s\","}],"source_content_type":"text/x-python","patch_set":10,"id":"cd182dc8_44aff291","line":11352,"in_reply_to":"e345eb64_05c68c74","updated":"2024-02-02 10:10:38.000000000","message":"Done","commit_id":"dae2853746b644c4b8aeb5ad52d37601b373d88f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f76c40049b4ee0d3cbd0b8d3d3ed264315f97197","unresolved":true,"context_lines":[{"line_number":9835,"context_line":"                      \u0027existing mediated devices \u0027"},{"line_number":9836,"context_line":"                      \u0027(source uuid : dest uuid): %s\u0027, str(target_mdevs))"},{"line_number":9837,"context_line":"            migrate_data.target_mdevs \u003d target_mdevs"},{"line_number":9838,"context_line":"            self.instance_claimed_mdevs[instance.uuid] \u003d dst_mdevs"},{"line_number":9839,"context_line":"        return migrate_data"},{"line_number":9840,"context_line":""},{"line_number":9841,"context_line":"    def post_claim_migrate_data(self, context, instance, migrate_data, claim):"}],"source_content_type":"text/x-python","patch_set":14,"id":"07a4cff4_2e46921b","line":9838,"updated":"2024-02-07 15:45:35.000000000","message":"So what I was saying before was this seems a little bit dangerous. We\u0027re persisting state in the libvirt driver that we don\u0027t periodically cleanse, or log, or expose anywhere that someone can see. If live migration fails because we have a rabbit outage or something like that and are unable to call the rollback immediately, we\u0027ll leak here. The user will see this as the compute refusing to allocate mdevs that they think should be available. They don\u0027t see any other indication that something is wrong, so they \"just restart it\" and things magically start working again. That kind of experience is quite frustrating.\n\n...later... Actually, I guess this leaves residue in placement as well. If that happens, what does recovery look like? If they retry migration, will we re-allocate more in placement and leak them there? It looks like we won\u0027t find and re-use the allocations/reservations here if the migration is retried.\n\nI\u0027m not really sure what to propose here, and I know we\u0027re doing similar things for other types of resources, but it kinda seems like we should do better for something as important as these. Perhaps log something each time we go to do this saying \"and just FYI, I\u0027m still holding %i mdevs for instance %s\" ?","commit_id":"7c2041019678d8662459098a10b50eb83d5f0a41"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5770dce26f68c6187b69a3ab66f36bbb3ed97aee","unresolved":false,"context_lines":[{"line_number":9835,"context_line":"                      \u0027existing mediated devices \u0027"},{"line_number":9836,"context_line":"                      \u0027(source uuid : dest uuid): %s\u0027, str(target_mdevs))"},{"line_number":9837,"context_line":"            migrate_data.target_mdevs \u003d target_mdevs"},{"line_number":9838,"context_line":"            self.instance_claimed_mdevs[instance.uuid] \u003d dst_mdevs"},{"line_number":9839,"context_line":"        return migrate_data"},{"line_number":9840,"context_line":""},{"line_number":9841,"context_line":"    def post_claim_migrate_data(self, context, instance, migrate_data, claim):"}],"source_content_type":"text/x-python","patch_set":14,"id":"46c7947c_b64a884e","line":9838,"in_reply_to":"07a4cff4_2e46921b","updated":"2024-02-08 14:40:48.000000000","message":"As discussed yesterday on IRC [1] : \n- I\u0027ll add a log for operators getting the list of reserved mdevs\n- I\u0027ll try to add some periodic (by a FUP) for making sure we can cleanup the dict in case of some rabbit outage\n\n[1] https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2024-02-07.log.html#t2024-02-07T18:28:34","commit_id":"7c2041019678d8662459098a10b50eb83d5f0a41"}]}