Merge "libvirt: Add new option to enforce multipath volume connections"

nova/conf/libvirt.py

@@ -1112,6 +1112,22 @@ Use multipath connection of the iSCSI or FC volume
 
 Volumes can be connected in the LibVirt as multipath devices. This will
 provide high availability and fault tolerance.
+"""),
+    cfg.BoolOpt('volume_enforce_multipath',
+                default=False,
+                help="""
+Require multipathd when attaching a volume to an instance.
+
+When enabled, attachment of volumes will be aborted when multipathd is not
+running. Otherwise, it will fallback to single path without error.
+
+When enabled, the libvirt driver checks availability of mulitpathd when it is
+initialized, and the compute service fails to start if multipathd is not
+running.
+
+Related options:
+
+* volume_use_multipath must be True when this is True
 """),
     cfg.IntOpt('num_volume_scan_tries',
                deprecated_name='num_iscsi_scan_tries',

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

@@ -1661,6 +1661,59 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
         mock_which.assert_not_called()
 
+    def test__check_multipath_misconfiguration(self):
+        self.flags(volume_use_multipath=False, volume_enforce_multipath=True,
+                   group='libvirt')
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        exc = self.assertRaises(
+            exception.InvalidConfiguration,
+            drvr._check_multipath)
+
+        self.assertIn(
+            "The 'volume_use_multipath' option should be 'True' when "
+            "the 'volume_enforce_multipath' option is 'True'.",
+            str(exc),
+        )
+
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running')
+    def test__check_multipath_disabled(self, is_multipath_running):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr._check_multipath()
+        is_multipath_running.assert_not_called()
+
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running')
+    def test__check_multipath_optional(self, is_multipath_running):
+        self.flags(volume_use_multipath=True, group='libvirt')
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr._check_multipath()
+        is_multipath_running.assert_not_called()
+
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running')
+    def test__check_multipath_enforced(self, is_multipath_running):
+        is_multipath_running.return_value = True
+        self.flags(volume_use_multipath=True, volume_enforce_multipath=True,
+                   group='libvirt')
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        drvr._check_multipath()
+        is_multipath_running.assert_called_once_with(root_helper=mock.ANY)
+
+    @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running')
+    def test__check_multipath_enforced_missing(self, is_multipath_running):
+        is_multipath_running.return_value = False
+        self.flags(volume_use_multipath=True, volume_enforce_multipath=True,
+                   group='libvirt')
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        exc = self.assertRaises(
+            exception.InvalidConfiguration,
+            drvr._check_multipath)
+
+        self.assertIn(
+            "The 'volume_enforce_multipath' option is 'True' but "
+            "multipathd is not running.",
+            str(exc),
+        )
+        is_multipath_running.assert_called_once_with(root_helper=mock.ANY)
+
     @mock.patch.object(libvirt_driver.LOG, 'warning')
     def test_check_cpu_set_configuration__no_configuration(self, mock_log):
         """Test that configuring no CPU option results no errors or logs.

nova/tests/unit/virt/libvirt/volume/test_nvme.py

@@ -30,7 +30,7 @@ class LibvirtNVMEVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase):
         nvme.LibvirtNVMEVolumeDriver(self.fake_host)
         mock_factory.assert_called_once_with(
             initiator.NVME, 'sudo', use_multipath=False,
-            device_scan_attempts=3)
+            device_scan_attempts=3, enforce_multipath=False)
 
     @mock.patch('os.path.exists', return_value=True)
     @mock.patch('nova.utils.get_root_helper')
@@ -44,7 +44,21 @@ class LibvirtNVMEVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase):
         nvme.LibvirtNVMEVolumeDriver(self.fake_host)
         mock_factory.assert_called_once_with(
             initiator.NVME, 'sudo', use_multipath=True,
-            device_scan_attempts=3)
+            device_scan_attempts=3, enforce_multipath=False)
+
+    @mock.patch('os.path.exists', return_value=True)
+    @mock.patch('nova.utils.get_root_helper')
+    @mock.patch('os_brick.initiator.connector.InitiatorConnector.factory')
+    def test_libvirt_nvme_driver_multipath_enforced(self, mock_factory,
+                                                    mock_helper, exists):
+        self.flags(num_nvme_discover_tries=3, volume_use_multipath=True,
+                   volume_enforce_multipath=True, group='libvirt')
+        mock_helper.return_value = 'sudo'
+
+        nvme.LibvirtNVMEVolumeDriver(self.fake_host)
+        mock_factory.assert_called_once_with(
+            initiator.NVME, 'sudo', use_multipath=True,
+            device_scan_attempts=3, enforce_multipath=True)
 
     @mock.patch('os_brick.initiator.connector.InitiatorConnector.factory',
         new=mock.Mock(return_value=mock.Mock()))

nova/virt/libvirt/driver.py

@@ -56,6 +56,7 @@ from os_brick import encryptors
 from os_brick.encryptors import luks as luks_encryptor
 from os_brick import exception as brick_exception
 from os_brick.initiator import connector
