Merge "Only unplug vif after the device is detached from libvirt"

Author: Zuul

nova/tests/unit/virt/libvirt/test_driver.py

@@ -23596,11 +23596,12 @@ def _test_detach_interface(self, power_state, expected_flags,
             mock.patch.object(guest, 'get_interface_by_cfg',
                               side_effect=get_interface_calls),
             mock.patch.object(domain, 'detachDeviceFlags'),
-            mock.patch('nova.virt.libvirt.driver.LOG.warning')
+            mock.patch('nova.virt.libvirt.driver.LOG.warning'),
+            mock.patch.object(self.drvr.vif_driver, 'unplug')
         ) as (
             mock_get_guest, mock_get_config,
             mock_get_interface, mock_detach_device_flags,
-            mock_warning
+            mock_warning, mock_unplug
         ):
             # run the detach method
             self.drvr.detach_interface(self.context, instance, network_info[0])
@@ -23621,6 +23622,8 @@ def _test_detach_interface(self, power_state, expected_flags,
                 expected_cfg.to_xml(), flags=expected_flags)
             mock_warning.assert_not_called()
 
+        mock_unplug.assert_called_once_with(instance, network_info[0])
+
     def test_detach_interface_with_running_instance(self):
         self._test_detach_interface(
             power_state.RUNNING,

nova/virt/libvirt/driver.py

@@ -2277,7 +2277,6 @@ def detach_interface(self, context, instance, vif):
                                          CONF.libvirt.virt_type)
         interface = guest.get_interface_by_cfg(cfg)
         try:
-            self.vif_driver.unplug(instance, vif)
             # NOTE(mriedem): When deleting an instance and using Neutron,
             # we can be racing against Neutron deleting the port and
             # sending the vif-deleted event which then triggers a call to
@@ -2357,6 +2356,14 @@ def detach_interface(self, context, instance, vif):
                 LOG.warning('Detaching interface %(mac)s failed because '
                             'the device is no longer found on the guest.',
                             {'mac': mac}, instance=instance)
+        finally:
+            # NOTE(gibi): we need to unplug the vif _after_ the detach is done
+            # on the libvirt side as otherwise libvirt will still manage the
+            # device that our unplug code trying to reset. This can cause a
+            # race and leave the detached device configured. Also even if we
+            # are failed to detach due to race conditions the unplug is
+            # necessary for the same reason
+            self.vif_driver.unplug(instance, vif)
 
     def _create_snapshot_metadata(self, image_meta, instance,
                                   img_fmt, snp_name):