Merge "Fixed copy-on-write mode in GPFS NFS driver"

3 files changed, 62 insertions, 18 deletions

diff --git a/cinder/tests/unit/volume/drivers/test_gpfs.py b/cinder/tests/unit/volume/drivers/test_gpfs.py
index 6ffea6116..67666c6ec 100644
--- a/cinder/tests/unit/volume/drivers/test_gpfs.py
+++ b/cinder/tests/unit/volume/drivers/test_gpfs.py
@@ -154,6 +154,25 @@ class GPFSDriverTestCase(test.TestCase):
                           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      '
                                   'Used Available Use%% Mounted on\n'
@@ -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 55c4eb222..aa9ea89eb 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 000000000..866cb820f
--- /dev/null
+++ b/releasenotes/notes/bug-gpfs-fix-nfs-cow.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    `Bug #1947134 <https://bugs.launchpad.net/cinder/+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 <https://bugs.launchpad.net/cinder/+bug/1947123>`_: Fixed
+    the volume creation issue in GPFS NFS driver when gpfs_images_share_mode
+    is set to copy_on_write.