Merge "Added mount fstype based validation of Quobyte mounts"

nova/exception.py

@@ -354,6 +354,10 @@ class InvalidVolumeAccessMode(Invalid):
     msg_fmt = _("Invalid volume access mode: %(access_mode)s")
 
 
+class StaleVolumeMount(InvalidVolume):
+    msg_fmt = _("The volume mount at %(mount_path)s is unusable.")
+
+
 class InvalidMetadata(Invalid):
     msg_fmt = _("Invalid metadata: %(reason)s")
 

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

@@ -217,7 +217,7 @@ class QuobyteTestCase(test.NoDBTestCase):
 
     @mock.patch.object(psutil, "disk_partitions")
     @mock.patch.object(os, "stat")
-    def test_validate_volume_all_good(self, stat_mock, part_mock):
+    def test_validate_volume_all_good_prefix_val(self, stat_mock, part_mock):
         part_mock.return_value = self.get_mock_partitions()
         drv = quobyte
 
@@ -234,6 +234,27 @@ class QuobyteTestCase(test.NoDBTestCase):
         stat_mock.assert_called_once_with(self.TEST_MNT_POINT)
         part_mock.assert_called_once_with(all=True)
 
+    @mock.patch.object(psutil, "disk_partitions")
+    @mock.patch.object(os, "stat")
+    def test_validate_volume_all_good_fs_type(self, stat_mock, part_mock):
+        part_mock.return_value = self.get_mock_partitions()
+        part_mock.return_value[0].device = "not_quobyte"
+        part_mock.return_value[0].fstype = "fuse.quobyte"
+        drv = quobyte
+
+        def statMockCall(*args):
+            if args[0] == self.TEST_MNT_POINT:
+                stat_result = mock.Mock()
+                stat_result.st_size = 0
+                return stat_result
+            return os.stat(args)
+        stat_mock.side_effect = statMockCall
+
+        drv.validate_volume(self.TEST_MNT_POINT)
+
+        stat_mock.assert_called_once_with(self.TEST_MNT_POINT)
+        part_mock.assert_called_once_with(all=True)
+
     @mock.patch.object(psutil, "disk_partitions")
     @mock.patch.object(os, "stat")
     def test_validate_volume_mount_not_working(self, stat_mock, part_mock):
@@ -307,13 +328,15 @@ class LibvirtQuobyteVolumeDriverTestCase(
         test_volume.LibvirtVolumeBaseTestCase):
     """Tests the LibvirtQuobyteVolumeDriver class."""
 
+    @mock.patch.object(quobyte, 'umount_volume')
     @mock.patch.object(quobyte, 'validate_volume')
     @mock.patch.object(quobyte, 'mount_volume')
     @mock.patch.object(libvirt_utils, 'is_mounted', return_value=False)
     def test_libvirt_quobyte_driver_mount(self,
                                           mock_is_mounted,
                                           mock_mount_volume,
-                                          mock_validate_volume
+                                          mock_validate_volume,
+                                          mock_umount_volume
                                           ):
         mnt_base = '/mnt'
         self.flags(quobyte_mount_point_base=mnt_base, group='libvirt')
