workarounds: Add option to locally attach RBD volumes on compute hosts

Building on the ``[workarounds]/disable_native_luksv1``
configurable introduced in Ia500eb614cf575ab846f64f4b69c9068274c8c1f
this change introduces another workaround configurable that when enabled
will connect RBD volumes to the compute host as block devices using
os-brick.

When used togther both options allow operators to workaround recently
discovered performance issues in the libgcrypt library used by QEMU when
natively decrypting LUKSv1 encrypted disks.

For now the extend_volume method raises a NotImplemented error in-line
with the underlying method in os-brick. Future work will be required to
both support this in os-brick and wire up the required calls in the
volume driver.

This workaround is temporary and will be removed during the W release
once all impacted distributions have been able to update their versions
of the libgcrypt library.

Finally os-brick 3.0.1 is now required as it provides the
Id507109df80391699074773f4787f74507c4b882 fix when attempting to
diconnect locally attached RBD volumes.

Closes-Bug: #1869184
Change-Id: Ied3732042738a6194b635c55e0304d71a6fb66e3

6 files changed, 151 insertions, 12 deletions

diff --git a/lower-constraints.txt b/lower-constraints.txt
index 2fb27f47e2..4dd9c35ccf 100644
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@@ -62,7 +62,7 @@ netifaces==0.10.4
 networkx==1.11
 numpy==1.14.2
 openstacksdk==0.35.0
-os-brick==2.6.2
+os-brick==3.0.1
 os-client-config==1.29.0
 os-resource-classes==0.4.0
 os-service-types==1.7.0
diff --git a/nova/conf/workarounds.py b/nova/conf/workarounds.py
index 4d315ebc81..366744ad23 100644
--- a/nova/conf/workarounds.py
+++ b/nova/conf/workarounds.py
@@ -290,12 +290,36 @@ Enabling this workaround option will cause Nova to use the legacy dm-crypt
 based os-brick encryptor to decrypt the LUKSv1 volume.
 
 Note that enabling this option while using volumes that do not provide a host
-block device such as Ceph will result in a failure to either boot from or
-attach the volume.
+block device such as Ceph will result in a failure to boot from or attach the
+volume to an instance. See the ``[workarounds]/rbd_block_device`` option for a
+way to avoid this for RBD.
 
 Related options:
 
 * ``compute_driver`` (libvirt)
