diff --git a/nova/exception.py b/nova/exception.py index 0be8f0f95934..48d9cc374dea 100644 --- a/nova/exception.py +++ b/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") diff --git a/nova/tests/unit/virt/libvirt/volume/test_quobyte.py b/nova/tests/unit/virt/libvirt/volume/test_quobyte.py index e84b9472c342..6eb256518d4e 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_quobyte.py +++ b/nova/tests/unit/virt/libvirt/volume/test_quobyte.py @@ -203,7 +203,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 @@ -220,6 +220,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): @@ -293,13 +314,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') @@ -313,6 +336,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) @@ -324,12 +350,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_is_mounted, - mock_umount_volume, + def test_libvirt_quobyte_driver_umount(self, mock_umount_volume, mock_validate_volume): mnt_base = '/mnt' self.flags(quobyte_mount_point_base=mnt_base, group='libvirt') @@ -352,7 +378,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') @@ -385,15 +413,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_is_mounted, - mock_mount_volume, - mock_validate_volume - ): + def test_libvirt_quobyte_driver_qcow2(self, mock_mount_volume, + mock_validate_volume, mock_umount): mnt_base = '/mnt' self.flags(quobyte_mount_point_base=mnt_base, group='libvirt') libvirt_driver = quobyte.LibvirtQuobyteVolumeDriver(self.fake_host) @@ -408,6 +435,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) @@ -423,6 +452,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, diff --git a/nova/virt/libvirt/volume/quobyte.py b/nova/virt/libvirt/volume/quobyte.py index e0885931cdc3..cd04eea141a6 100644 --- a/nova/virt/libvirt/volume/quobyte.py +++ b/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 @@ -26,7 +25,6 @@ import nova.conf from nova import exception as nova_exception from nova.i18n import _ from nova import utils -from nova.virt.libvirt import utils as libvirt_utils from nova.virt.libvirt.volume import fs LOG = logging.getLogger(__name__) @@ -75,12 +73,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: @@ -90,10 +91,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" - " Quobyte volume according to partition list.") + msg = (_("The mount %(mount_path)s is not a valid " + "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" @@ -128,39 +129,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, - SOURCE_PROTOCOL - + '@' + quobyte_volume) - if mounted: - try: - os.stat(mount_path) - except OSError as exc: - if exc.errno == errno.ENOTCONN: - mounted = False - LOG.info('Fixing previous mount %s which was not' - ' unmounted correctly.', mount_path) - umount_volume(mount_path) + try: + validate_volume(mount_path) + mounted = True + except nova_exception.StaleVolumeMount: + mounted = False + LOG.info('Fixing previous mount %s which was not ' + 'unmounted correctly.', mount_path) + umount_volume(mount_path) + except nova_exception.InvalidVolume: + mounted = False 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) - - if libvirt_utils.is_mounted(mount_path, 'quobyte@' + quobyte_volume): - umount_volume(mount_path) + try: + validate_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", - mount_path) + umount_volume(mount_path) def _normalize_export(self, export): protocol = SOURCE_PROTOCOL + "://" diff --git a/releasenotes/notes/qb-bug-1730933-6695470ebaee0fbd.yaml b/releasenotes/notes/qb-bug-1730933-6695470ebaee0fbd.yaml new file mode 100644 index 000000000000..758cb98a355b --- /dev/null +++ b/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.