+from os_brick.initiator import linuxscsi
 import os_resource_classes as orc
 import os_traits as ot
 from oslo_concurrency import processutils
@@ -899,6 +900,8 @@ class LibvirtDriver(driver.ComputeDriver):
 
         self._check_vtpm_support()
 
+        self._check_multipath()
+
         # Set REGISTER_IMAGE_PROPERTY_DEFAULTS in the instance system_metadata
         # to default values for properties that have not already been set.
         self._register_all_undefined_instance_details()
@@ -1163,6 +1166,22 @@ class LibvirtDriver(driver.ComputeDriver):
 
         LOG.debug('Enabling emulated TPM support')
 
+    def _check_multipath(self) -> None:
+        if not CONF.libvirt.volume_enforce_multipath:
+            return
+
+        if not CONF.libvirt.volume_use_multipath:
+            msg = _("The 'volume_use_multipath' option should be 'True' when "
+                    "the 'volume_enforce_multipath' option is 'True'.")
+            raise exception.InvalidConfiguration(msg)
+
+        multipath_running = linuxscsi.LinuxSCSI.is_multipath_running(
+            root_helper=utils.get_root_helper())
+        if not multipath_running:
+            msg = _("The 'volume_enforce_multipath' option is 'True' but "
+                    "multipathd is not running.")
+            raise exception.InvalidConfiguration(msg)
+
     def _start_inactive_mediated_devices(self):
         # Get a list of inactive mdevs so we can start them and make them
         # active. We need to start inactive mdevs even if they are not

nova/virt/libvirt/volume/fibrechannel.py

@@ -35,7 +35,8 @@ class LibvirtFibreChannelVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
         self.connector = connector.InitiatorConnector.factory(
             initiator.FIBRE_CHANNEL, utils.get_root_helper(),
             use_multipath=CONF.libvirt.volume_use_multipath,
-            device_scan_attempts=CONF.libvirt.num_volume_scan_tries)
+            device_scan_attempts=CONF.libvirt.num_volume_scan_tries,
+            enforce_multipath=CONF.libvirt.volume_enforce_multipath)
 
     def get_config(self, connection_info, disk_info):
         """Returns xml for libvirt."""

nova/virt/libvirt/volume/iscsi.py

@@ -38,7 +38,8 @@ class LibvirtISCSIVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
             initiator.ISCSI, utils.get_root_helper(),
             use_multipath=CONF.libvirt.volume_use_multipath,
             device_scan_attempts=CONF.libvirt.num_volume_scan_tries,
-            transport=self._get_transport())
+            transport=self._get_transport(),
+            enforce_multipath=CONF.libvirt.volume_enforce_multipath)
 
     def _get_transport(self):
         if CONF.libvirt.iscsi_iface:

nova/virt/libvirt/volume/iser.py

@@ -33,7 +33,8 @@ class LibvirtISERVolumeDriver(iscsi.LibvirtISCSIVolumeDriver):
             initiator.ISER, utils.get_root_helper(),
             use_multipath=CONF.libvirt.iser_use_multipath,
             device_scan_attempts=CONF.libvirt.num_iser_scan_tries,
-            transport=self._get_transport())
+            transport=self._get_transport(),
+            enforce_multipath=CONF.libvirt.volume_enforce_multipath)
 
     def _get_transport(self):
         return 'iser'

nova/virt/libvirt/volume/nvme.py

@@ -34,7 +34,8 @@ class LibvirtNVMEVolumeDriver(libvirt_volume.LibvirtVolumeDriver):
         self.connector = connector.InitiatorConnector.factory(
             initiator.NVME, utils.get_root_helper(),
             use_multipath=CONF.libvirt.volume_use_multipath,
-            device_scan_attempts=CONF.libvirt.num_nvme_discover_tries)
+            device_scan_attempts=CONF.libvirt.num_nvme_discover_tries,
+            enforce_multipath=CONF.libvirt.volume_enforce_multipath)
 
     def connect_volume(self, connection_info, instance):
 

releasenotes/notes/volume_enforce_multipath-c790e98b9b05848e.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    The new ``[libvirt] volume_enforce_multipath`` option has been added. When
+    this option is set to ``True``, attachment of volumes is aborted when
+    multipathd is not running in the host. Otherwise it falls back to single
+    path. This option also makes the libvirt driver to check multipathd during
+    initialization, and the compute service fails to start if mulitipathd is
+    not running.

requirements.txt

@@ -49,7 +49,7 @@ rfc3986>=1.2.0 # Apache-2.0
 oslo.middleware>=3.31.0 # Apache-2.0
 psutil>=3.2.2 # BSD
 oslo.versionedobjects>=1.35.0 # Apache-2.0
-os-brick>=6.0 # Apache-2.0
+os-brick>=6.10.0 # Apache-2.0
 os-resource-classes>=1.1.0 # Apache-2.0
 os-traits>=3.3.0 # Apache-2.0
 os-vif>=3.1.0 # Apache-2.0