+* ``rbd_block_device`` (workarounds)
+"""),
+    cfg.BoolOpt('rbd_volume_local_attach',
+                default=False,
+                help="""
+Attach RBD Cinder volumes to the compute as host block devices.
+
+When enabled this option instructs os-brick to connect RBD volumes locally on
+the compute host as block devices instead of natively through QEMU.
+
+This workaround does not currently support extending attached volumes.
+
+This can be used with the disable_native_luksv1 workaround configuration
+option to avoid the recently discovered performance issues found within the
+libgcrypt library.
+
+This workaround is temporary and will be removed during the W release once
+all impacted distributions have been able to update their versions of the
+libgcrypt library.
+
+Related options:
+* ``compute_driver`` (libvirt)
+* ``disable_qemu_native_luksv1`` (workarounds)
 """),
 ]
 
diff --git a/nova/tests/unit/virt/libvirt/volume/test_net.py b/nova/tests/unit/virt/libvirt/volume/test_net.py
index 0332afc3c2..25e0af7f39 100644
--- a/nova/tests/unit/virt/libvirt/volume/test_net.py
+++ b/nova/tests/unit/virt/libvirt/volume/test_net.py
@@ -221,7 +221,9 @@ class LibvirtNetVolumeDriverTestCase(
 
     def test_extend_volume(self):
         device_path = '/dev/fake-dev'
-        connection_info = {'data': {'device_path': device_path}}
+        connection_info = {
+            'driver_volume_type': 'net',
+            'data': {'device_path': device_path}}
 
         requested_size = 20 * pow(1024, 3)  # 20GiB
 
@@ -231,3 +233,49 @@ class LibvirtNetVolumeDriverTestCase(
                                                 requested_size)
 
         self.assertEqual(requested_size, new_size)
+
+    def test_libvirt_rbd_driver_block_connect(self):
+        self.flags(rbd_volume_local_attach=True, group='workarounds')
+        connection_info = self.rbd_connection(self.vol)
+        libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_host)
+        libvirt_driver.connector.connect_volume = mock.MagicMock(
+            return_value = {'path': mock.sentinel.rbd_dev})
+        libvirt_driver.connect_volume(connection_info, mock.sentinel.instance)
+
+        # Assert that the connector is called correctly and device_path updated
+        libvirt_driver.connector.connect_volume.assert_called_once_with(
+            connection_info['data'])
+
+    def test_libvirt_rbd_driver_block_disconnect(self):
+        self.flags(rbd_volume_local_attach=True, group='workarounds')
+        connection_info = self.rbd_connection(self.vol)
+        libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_host)
+        libvirt_driver.connector.disconnect_volume = mock.MagicMock()
+        libvirt_driver.disconnect_volume(connection_info,
+                                         mock.sentinel.instance)
+
+        # Assert that the connector is called correctly
+        libvirt_driver.connector.disconnect_volume.assert_called_once_with(
+            connection_info['data'], None)
+
+    def test_libvirt_rbd_driver_block_config(self):
+        self.flags(rbd_volume_local_attach=True, group='workarounds')
+        connection_info = self.rbd_connection(self.vol)
+        connection_info['data']['device_path'] = '/dev/rbd0'
+        libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_host)
+        conf = libvirt_driver.get_config(connection_info, self.disk_info)
+
+        # Assert that the returned config is for a RBD block device
+        self.assertEqual('block', conf.source_type)
+        self.assertEqual('/dev/rbd0', conf.source_path)
+        self.assertEqual('native', conf.driver_io)
+
+    def test_libvirt_rbd_driver_block_extend(self):
+        self.flags(rbd_volume_local_attach=True, group='workarounds')
+        connection_info = self.rbd_connection(self.vol)
+        libvirt_driver = net.LibvirtNetVolumeDriver(self.fake_host)
+
+        # Assert NotImplementedError is raised for extend_volume
+        self.assertRaises(NotImplementedError, libvirt_driver.extend_volume,
+                          connection_info, mock.sentinel.instance,
+                          mock.sentinel.requested_size)
diff --git a/nova/virt/libvirt/volume/net.py b/nova/virt/libvirt/volume/net.py
index 1164395a4b..ef065c8f5a 100644
--- a/nova/virt/libvirt/volume/net.py
+++ b/nova/virt/libvirt/volume/net.py
@@ -10,9 +10,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from os_brick import exception as os_brick_exception
+from os_brick import initiator
+from os_brick.initiator import connector
 from oslo_log import log as logging
 
 import nova.conf
+from nova import utils
 from nova.virt.libvirt.volume import volume as libvirt_volume
 
 
@@ -25,6 +29,10 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
     def __init__(self, host):
         super(LibvirtNetVolumeDriver,
               self).__init__(host, is_block_dev=False)
+        self.connector = None
+        if CONF.workarounds.rbd_volume_local_attach:
+            self.connector = connector.InitiatorConnector.factory(
+                initiator.RBD, utils.get_root_helper(), do_local_attach=True)
 
     def _set_auth_config_rbd(self, conf, netdisk_properties):
         # The rbd volume driver in cinder sets auth_enabled if the rbd_user is
@@ -67,11 +75,37 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
             # secret_type is always hard-coded to 'ceph' in cinder
             conf.auth_secret_type = netdisk_properties['secret_type']
 
-    def get_config(self, connection_info, disk_info):
-        """Returns xml for libvirt."""
-        conf = super(LibvirtNetVolumeDriver,
-                     self).get_config(connection_info, disk_info)
+    def _use_rbd_volume_local_attach(self, connection_info):
+        return (connection_info['driver_volume_type'] == 'rbd' and
+            CONF.workarounds.rbd_volume_local_attach)
 
+    def connect_volume(self, connection_info, instance):
+        if self._use_rbd_volume_local_attach(connection_info):
+            LOG.debug("Calling os-brick to attach RBD Volume as block device",
+                      instance=instance)
+            device_info = self.connector.connect_volume(
+                connection_info['data'])
+            LOG.debug("Attached RBD volume %s", device_info, instance=instance)
+            connection_info['data']['device_path'] = device_info['path']
+
+    def disconnect_volume(self, connection_info, instance):
+        if self._use_rbd_volume_local_attach(connection_info):
+            LOG.debug("calling os-brick to detach RBD Volume",
+                      instance=instance)
+            try:
+                self.connector.disconnect_volume(connection_info['data'], None)
+            except os_brick_exception.VolumeDeviceNotFound as exc:
+                LOG.warning('Ignoring VolumeDeviceNotFound: %s', exc)
+                return
+            LOG.debug("Disconnected RBD Volume", instance=instance)
+
+    def _get_block_config(self, conf, connection_info):
+        conf.source_type = "block"
+        conf.source_path = connection_info['data']['device_path']
+        conf.driver_io = "native"
+        return conf
+
+    def _get_net_config(self, conf, connection_info):
         netdisk_properties = connection_info['data']
         conf.source_type = "network"
         conf.source_protocol = connection_info['driver_volume_type']
@@ -82,8 +116,18 @@ class LibvirtNetVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver):
             self._set_auth_config_rbd(conf, netdisk_properties)
         return conf
 
+    def get_config(self, connection_info, disk_info):
+        """Returns xml for libvirt."""
+        conf = super(LibvirtNetVolumeDriver,
+                     self).get_config(connection_info, disk_info)
+        if self._use_rbd_volume_local_attach(connection_info):
+            return self._get_block_config(conf, connection_info)
+        return self._get_net_config(conf, connection_info)
+
     def extend_volume(self, connection_info, instance, requested_size):
-        # There is nothing to do for network volumes. Cinder already extended
-        # the volume and there is no local block device which needs to be
-        # refreshed.
+        if self._use_rbd_volume_local_attach(connection_info):
+            raise NotImplementedError
+        # There is nothing to do for network volumes. Cinder already
+        # extended the volume and there is no local block device which
+        # needs to be refreshed.
         return requested_size
diff --git a/releasenotes/notes/workarounds-libvirt-rbd-host-block-devices-ca5e3c187342ab4d.yaml b/releasenotes/notes/workarounds-libvirt-rbd-host-block-devices-ca5e3c187342ab4d.yaml
new file mode 100644
index 0000000000..eb740d0ada
--- /dev/null
+++ b/releasenotes/notes/workarounds-libvirt-rbd-host-block-devices-ca5e3c187342ab4d.yaml
@@ -0,0 +1,23 @@
+---
+other:
+  - |
+    The ``[workarounds]/rbd_volume_local_attach`` configuration option has been
+    introduced. This can be used by operators to ensure RBD volumes are
+    connected to compute hosts as block devices. This can be used with
+    the ``[worarounds]/disable_native_luksv1`` configuration option to
+    workaround recently discovered performance issues found within the
+    `libgcrypt library`__ used by QEMU when natively decrypting LUKSv1
+    encrypted disks.
+
+    This workaround does not currently support extending attached volumes.
+
+    This workaround is temporary and will be removed during the W release once
+    all impacted distributions have been able to update their versions of the
+    libgcrypt library.
+
+    .. warning:: Operators must ensure no instances are running on the compute
+      host before enabling this workaround. Any instances with attached RBD
+      volumes left running on the hosts will fail to migrate or stop after this
+      workaround has been enabled.
+
+    .. __: https://bugzilla.redhat.com/show_bug.cgi?id=1762765
diff --git a/requirements.txt b/requirements.txt
index ec11b519eb..3dd1a0859f 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -53,7 +53,7 @@ rfc3986>=1.1.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>=2.6.2 # Apache-2.0
+os-brick>=3.0.1 # Apache-2.0
 os-resource-classes>=0.4.0 # Apache-2.0
 os-traits>=2.2.0 # Apache-2.0
 os-vif>=1.14.0 # Apache-2.0