libvirt: Add new option to enforce multipath volume connections

Currently, even when [libvirt] volume_use_multipath is set to True,
volume attachment silently falls back to single path if multipathd is
not running in the host. This sometimes prevents operators from
noticing the misconfiguration, until they face any issue caused by
missing redundancy.

Introduce the new [libvirt] volume_enforce_multipath option, which
makes the attachment process fail if multipathd is not running.
A similar parameter (enforce_multipath_for_image_xfer) was already
implemented in cinder and this change follows how the parameter is
implemented there.

Also add the check in init phase to detect lack of mulitipath daemon
during initializing driver.

Min version of os-brick has to be bumped due to the interface change
made by 8d919696a9f1b1361f00aac7032647b5e1656082 .

Implements: blueprint enforce-multipath
Change-Id: I828de70ca7b343a4562ace4049d2b3857dbf900a

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
@@ -900,6 +901,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()
@@ -1164,6 +1167,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