RemoteFS: revert snapshot support

This change implements the 'revert_to_snapshot' method at the remotefs
base driver level in a generic way. This should work right away with
most (if not all) remotefs based drivers.

Note that we only consider the case in which the latest snapshot is
being reverted (the Cinder API won't allow reverting volumes to other
snapshots anyway, at least for the time being).

The RemoteFS based drivers (e.g. NFS driver, SMB driver) use COW images
(e.g. qcow2, differencing vhdx) in order to provide volume snapshot
functionality. Reverting a volume to its latest snapshot is a trivial
operation in this case and can be achived by simply cleaning up (e.g.
recreating) the latest image from chain.

In order to avoid breaking drivers, the 'revert_to_snapshot' method
is implemented in a mixin that may be inherited by any RemoteFS based
driver.

Implements: blueprint remotefs-revert-snapshot

Change-Id: Id66d3f0fe36cf7c32ef3875bf5fcb1c7a4678273

cinder/tests/unit/volume/drivers/test_remotefs.py

@@ -702,3 +702,94 @@ class RemoteFSPoolMixinTestCase(test.TestCase):
             mock.sentinel.share)
         self._driver.configuration.safe_get.assert_called_once_with(
             'volume_backend_name')
+
+
+@ddt.ddt
+class RevertToSnapshotMixinTestCase(test.TestCase):
+
+    _FAKE_MNT_POINT = '/mnt/fake_hash'
+
+    def setUp(self):
+        super(RevertToSnapshotMixinTestCase, self).setUp()
+        self._driver = remotefs.RevertToSnapshotMixin()
+        self._driver._remotefsclient = mock.Mock()
+        self._driver._execute = mock.Mock()
+        self._driver._delete = mock.Mock()
+
+        self.context = context.get_admin_context()
+
+        self._fake_volume = fake_volume.fake_volume_obj(
+            self.context, provider_location='fake_share')
+        self._fake_volume_path = os.path.join(self._FAKE_MNT_POINT,
+                                              self._fake_volume.name)
+        self._fake_snapshot = fake_snapshot.fake_snapshot_obj(self.context)
+        self._fake_snapshot_path = (self._fake_volume_path + '.' +
+                                    self._fake_snapshot.id)
+        self._fake_snapshot_name = os.path.basename(
+            self._fake_snapshot_path)
+        self._fake_snapshot.volume = self._fake_volume
+
+    @ddt.data(True, False)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin, '_validate_state',
+                       create=True)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin, '_read_info_file',
+                       create=True)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin,
+                       '_local_path_volume_info', create=True)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin, '_qemu_img_info',
+                       create=True)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin, '_do_create_snapshot',
+                       create=True)
+    @mock.patch.object(remotefs.RevertToSnapshotMixin, '_local_volume_dir',
+                       create=True)
+    def test_revert_to_snapshot(self,
+                                is_latest_snapshot,
+                                mock_local_vol_dir,
+                                mock_do_create_snapshot,
+                                mock_qemu_img_info,
+                                mock_local_path_vol_info,
+                                mock_read_info_file,
+                                mock_validate_state):
+
+        active_file = (self._fake_snapshot_name if is_latest_snapshot
+                       else 'fake_latest_snap')
+        fake_snapshot_info = {
+            'active': active_file,
+            self._fake_snapshot.id: self._fake_snapshot_name
+        }
+
+        mock_read_info_file.return_value = fake_snapshot_info
+
+        fake_snap_img_info = mock.Mock()
+        fake_snap_img_info.backing_file = self._fake_volume.name
+
+        mock_qemu_img_info.return_value = fake_snap_img_info
+        mock_local_vol_dir.return_value = self._FAKE_MNT_POINT
+
+        if is_latest_snapshot:
+            self._driver._revert_to_snapshot(self.context, self._fake_volume,
+                                             self._fake_snapshot)
+            self._driver._delete.assert_called_once_with(
+                self._fake_snapshot_path)
+            mock_do_create_snapshot.assert_called_once_with(
+                self._fake_snapshot,
+                fake_snap_img_info.backing_file,
+                self._fake_snapshot_path)
+            mock_qemu_img_info.assert_called_once_with(
+                self._fake_snapshot_path,
+                self._fake_volume.name)
+        elif not is_latest_snapshot:
+            self.assertRaises(exception.InvalidSnapshot,
+                              self._driver._revert_to_snapshot,
+                              self.context, self._fake_volume,
+                              self._fake_snapshot)
+            self._driver._delete.assert_not_called()
+
+        exp_acceptable_states = ['available', 'reverting']
+        mock_validate_state.assert_called_once_with(
+            self._fake_snapshot.volume.status,
+            exp_acceptable_states)
+        mock_local_path_vol_info.assert_called_once_with(
+            self._fake_snapshot.volume)
+        mock_read_info_file.assert_called_once_with(
+            mock_local_path_vol_info.return_value)

