)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"fc1e8382612f179718c6ca595f1a9b5e4ca126e3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"29635d3a_5ce65811","updated":"2023-03-28 13:13:25.000000000","message":"Hello, I have been assigned to a bug ticket that is closely related to what this change aims to fix. If it is not a problem, I am going to pick this up and continue progress on it. \n\n@smooney Could you please review Tyler\u0027s comments so that we could finish defining the change. Thanks a lot.","commit_id":"61403c7bbcfdf779615ebdb1b50037e1199cc350"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"f586160e432b5137a5e57e1abeda66bad467ba44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3e505afc_fb6370fd","updated":"2022-10-24 18:13:00.000000000","message":"Hello: apologies for not responding to your review in a timely fashion...\n\nIs that the same issue? It looks different... the context of this is that the MTU of the VIF follows the definition of the Neutron network, so it can change during live-migration (which libvirt cannot support).\n\nThe fix here was to warn users by throwing an exception or something earlier in the live-migration process, rather than letting execution bubble all the way down into libvirt-specific logic and then exploding with a less-than-helpful error message.\n","commit_id":"61403c7bbcfdf779615ebdb1b50037e1199cc350"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"b770bfa9897df726d0d9dd6c28fb0edb4da4c7ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4cd5579f_ed3d408a","updated":"2022-08-09 03:27:44.000000000","message":"Needs unit tests, may be able to clean up extraction of the guest MTU.","commit_id":"61403c7bbcfdf779615ebdb1b50037e1199cc350"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8a10e0cfa9f5c04b004fe99d60ae3e5c011ad21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"96e332b4_5f3c6d99","updated":"2022-10-24 18:47:51.000000000","message":"i have restored this for now but i need to re review and see if this approhc is still valid.\n\ni wont get to this today but ill try and take a look again tomorrow\n\nin general we can do one of two things \n\ncheck if the souce/destination vif object have the same mtu on each vif.\nor check if the source vifs match the curret  network mtu\n\ni think you did the later if i rememebre correctly but ill take a look again wehn i review.","commit_id":"61403c7bbcfdf779615ebdb1b50037e1199cc350"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f9d000ad5c99abc442a61bed71157b490d70d4ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"747e8e38_b0385287","in_reply_to":"3e505afc_fb6370fd","updated":"2022-10-24 18:45:26.000000000","message":"i guess we can resotre this and do this also\n\nwe cannot support changing the MTU when live migrating but we can warn and check for it in prelive migration.\ni did not fully reread this when i was updating the ptg summery email so you are correc this is not entirly the same and we can do both.","commit_id":"61403c7bbcfdf779615ebdb1b50037e1199cc350"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1469aa052c6a0c6e9ca54df15deecd32cf95c10","unresolved":true,"context_lines":[{"line_number":9585,"context_line":"        if instance.numa_topology:"},{"line_number":9586,"context_line":"            dest_check_data.src_supports_numa_live_migration \u003d True"},{"line_number":9587,"context_line":""},{"line_number":9588,"context_line":"        for vif in instance.get_network_info():"},{"line_number":9589,"context_line":"            self.vif_driver.check_can_live_migrate_vif(instance, vif,"},{"line_number":9590,"context_line":"                                                       self._host)"},{"line_number":9591,"context_line":""},{"line_number":9592,"context_line":"        return dest_check_data"},{"line_number":9593,"context_line":""},{"line_number":9594,"context_line":"    def _is_shared_block_storage(self, instance, dest_check_data,"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ef4ec4f_0742fe01","line":9591,"range":{"start_line":9588,"start_character":7,"end_line":9591,"end_character":0},"updated":"2022-08-09 12:17:10.000000000","message":"im not sure this is the correct placce to check.\n\nwe shoudl should proably be using the destination vifs form the migrate data object\n\nsince we need to check if the destination vifs will hae a differnt mtu form the source vifs.\n\n\nthe network info cache will still have the source vif info so that not corect to check at this point","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"79226b756f063611abbd3b10ef5208ac1770e839","unresolved":true,"context_lines":[{"line_number":9585,"context_line":"        if instance.numa_topology:"},{"line_number":9586,"context_line":"            dest_check_data.src_supports_numa_live_migration \u003d True"},{"line_number":9587,"context_line":""},{"line_number":9588,"context_line":"        for vif in instance.get_network_info():"},{"line_number":9589,"context_line":"            self.vif_driver.check_can_live_migrate_vif(instance, vif,"},{"line_number":9590,"context_line":"                                                       self._host)"},{"line_number":9591,"context_line":""},{"line_number":9592,"context_line":"        return dest_check_data"},{"line_number":9593,"context_line":""},{"line_number":9594,"context_line":"    def _is_shared_block_storage(self, instance, dest_check_data,"}],"source_content_type":"text/x-python","patch_set":1,"id":"60698772_fb844e1b","line":9591,"range":{"start_line":9588,"start_character":7,"end_line":9591,"end_character":0},"in_reply_to":"6ef4ec4f_0742fe01","updated":"2022-10-25 01:55:47.000000000","message":"Hmm, why would this not be the right place? This is the live migration checks at source/dest that the conductor does before committing any changes for the live-migration. We need the domain XML at the source to compare to the Neutron network.\n\n---\n\nFrom what I can gather, the VIFMigrateData/destination VIFs are just deep-copied source VIFs i.e., they are hydrated with instance.get_network_info() and only other fields like binding profile are replaced for the destination. I can drag the migrate_data over from check_can_live_migrate_destination() though as it\u0027s still a bit clearer.\n\nThough, more importantly, your mention of this makes me realize we probably need to use network_api.get_instance_nw_info() to hydrate the VIFMigrateData, not the instance info cache. Fresh network info is what pre_live_migration() uses and that\u0027s not called until *AFTER* conductor calls the check code (here).\n\nRight now we use cached instance info which could be stale if a user changes a Neutron network and then immediately fires off a live-migration. In which case we might let a live-migration through the pre-checks here that still fails later.\n\nWhat do you think?","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1469aa052c6a0c6e9ca54df15deecd32cf95c10","unresolved":true,"context_lines":[{"line_number":870,"context_line":"        # same time, Neutron will assume the current MTU of the network is how"},{"line_number":871,"context_line":"        # the VIF should be configured. So, in order for migration to not"},{"line_number":872,"context_line":"        # break the current host configured MTU must match that of the network."},{"line_number":873,"context_line":"        network \u003d vif.get(\u0027network\u0027)"},{"line_number":874,"context_line":"        mtu \u003d network.get_meta(\u0027mtu\u0027) if network else None"},{"line_number":875,"context_line":"        cfg \u003d self.get_config(instance, vif, instance.image_meta,"},{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0b89570f_2ef8a3bd","line":874,"range":{"start_line":873,"start_character":7,"end_line":874,"end_character":58},"updated":"2022-08-09 12:17:10.000000000","message":"so thses shoudl alwasy be present\n\nyou cannot create a neutorn port that is not assocatied with a network and networks always have mtus defiend i belive.\n\nthe mtu im less sure about but the network will always be present in neutron","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"79226b756f063611abbd3b10ef5208ac1770e839","unresolved":false,"context_lines":[{"line_number":870,"context_line":"        # same time, Neutron will assume the current MTU of the network is how"},{"line_number":871,"context_line":"        # the VIF should be configured. So, in order for migration to not"},{"line_number":872,"context_line":"        # break the current host configured MTU must match that of the network."},{"line_number":873,"context_line":"        network \u003d vif.get(\u0027network\u0027)"},{"line_number":874,"context_line":"        mtu \u003d network.get_meta(\u0027mtu\u0027) if network else None"},{"line_number":875,"context_line":"        cfg \u003d self.get_config(instance, vif, instance.image_meta,"},{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7bd3e831_918f0c4e","line":874,"range":{"start_line":873,"start_character":7,"end_line":874,"end_character":58},"in_reply_to":"0b89570f_2ef8a3bd","updated":"2022-10-25 01:55:47.000000000","message":"Ack","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"b770bfa9897df726d0d9dd6c28fb0edb4da4c7ab","unresolved":true,"context_lines":[{"line_number":873,"context_line":"        network \u003d vif.get(\u0027network\u0027)"},{"line_number":874,"context_line":"        mtu \u003d network.get_meta(\u0027mtu\u0027) if network else None"},{"line_number":875,"context_line":"        cfg \u003d self.get_config(instance, vif, instance.image_meta,"},{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        if mtu is not None and iface.mtu !\u003d mtu:"}],"source_content_type":"text/x-python","patch_set":1,"id":"97f7da68_79920627","line":876,"updated":"2022-08-09 03:27:44.000000000","message":"Pretty sure this only applies to vhost or a specific kind of VIF?","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1469aa052c6a0c6e9ca54df15deecd32cf95c10","unresolved":true,"context_lines":[{"line_number":873,"context_line":"        network \u003d vif.get(\u0027network\u0027)"},{"line_number":874,"context_line":"        mtu \u003d network.get_meta(\u0027mtu\u0027) if network else None"},{"line_number":875,"context_line":"        cfg \u003d self.get_config(instance, vif, instance.image_meta,"},{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        if mtu is not None and iface.mtu !\u003d mtu:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a4e0368b_3eaa25d3","line":876,"in_reply_to":"97f7da68_79920627","updated":"2022-08-09 12:17:10.000000000","message":"we have had a long standign item to add a check like this for some time.\nbasically this happens if an only if you use the mutable mtu extion in neutron or you change the encapsulation type which is technially not allowed but is doen when changing from ml2/ovs to ml2/ovn\n\nif you do either then to be able to live migration you need to hard reboot the guest for the xml to be updated as it cant be changed on a live domain.\n\nso this realy applies ato all vif types including macvtap sriov, kernel/userspace vhost and any other casue where we specify the mtu in the xml.\n\nso we need to check all vifs that have an mtu in the xml.","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"79226b756f063611abbd3b10ef5208ac1770e839","unresolved":false,"context_lines":[{"line_number":873,"context_line":"        network \u003d vif.get(\u0027network\u0027)"},{"line_number":874,"context_line":"        mtu \u003d network.get_meta(\u0027mtu\u0027) if network else None"},{"line_number":875,"context_line":"        cfg \u003d self.get_config(instance, vif, instance.image_meta,"},{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        if mtu is not None and iface.mtu !\u003d mtu:"}],"source_content_type":"text/x-python","patch_set":1,"id":"90acfb6e_efd93725","line":876,"in_reply_to":"a4e0368b_3eaa25d3","updated":"2022-10-25 01:55:47.000000000","message":"Ack","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d1469aa052c6a0c6e9ca54df15deecd32cf95c10","unresolved":true,"context_lines":[{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        if mtu is not None and iface.mtu !\u003d mtu:"},{"line_number":880,"context_line":"            reason \u003d _(\"Instance must be hard rebooted after a Neutron network \""},{"line_number":881,"context_line":"                       \"MTU change before live-migrating it.\")"},{"line_number":882,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":1,"id":"007a45d9_90dbd291","line":879,"updated":"2022-08-09 12:17:10.000000000","message":"the iface.mtu may or may not be set depending on the network type so you need to check that it is not none before comparing to the network mtu.\n\nsriov direct-physical ports wont have an mtu set and im not sure about direct\n\nmacvtap will but we shoudl make the code handel this correctly.\n\ncurrently for sriov we do hotplug live migration meaning we unplug the vf then migrate then reattach the vf so the mtu does not techically matter in that case but\nwe should still handel that here.","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"},{"author":{"_account_id":33910,"name":"Tyler Stachecki","display_name":"Tyler Stachecki","email":"tstachecki@bloomberg.net","username":"tjstachecki"},"change_message_id":"79226b756f063611abbd3b10ef5208ac1770e839","unresolved":true,"context_lines":[{"line_number":876,"context_line":"                              instance.flavor, CONF.libvirt.virt_type, host)"},{"line_number":877,"context_line":"        iface \u003d host.get_guest(instance).get_interface_by_cfg(cfg)"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"        if mtu is not None and iface.mtu !\u003d mtu:"},{"line_number":880,"context_line":"            reason \u003d _(\"Instance must be hard rebooted after a Neutron network \""},{"line_number":881,"context_line":"                       \"MTU change before live-migrating it.\")"},{"line_number":882,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":1,"id":"54603077_045f3dc3","line":879,"in_reply_to":"007a45d9_90dbd291","updated":"2022-10-25 01:55:47.000000000","message":"Just checking -- io in the case the iface has no MTU set, we should skip the gate check against the MTU from the Neutron network and just let it through right?","commit_id":"118d524574717a25f141539d7dd732b38134c5f2"}]}