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.