libvirt: Do not reference VIR_ERR_DEVICE_MISSING when libvirt is < v4.1.0

I7eb86edc130d186a66c04b229d46347ec5c0b625 introduced
VIR_ERR_DEVICE_MISSING into the hot unplug libvirt error code list
within detach_device_with_retry. While the change correctly referenced
that the error code was introduced in v4.1.0 it made no attempt to
handle versions prior to this. With MIN_LIBVIRT_VERSION currently pinned
to v4.0.0 we need to handle libvirt < v4.1.0 to avoid referencing the
non-existent error code within the libvirt module.

Closes-Bug: #1891547
Change-Id: I32908b77c18f8ec08211dd67be49bbf903611c34

Author: lyarwood

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

@@ -9460,6 +9460,51 @@ def test_attach_volume_with_vir_domain_affect_live_flag(self,
                 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):
@@ -18812,6 +18857,51 @@ def test_detach_volume_with_instance_not_found(self):
             _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().

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

@@ -294,8 +294,8 @@ def test_detach_device_with_retry_device_not_found_alt_name(self):
 
     @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)
@@ -312,7 +312,8 @@ def _test_detach_device_with_retry_second_detach_failure(
         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
@@ -325,20 +326,25 @@ def _test_detach_device_with_retry_second_detach_failure(
     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
@@ -369,7 +375,8 @@ def _test_detach_device_with_retry_first_detach_failure(
         # 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(
             "</xml>", flags=(fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
@@ -385,22 +392,28 @@ def _test_detach_device_with_retry_first_detach_failure(
     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()

nova/virt/libvirt/driver.py

@@ -275,6 +275,8 @@ def repr_method(self):
 MIN_LIBVIRT_BLOCKDEV = (6, 0, 0)
 MIN_QEMU_BLOCKDEV = (4, 2, 0)
 
+MIN_LIBVIRT_VIR_ERR_DEVICE_MISSING = (4, 1, 0)
+
 
 class LibvirtDriver(driver.ComputeDriver):
     def __init__(self, virtapi, read_only=False):
@@ -2015,9 +2017,11 @@ def detach_volume(self, context, connection_info, instance, mountpoint,
             # 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:
@@ -2220,9 +2224,12 @@ def detach_interface(self, context, instance, vif):
             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

nova/virt/libvirt/guest.py

@@ -375,7 +375,8 @@ def get_all_devices(self, devtype=None):
     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
@@ -396,8 +397,16 @@ def detach_device_with_retry(self, get_device_conf_func, device, live,
                                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
@@ -414,9 +423,10 @@ def _try_detach_device(conf, persistent=False, live=False):
                     # 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()