cinder/tests/unit/volume/test_volume.py

@@ -1771,8 +1771,13 @@ class VolumeTestCase(base.BaseVolumeTestCase):
             if driver_error:
                 generic_revert.assert_called_once_with(self.context, {}, {})
 
-    @ddt.data(True, False)
-    def test_revert_to_snapshot(self, has_snapshot):
+    @ddt.data({},
+              {'has_snapshot': True},
+              {'use_temp_snapshot': True},
+              {'use_temp_snapshot': True, 'has_snapshot': True})
+    @ddt.unpack
+    def test_revert_to_snapshot(self, has_snapshot=False,
+                                use_temp_snapshot=False):
         fake_volume = tests_utils.create_volume(self.context,
                                                 status='reverting',
                                                 project_id='123',
@@ -1786,8 +1791,13 @@ class VolumeTestCase(base.BaseVolumeTestCase):
             mock.patch.object(self.volume,
                               '_create_backup_snapshot') as _create_snapshot,\
             mock.patch.object(self.volume,
-                              'delete_snapshot') as _delete_snapshot:
+                              'delete_snapshot') as _delete_snapshot, \
+            mock.patch.object(self.volume.driver,
+                              'snapshot_revert_use_temp_snapshot') as \
+                _use_temp_snap:
             _revert.return_value = None
+            _use_temp_snap.return_value = use_temp_snapshot
+
             if has_snapshot:
                 _create_snapshot.return_value = {'id': 'fake_snapshot'}
             else:
@@ -1796,12 +1806,19 @@ class VolumeTestCase(base.BaseVolumeTestCase):
                                            fake_snapshot)
             _revert.assert_called_once_with(self.context, fake_volume,
                                             fake_snapshot)
-            _create_snapshot.assert_called_once_with(self.context, fake_volume)
-            if has_snapshot:
+
+            if not use_temp_snapshot:
+                _create_snapshot.assert_not_called()
+            else:
+                _create_snapshot.assert_called_once_with(self.context,
+                                                         fake_volume)
+
+            if use_temp_snapshot and has_snapshot:
                 _delete_snapshot.assert_called_once_with(
                     self.context, {'id': 'fake_snapshot'}, handle_quota=False)
             else:
                 _delete_snapshot.assert_not_called()
+
             fake_volume.refresh()
             fake_snapshot.refresh()
             self.assertEqual('available', fake_volume['status'])

cinder/volume/driver.py