@@ -327,6 +350,9 @@ class LibvirtQuobyteVolumeDriverTestCase(
 
         connection_info = {'data': {'export': export_string,
                                     'name': self.name}}
+        mock_validate_volume.side_effect = [nova_exception.StaleVolumeMount(
+            "This shall fail."), True, True]
+
         libvirt_driver.connect_volume(connection_info, mock.sentinel.instance)
 
         conf = libvirt_driver.get_config(connection_info, self.disk_info)
@@ -338,12 +364,12 @@ class LibvirtQuobyteVolumeDriverTestCase(
                                                   export_mnt_base,
                                                   mock.ANY)
         mock_validate_volume.assert_called_with(export_mnt_base)
+        mock_umount_volume.assert_called_once_with(
+            libvirt_driver._get_mount_path(connection_info))
 
-    @mock.patch.object(quobyte, 'validate_volume')
+    @mock.patch.object(quobyte, 'validate_volume', return_value=True)
     @mock.patch.object(quobyte, 'umount_volume')
-    @mock.patch.object(libvirt_utils, 'is_mounted', return_value=True)
+    def test_libvirt_quobyte_driver_umount(self, mock_umount_volume,
-    def test_libvirt_quobyte_driver_umount(self, mock_is_mounted,
-                                           mock_umount_volume,
                                            mock_validate_volume):
         mnt_base = '/mnt'
         self.flags(quobyte_mount_point_base=mnt_base, group='libvirt')
@@ -366,7 +392,9 @@ class LibvirtQuobyteVolumeDriverTestCase(
         libvirt_driver.disconnect_volume(connection_info,
                                          mock.sentinel.instance)
 
-        mock_validate_volume.assert_called_once_with(export_mnt_base)
+        mock_validate_volume.assert_has_calls([mock.call(export_mnt_base),
+                                               mock.call(export_mnt_base),
+                                               mock.call(export_mnt_base)])
         mock_umount_volume.assert_called_once_with(export_mnt_base)
 
     @mock.patch.object(quobyte, 'validate_volume')
@@ -399,15 +427,14 @@ class LibvirtQuobyteVolumeDriverTestCase(
                                          mock.sentinel.instance)
 
         mock_umount_volume.assert_called_once_with(export_mnt_base)
-        mock_validate_volume.assert_called_once_with(export_mnt_base)
+        mock_validate_volume.assert_has_calls([mock.call(export_mnt_base),
+                                               mock.call(export_mnt_base)])
 
+    @mock.patch.object(quobyte, 'umount_volume')
     @mock.patch.object(quobyte, 'validate_volume')
     @mock.patch.object(quobyte, 'mount_volume')
-    @mock.patch.object(libvirt_utils, 'is_mounted', return_value=False)
+    def test_libvirt_quobyte_driver_qcow2(self, mock_mount_volume,
-    def test_libvirt_quobyte_driver_qcow2(self, mock_is_mounted,
+                                          mock_validate_volume, mock_umount):
-                                          mock_mount_volume,
-                                          mock_validate_volume
-                                          ):
         mnt_base = '/mnt'
         self.flags(quobyte_mount_point_base=mnt_base, group='libvirt')
         libvirt_driver = quobyte.LibvirtQuobyteVolumeDriver(self.fake_host)
@@ -422,6 +449,8 @@ class LibvirtQuobyteVolumeDriverTestCase(
 
         export_mnt_base = os.path.join(mnt_base,
                                        utils.get_hash_str(quobyte_volume))
+        mock_validate_volume.side_effect = [nova_exception.StaleVolumeMount(
+            "This shall fail."), True, True]
 
         libvirt_driver.connect_volume(connection_info, mock.sentinel.instance)
         conf = libvirt_driver.get_config(connection_info, self.disk_info)
@@ -437,6 +466,8 @@ class LibvirtQuobyteVolumeDriverTestCase(
 
         libvirt_driver.disconnect_volume(connection_info,
                                          mock.sentinel.instance)
+        mock_umount.assert_has_calls([mock.call(export_mnt_base),
+                                      mock.call(export_mnt_base)])
 
     @mock.patch.object(libvirt_utils, 'is_mounted', return_value=True)
     def test_libvirt_quobyte_driver_mount_non_quobyte_volume(self,

nova/virt/libvirt/volume/quobyte.py

@@ -13,7 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import errno
 import os
 
 from oslo_concurrency import processutils
@@ -27,7 +26,6 @@ from nova import exception as nova_exception
 from nova.i18n import _
 import nova.privsep.libvirt
 from nova import utils
-from nova.virt.libvirt import utils as libvirt_utils
 from nova.virt.libvirt.volume import fs
 
 LOG = logging.getLogger(__name__)
@@ -103,12 +101,15 @@ def umount_volume(mnt_base):
 
 
 def validate_volume(mount_path):
-    """Runs a number of tests to be sure this is a (working) Quobyte mount"""
+    """Determine if the volume is a valid Quobyte mount.
+
+    Runs a number of tests to be sure this is a (working) Quobyte mount
+    """
     partitions = psutil.disk_partitions(all=True)
     for p in partitions:
         if mount_path != p.mountpoint:
             continue
-        if p.device.startswith("quobyte@"):
+        if p.device.startswith("quobyte@") or p.fstype == "fuse.quobyte":
             statresult = os.stat(mount_path)
             # Note(kaisers): Quobyte always shows mount points with size 0
             if statresult.st_size == 0:
@@ -118,10 +119,10 @@ def validate_volume(mount_path):
                 msg = (_("The mount %(mount_path)s is not a "
                          "valid Quobyte volume. Stale mount?")
                        % {'mount_path': mount_path})
-            raise nova_exception.InvalidVolume(msg)
+            raise nova_exception.StaleVolumeMount(msg, mount_path=mount_path)
         else:
-            msg = (_("The mount %(mount_path)s is not a valid"
+            msg = (_("The mount %(mount_path)s is not a valid "
-                     " Quobyte volume according to partition list.")
+                     "Quobyte volume according to partition list.")
                    % {'mount_path': mount_path})
             raise nova_exception.InvalidVolume(msg)
     msg = (_("No matching Quobyte mount entry for %(mount_path)s"
@@ -164,39 +165,40 @@ class LibvirtQuobyteVolumeDriver(fs.LibvirtBaseFileSystemVolumeDriver):
         data = connection_info['data']
         quobyte_volume = self._normalize_export(data['export'])
         mount_path = self._get_mount_path(connection_info)
-        mounted = libvirt_utils.is_mounted(mount_path,
+        try:
-                                           SOURCE_PROTOCOL
+            validate_volume(mount_path)
-                                           + '@' + quobyte_volume)
+            mounted = True
-        if mounted:
+        except nova_exception.StaleVolumeMount:
-            try:
+            mounted = False
-                os.stat(mount_path)
+            LOG.info('Fixing previous mount %s which was not '
-            except OSError as exc:
+                     'unmounted correctly.', mount_path)
-                if exc.errno == errno.ENOTCONN:
+            umount_volume(mount_path)
-                    mounted = False
+        except nova_exception.InvalidVolume:
-                    LOG.info('Fixing previous mount %s which was not'
+            mounted = False
-                             ' unmounted correctly.', mount_path)
-                    umount_volume(mount_path)
 
         if not mounted:
             mount_volume(quobyte_volume,
                          mount_path,
                          CONF.libvirt.quobyte_client_cfg)
 
-        validate_volume(mount_path)
+        try:
+            validate_volume(mount_path)
+        except (nova_exception.InvalidVolume,
+                nova_exception.StaleVolumeMount) as nex:
+            LOG.error("Could not mount Quobyte volume: %s", nex)
 
     @utils.synchronized('connect_qb_volume')
     def disconnect_volume(self, connection_info, instance):
         """Disconnect the volume."""
 
-        quobyte_volume = self._normalize_export(
-                                        connection_info['data']['export'])
         mount_path = self._get_mount_path(connection_info)
-
+        try:
-        if libvirt_utils.is_mounted(mount_path, 'quobyte@' + quobyte_volume):
+            validate_volume(mount_path)
-            umount_volume(mount_path)
+        except (nova_exception.InvalidVolume,
+                nova_exception.StaleVolumeMount) as exc:
+            LOG.warning("Could not disconnect Quobyte volume mount: %s", exc)
         else:
-            LOG.info("Trying to disconnected unmounted volume at %s",
+            umount_volume(mount_path)
-                     mount_path)
 
     def _normalize_export(self, export):
         protocol = SOURCE_PROTOCOL + "://"

releasenotes/notes/qb-bug-1730933-6695470ebaee0fbd.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    The Quobyte Nova volume driver now supports identifying Quobyte
+    mounts via the mounts fstype field, which is used by Quobyte 2.x
+    clients. The previous behaviour is deprecated and may be removed
+    from the Quobyte clients in the future.
+fixes:
+  - |
+    Fixes a bug that caused Nova to fail on mounting Quobyte volumes
+    whose volume URL contained multiple registries.