diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 85a237bd7229..95a43227ed37 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -7928,6 +7928,51 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_build_metadata.assert_called_with(self.context, instance) mock_save.assert_called_with() + @mock.patch('nova.virt.libvirt.host.Host.get_guest') + @mock.patch.object(fakelibvirt.Connection, 'getLibVersion') + @mock.patch( + 'nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume', + new=mock.Mock()) + def test_detach_volume_supports_device_missing( + self, mock_get_version, mock_get_guest): + """Assert that VIR_ERR_DEVICE_MISSING is only used if libvirt >= v4.1.0 + """ + mock_guest = mock.Mock(spec=libvirt_guest.Guest) + mock_guest.get_power_state.return_value = power_state.RUNNING + mock_get_guest.return_value = mock_guest + + v4_0_0 = versionutils.convert_version_to_int((4, 0, 0)) + mock_get_version.return_value = v4_0_0 + + mountpoint = "/dev/foo" + expected_disk_dev = "foo" + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr.detach_volume( + self.context, mock.sentinel.connection_info, + mock.sentinel.instance, mountpoint) + + # Assert supports_device_missing_error_code=False is used + mock_guest.detach_device_with_retry.assert_called_once_with( + mock_guest.get_disk, expected_disk_dev, live=True, + supports_device_missing_error_code=False) + + # reset and try again with v4.1.0 + mock_guest.reset_mock() + mock_get_version.reset_mock() + + v4_1_0 = versionutils.convert_version_to_int((4, 1, 0)) + mock_get_version.return_value = v4_1_0 + + drvr.detach_volume( + self.context, mock.sentinel.connection_info, + mock.sentinel.instance, mountpoint) + + # Assert supports_device_missing_error_code=True is used + mock_guest.detach_device_with_retry.assert_called_once_with( + mock_guest.get_disk, expected_disk_dev, live=True, + supports_device_missing_error_code=True) + @mock.patch('nova.virt.libvirt.host.Host._get_domain') def test_detach_volume_with_vir_domain_affect_live_flag(self, mock_get_domain): @@ -16862,6 +16907,51 @@ class LibvirtConnTestCase(test.NoDBTestCase, _disconnect_volume.assert_called_once_with( self.context, connection_info, instance, encryption=None) + @mock.patch('nova.virt.libvirt.host.Host.get_guest') + @mock.patch.object(fakelibvirt.Connection, 'getLibVersion') + def test_detach_interface_supports_device_missing( + self, mock_get_version, mock_get_guest): + """Assert that VIR_ERR_DEVICE_MISSING is only used if libvirt >= v4.1.0 + """ + mock_guest = mock.Mock(spec=libvirt_guest.Guest) + mock_guest.get_power_state.return_value = power_state.RUNNING + mock_get_guest.return_value = mock_guest + + v4_0_0 = versionutils.convert_version_to_int((4, 0, 0)) + mock_get_version.return_value = v4_0_0 + + instance = objects.Instance(**self.test_instance) + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + with mock.patch.object(drvr, 'vif_driver') as mock_vif_driver: + mock_vif_driver.get_config.return_value = mock.sentinel.cfg + mock_vif_driver.get_vif_devname.return_value = mock.sentinel.dev + + drvr.detach_interface( + self.context, instance, mock.sentinel.vif) + + # Assert supports_device_missing_error_code=False is used + mock_guest.detach_device_with_retry.assert_called_once_with( + mock_guest.get_interface_by_cfg, mock.sentinel.cfg, live=True, + alternative_device_name=mock.sentinel.dev, + supports_device_missing_error_code=False) + + # reset and try again with v4.1.0 + mock_guest.reset_mock() + mock_get_version.reset_mock() + + v4_1_0 = versionutils.convert_version_to_int((4, 1, 0)) + mock_get_version.return_value = v4_1_0 + + drvr.detach_interface( + self.context, instance, mock.sentinel.vif) + + # Assert supports_device_missing_error_code=True is used + mock_guest.detach_device_with_retry.assert_called_once_with( + mock_guest.get_interface_by_cfg, mock.sentinel.cfg, live=True, + alternative_device_name=mock.sentinel.dev, + supports_device_missing_error_code=True) + def _test_attach_detach_interface_get_config(self, method_name): """Tests that the get_config() method is properly called in attach_interface() and detach_interface(). diff --git a/nova/tests/unit/virt/libvirt/test_guest.py b/nova/tests/unit/virt/libvirt/test_guest.py index 118d5dd8cf1b..e019d9cde1a9 100644 --- a/nova/tests/unit/virt/libvirt/test_guest.py +++ b/nova/tests/unit/virt/libvirt/test_guest.py @@ -308,8 +308,8 @@ class GuestTestCase(test.NoDBTestCase): @mock.patch.object(libvirt_guest.Guest, "detach_device") def _test_detach_device_with_retry_second_detach_failure( - self, mock_detach, error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, - error_message="device not found: disk vdb not found"): + self, mock_detach, error_code=None, error_message=None, + supports_device_missing=False): # This simulates a retry of the transient/live domain detach # failing because the device is not found conf = mock.Mock(spec=vconfig.LibvirtConfigGuestDevice) @@ -326,7 +326,8 @@ class GuestTestCase(test.NoDBTestCase): mock_detach.side_effect = [None, fake_exc] retry_detach = self.guest.detach_device_with_retry( get_config, fake_device, live=True, - inc_sleep_time=.01, max_retry_count=3) + inc_sleep_time=.01, max_retry_count=3, + supports_device_missing_error_code=supports_device_missing) # Some time later, we can do the wait/retry to ensure detach self.assertRaises(exception.DeviceNotFound, retry_detach) # Check that the save_and_reraise_exception context manager didn't log @@ -339,20 +340,25 @@ class GuestTestCase(test.NoDBTestCase): def test_detach_device_with_retry_second_detach_operation_failed(self): self._test_detach_device_with_retry_second_detach_failure( error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, - error_message="operation failed: disk vdb not found") + error_message="operation failed: disk vdb not found", + supports_device_missing=False) # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 def test_detach_device_with_retry_second_detach_internal_error(self): self._test_detach_device_with_retry_second_detach_failure( error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR, - error_message="operation failed: disk vdb not found") + error_message="operation failed: disk vdb not found", + supports_device_missing=False) def test_detach_device_with_retry_second_detach_device_missing(self): - self._test_detach_device_with_retry_second_detach_failure() + self._test_detach_device_with_retry_second_detach_failure( + error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, + error_message="device not found: disk vdb not found", + supports_device_missing=True) def _test_detach_device_with_retry_first_detach_failure( - self, error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, - error_message="device not found: disk vdb not found"): + self, error_code=None, error_message=None, + supports_device_missing=False): # This simulates a persistent or live domain detach failing because the # device is not found during the first attempt to detach the device. # We should still attempt to detach the device from the live config if @@ -383,7 +389,8 @@ class GuestTestCase(test.NoDBTestCase): # succeeds afterward self.domain.detachDeviceFlags.side_effect = [fake_exc, None] retry_detach = self.guest.detach_device_with_retry(get_config, - fake_device, live=True, inc_sleep_time=.01, max_retry_count=3) + fake_device, live=True, inc_sleep_time=.01, max_retry_count=3, + supports_device_missing_error_code=supports_device_missing) # We should have tried to detach from the persistent domain self.domain.detachDeviceFlags.assert_called_once_with( "", flags=(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG | @@ -399,22 +406,28 @@ class GuestTestCase(test.NoDBTestCase): def test_detach_device_with_retry_first_detach_operation_failed(self): self._test_detach_device_with_retry_first_detach_failure( error_code=fakelibvirt.VIR_ERR_OPERATION_FAILED, - error_message="operation failed: disk vdb not found") + error_message="operation failed: disk vdb not found", + supports_device_missing=False) # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 def test_detach_device_with_retry_first_detach_internal_error(self): self._test_detach_device_with_retry_first_detach_failure( error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR, - error_message="operation failed: disk vdb not found") + error_message="operation failed: disk vdb not found", + supports_device_missing=False) # TODO(lyarwood): Remove this test once MIN_LIBVIRT_VERSION is >= 4.1.0 def test_detach_device_with_retry_first_detach_invalid_arg(self): self._test_detach_device_with_retry_first_detach_failure( error_code=fakelibvirt.VIR_ERR_INVALID_ARG, - error_message="invalid argument: no target device vdb") + error_message="invalid argument: no target device vdb", + supports_device_missing=False) def test_detach_device_with_retry_first_detach_device_missing(self): - self._test_detach_device_with_retry_first_detach_failure() + self._test_detach_device_with_retry_first_detach_failure( + error_code=fakelibvirt.VIR_ERR_DEVICE_MISSING, + error_message="device not found: disk vdb not found", + supports_device_missing=True) def test_get_xml_desc(self): self.guest.get_xml_desc() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index a7e60fd0e482..9b9f8151a9d0 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -292,6 +292,8 @@ MIN_LIBVIRT_BETTER_SIGKILL_HANDLING = (4, 7, 0) VGPU_RESOURCE_SEMAPHORE = "vgpu_resources" +MIN_LIBVIRT_VIR_ERR_DEVICE_MISSING = (4, 1, 0) + class LibvirtDriver(driver.ComputeDriver): def __init__(self, virtapi, read_only=False): @@ -1754,9 +1756,11 @@ class LibvirtDriver(driver.ComputeDriver): # detaching any attached encryptors or disconnecting the underlying # volume in _disconnect_volume. Otherwise, the encryptor or volume # driver may report that the volume is still in use. - wait_for_detach = guest.detach_device_with_retry(guest.get_disk, - disk_dev, - live=live) + supports_device_missing = self._host.has_min_version( + MIN_LIBVIRT_VIR_ERR_DEVICE_MISSING) + wait_for_detach = guest.detach_device_with_retry( + guest.get_disk, disk_dev, live=live, + supports_device_missing_error_code=supports_device_missing) wait_for_detach() except exception.InstanceNotFound: @@ -1902,9 +1906,12 @@ class LibvirtDriver(driver.ComputeDriver): live = state in (power_state.RUNNING, power_state.PAUSED) # Now we are going to loop until the interface is detached or we # timeout. + supports_device_missing = self._host.has_min_version( + MIN_LIBVIRT_VIR_ERR_DEVICE_MISSING) wait_for_detach = guest.detach_device_with_retry( guest.get_interface_by_cfg, cfg, live=live, - alternative_device_name=self.vif_driver.get_vif_devname(vif)) + alternative_device_name=self.vif_driver.get_vif_devname(vif), + supports_device_missing_error_code=supports_device_missing) wait_for_detach() except exception.DeviceDetachFailed: # We failed to detach the device even with the retry loop, so let's diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index f1b06717ad5c..0169a6747d4a 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -370,7 +370,8 @@ class Guest(object): def detach_device_with_retry(self, get_device_conf_func, device, live, max_retry_count=7, inc_sleep_time=2, max_sleep_time=30, - alternative_device_name=None): + alternative_device_name=None, + supports_device_missing_error_code=False): """Detaches a device from the guest. After the initial detach request, a function is returned which can be used to ensure the device is successfully removed from the guest domain (retrying the removal as @@ -391,8 +392,16 @@ class Guest(object): max_sleep_time will be used as the sleep time. :param alternative_device_name: This is an alternative identifier for the device if device is not an ID, used solely for error messages. + :param supports_device_missing_error_code: does the installed version + of libvirt provide the + VIR_ERR_DEVICE_MISSING error + code. """ alternative_device_name = alternative_device_name or device + unplug_libvirt_error_codes = set([ + libvirt.VIR_ERR_OPERATION_FAILED, + libvirt.VIR_ERR_INTERNAL_ERROR + ]) def _try_detach_device(conf, persistent=False, live=False): # Raise DeviceNotFound if the device isn't found during detach @@ -409,9 +418,10 @@ class Guest(object): # TODO(lyarwood): Remove libvirt.VIR_ERR_OPERATION_FAILED # and libvirt.VIR_ERR_INTERNAL_ERROR once # MIN_LIBVIRT_VERSION is >= 4.1.0 - if errcode in (libvirt.VIR_ERR_OPERATION_FAILED, - libvirt.VIR_ERR_INTERNAL_ERROR, - libvirt.VIR_ERR_DEVICE_MISSING): + if supports_device_missing_error_code: + unplug_libvirt_error_codes.add( + libvirt.VIR_ERR_DEVICE_MISSING) + if errcode in unplug_libvirt_error_codes: # TODO(lyarwood): Remove the following error message # check once we only care about VIR_ERR_DEVICE_MISSING errmsg = ex.get_error_message()