@@ -1102,6 +1102,13 @@ class BaseVD(object):
         """
         return self.configuration.safe_get("backup_use_temp_snapshot")
 
+    def snapshot_revert_use_temp_snapshot(self):
+        # Specify whether a temporary backup snapshot should be used when
+        # reverting a snapshot. For some backends, this operation is not
+        # needed or not supported, in which case the driver should override
+        # this method.
+        return True
+
     def snapshot_remote_attachable(self):
         # TODO(lixiaoy1): the method will be deleted later when remote
         # attach snapshot is implemented.

cinder/volume/drivers/remotefs.py

@@ -717,6 +717,13 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
 
         self._nova = compute.API()
 
+    def snapshot_revert_use_temp_snapshot(self):
+        # Considering that RemoteFS based drivers use COW images
+        # for storing snapshots, having chains of such images,
+        # creating a backup snapshot when reverting one is not
+        # actutally helpful.
+        return False
+
     def _local_volume_dir(self, volume):
         share = volume.provider_location
         local_dir = self._get_mount_point_for_share(share)
@@ -1607,6 +1614,9 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
     def _extend_volume(self, volume, size_gb):
         raise NotImplementedError()
 
+    def _revert_to_snapshot(self, context, volume, snapshot):
+        raise NotImplementedError()
+
 
 class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
     @locked_volume_id_operation
@@ -1642,6 +1652,12 @@ class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
     def extend_volume(self, volume, size_gb):
         return self._extend_volume(volume, size_gb)
 
+    @locked_volume_id_operation
+    def revert_to_snapshot(self, context, volume, snapshot):
+        """Revert to specified snapshot."""
+
+        return self._revert_to_snapshot(context, volume, snapshot)
+
 
 class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
     @coordination.synchronized('{self.driver_prefix}-{snapshot.volume.id}')
@@ -1677,6 +1693,12 @@ class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
     def extend_volume(self, volume, size_gb):
         return self._extend_volume(volume, size_gb)
 
+    @coordination.synchronized('{self.driver_prefix}-{volume.id}')
+    def revert_to_snapshot(self, context, volume, snapshot):
+        """Revert to specified snapshot."""
+
+        return self._revert_to_snapshot(context, volume, snapshot)
+
 
 class RemoteFSPoolMixin(object):
     """Drivers inheriting this will report each share as a pool."""
@@ -1737,3 +1759,48 @@ class RemoteFSPoolMixin(object):
         data['pools'] = pools
 
         self._stats = data
+
+
+class RevertToSnapshotMixin(object):
+
+    def _revert_to_snapshot(self, context, volume, snapshot):
+        """Revert a volume to specified snapshot
+
+        The volume must not be attached. Only the latest snapshot
+        can be used.
+        """
+        status = snapshot.volume.status
+        acceptable_states = ['available', 'reverting']
+
+        self._validate_state(status, acceptable_states)
+
+        LOG.debug('Reverting volume %(vol)s to snapshot %(snap)s',
+                  {'vol': snapshot.volume.id, 'snap': snapshot.id})
+
+        info_path = self._local_path_volume_info(snapshot.volume)
+        snap_info = self._read_info_file(info_path)
+
+        snapshot_file = snap_info[snapshot.id]
+        active_file = snap_info['active']
+
+        if not utils.paths_normcase_equal(snapshot_file, active_file):
+            msg = _("Could not revert volume '%(volume_id)s' to snapshot "
+                    "'%(snapshot_id)s' as it does not "
+                    "appear to be the latest snapshot. Current active "
+                    "image: %(active_file)s.")
+            raise exception.InvalidSnapshot(
+                msg % dict(snapshot_id=snapshot.id,
+                           active_file=active_file,
+                           volume_id=volume.id))
+
+        snapshot_path = os.path.join(
+            self._local_volume_dir(snapshot.volume), snapshot_file)
+        backing_filename = self._qemu_img_info(
+            snapshot_path, volume.name).backing_file
+
+        # We revert the volume to the latest snapshot by recreating the top
+        # image from the chain.
+        # This workflow should work with most (if not all) drivers inheriting
+        # this class.
+        self._delete(snapshot_path)
+        self._do_create_snapshot(snapshot, backing_filename, snapshot_path)

cinder/volume/manager.py

@@ -924,7 +924,10 @@ class VolumeManager(manager.CleanableManager,
             LOG.info("Start to perform revert to snapshot process.")
             # Create a snapshot which can be used to restore the volume
             # data by hand if revert process failed.
-            backup_snapshot = self._create_backup_snapshot(context, volume)
+
+            if self.driver.snapshot_revert_use_temp_snapshot():
+                backup_snapshot = self._create_backup_snapshot(context,
+                                                               volume)
             self._revert_to_snapshot(context, volume, snapshot)
         except Exception as error:
             with excutils.save_and_reraise_exception():