From dcc191646985fa0be76ee763935ef37a8456d1e6 Mon Sep 17 00:00:00 2001 From: digvijay2016 Date: Thu, 14 Oct 2021 07:24:18 -0400 Subject: [PATCH] Fixed copy-on-write mode in GPFS NFS driver IBM Spectrum Scale cinder driver (GPFS) support copy-on-write feature in all the configuration. Resolving the bug mentioned below will enable mmclone feature of the IBM Spectrum Scale filesystem to provide better performance while configured in GPFS NFS mode. Closes-Bug: #1947134 Closes-Bug: #1947123 Change-Id: I3e77c890c7abca85dab92500eae989b4dff9824d --- cinder/tests/unit/volume/drivers/test_gpfs.py | 40 +++++++++++++++---- cinder/volume/drivers/ibm/gpfs.py | 30 +++++++++----- releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml | 10 +++++ 3 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml diff --git a/cinder/tests/unit/volume/drivers/test_gpfs.py b/cinder/tests/unit/volume/drivers/test_gpfs.py index 6ffea61165c..67666c6ec15 100644 --- a/cinder/tests/unit/volume/drivers/test_gpfs.py +++ b/cinder/tests/unit/volume/drivers/test_gpfs.py @@ -153,6 +153,25 @@ class GPFSDriverTestCase(test.TestCase): self.assertRaises(exception.VolumeBackendAPIException, self.driver._check_gpfs_state) + @mock.patch('cinder.utils.execute') + def test_same_filesystem_ok(self, mock_exec): + # returns filesystem id in hex + mock_exec.return_value = ('ef0009600000002\nef0009600000002\n', '') + self.assertTrue(self.driver._same_filesystem('/path1', '/path2')) + + @mock.patch('cinder.utils.execute') + def test_same_filesystem_not_ok(self, mock_exec): + # returns filesystem id in hex + mock_exec.return_value = ('ef0009600000002\n000000000000007\n', '') + self.assertFalse(self.driver._same_filesystem('/path1', '/path2')) + + @mock.patch('cinder.utils.execute') + def test_same_filesystem_failed(self, mock_exec): + mock_exec.side_effect = processutils.ProcessExecutionError( + stdout='test', stderr='test') + self.assertRaises(exception.VolumeBackendAPIException, + self.driver._same_filesystem, '', '') + @mock.patch('cinder.utils.execute') def test_get_fs_from_path_ok(self, mock_exec): mock_exec.return_value = ('Filesystem 1K-blocks ' @@ -559,12 +578,12 @@ class GPFSDriverTestCase(test.TestCase): conf.SHARED_CONF_GROUP) # fail configuration.gpfs_images_share_mode == 'copy_on_write' and not - # _same_filesystem(configuration.gpfs_mount_point_base, + # self._same_filesystem(configuration.gpfs_mount_point_base, # configuration.gpfs_images_dir) self.override_config('gpfs_images_share_mode', 'copy_on_write', conf.SHARED_CONF_GROUP) - with mock.patch('cinder.volume.drivers.ibm.gpfs._same_filesystem', - return_value=False): + with mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.' + '_same_filesystem', return_value=False): self.assertRaises(exception.VolumeBackendAPIException, self.driver.check_for_setup_error) @@ -1281,21 +1300,22 @@ class GPFSDriverTestCase(test.TestCase): @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.' '_is_gpfs_parent_file') - @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path') + @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.' + '_get_volume_path') @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._is_cloneable') @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.' '_verify_gpfs_path_state') def test_clone_image_format_raw_copy(self, mock_verify_gpfs_path_state, mock_is_cloneable, - mock_local_path, + mock_get_volume_path, mock_is_gpfs_parent_file, mock_qemu_img_info, mock_copyfile, mock_set_rw_permission, mock_resize_volume_file): mock_is_cloneable.return_value = (True, 'test', self.images_dir) - mock_local_path.return_value = self.volumes_path + mock_get_volume_path.return_value = self.volumes_path mock_qemu_img_info.return_value = self._fake_qemu_raw_image_info('') volume = self._fake_volume() org_value = self.driver.configuration.gpfs_images_share_mode @@ -2255,7 +2275,11 @@ class GPFSNFSDriverTestCase(test.TestCase): @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type') @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSNFSDriver.' '_get_mount_point_for_share') - def test_local_path(self, mock_mount_point, + @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSNFSDriver.' + '_find_share') + def test_local_path(self, + mock_find_share, + mock_mount_point, mock_group_cg_snapshot_type, mock_group): mock_mount_point.return_value = self.TEST_MNT_POINT_BASE @@ -2263,7 +2287,7 @@ class GPFSNFSDriverTestCase(test.TestCase): volume = self._fake_volume() group = self._fake_group() mock_group.return_value = group - volume['provider_location'] = self.TEST_MNT_POINT_BASE + mock_find_share.return_value = self.TEST_VOLUME_PATH local_volume_path_in_cg = os.path.join(self.TEST_MNT_POINT_BASE, 'consisgroup-' + fake.CONSISTENCY_GROUP_ID, diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py index 55c4eb22227..aa9ea89ebe5 100644 --- a/cinder/volume/drivers/ibm/gpfs.py +++ b/cinder/volume/drivers/ibm/gpfs.py @@ -134,11 +134,6 @@ def _different(difference_tuple): return False -def _same_filesystem(path1, path2): - """Return true if the two paths are in the same GPFS file system.""" - return os.lstat(path1).st_dev == os.lstat(path2).st_dev - - def _sizestr(size_in_g): """Convert the specified size into a string value.""" return '%sG' % size_in_g @@ -204,6 +199,19 @@ class GPFSDriver(driver.CloneableImageVD, raise exception.VolumeBackendAPIException( data=_('GPFS is not running, state: %s.') % gpfs_state) + def _same_filesystem(self, path1, path2): + """Return true if the two paths are in the same GPFS file system.""" + try: + (out, err) = self.gpfs_execute('stat', '-f', '-c', '"%i"', + path1, path2) + lines = out.splitlines() + return lines[0] == lines[1] + except processutils.ProcessExecutionError as exc: + LOG.error('Failed to issue stat command on path ' + '%(path1)s and path %(path2)s, error: %(error)s', + {'path1': path1, 'path2': path2, 'error': exc.stderr}) + raise exception.VolumeBackendAPIException(data=exc.stderr) + def _get_filesystem_from_path(self, path): """Return filesystem for specified path.""" try: @@ -435,8 +443,8 @@ class GPFSDriver(driver.CloneableImageVD, raise exception.VolumeBackendAPIException(data=msg) if(self.configuration.gpfs_images_share_mode == 'copy_on_write' and - not _same_filesystem(self.configuration.gpfs_mount_point_base, - self.configuration.gpfs_images_dir)): + not self._same_filesystem(self.configuration.gpfs_mount_point_base, + self.configuration.gpfs_images_dir)): msg = (_('gpfs_images_share_mode is set to copy_on_write, but ' '%(vol)s and %(img)s belong to different file ' 'systems.') % @@ -907,13 +915,13 @@ class GPFSDriver(driver.CloneableImageVD, {'img': image_id, 'reas': reason}) return (None, False) - vol_path = self.local_path(volume) - data = image_utils.qemu_img_info(image_path) # if image format is already raw either clone it or # copy it depending on config file settings + # GPFS command (mmclone) needs to run on GPFS node on GPFS path if data.file_format == 'raw': + vol_path = self._get_volume_path(volume) if (self.configuration.gpfs_images_share_mode == 'copy_on_write'): LOG.debug('Clone image to vol %s using mmclone.', @@ -929,7 +937,9 @@ class GPFSDriver(driver.CloneableImageVD, shutil.copyfile(image_path, vol_path) # if image is not raw convert it to raw into vol_path destination + # Image conversion can be run locally on GPFS mount path else: + vol_path = self.local_path(volume) LOG.debug('Clone image to vol %s using qemu convert.', volume['id']) image_utils.convert_image(image_path, vol_path, 'raw') @@ -1587,7 +1597,7 @@ class GPFSNFSDriver(GPFSDriver, nfs.NfsDriver, san.SanDriver): def local_path(self, volume): """Returns the local path for the specified volume.""" - remotefs_share = volume['provider_location'] + remotefs_share = self._find_share(volume) base_local_path = self._get_mount_point_for_share(remotefs_share) # Check if the volume is part of a consistency group and return diff --git a/releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml b/releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml new file mode 100644 index 00000000000..866cb820fee --- /dev/null +++ b/releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + `Bug #1947134 `_: Fixed + the initialization of GPFS NFS driver when gpfs_images_share_mode + is set to copy_on_write by correcting _same_filesystem functionality. + - | + `Bug #1947123 `_: Fixed + the volume creation issue in GPFS NFS driver when gpfs_images_share_mode + is set to copy_on_write.