VMware: Disallow snapshot of attached volume

The current implementation perform snapshot of attached volume.
This is invalid because when we take snapshot of the backing, the instance
continues pointing to the readonly delta and tries writing to it.
This patch will disallow snapshot creation of attached volume.

We have similar issue with delete of snapshot, when deleted the instance will
point to an invalid disk file, since that will get merged with its parent.
Hence any writes by the instance will fail with disk not found error.
This patch will ignore delete snapshot of attached volume.

Since we disallow snapshot of attached volume, we disallow linked clone of
attached source volume and also upload of attached volume.

Fixes bug: 1236891

Change-Id: I6a91eac8529836d1fe1de73bf6b28ec098aa7c54
(cherry picked from commit 2451a027bee50c834890f9e049fbaf4bfa911546)

diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py
index c974b200786f30a6bfa97467581d26ec0838181b..73854c41e57e8049172a3bf94e3145cdc6471654 100644 (file)
--- a/cinder/tests/test_vmware_vmdk.py
+++ b/cinder/tests/test_vmware_vmdk.py
@@ -66,13 +66,14 @@ class FakeMor(object):
 
 
 class FakeObject(object):
-    fields = {}
+    def __init__(self):
+        self._fields = {}
 
     def __setitem__(self, key, value):
-        self.fields[key] = value
+        self._fields[key] = value
 
     def __getitem__(self, item):
-        return self.fields[item]
+        return self._fields[item]
 
 
 class FakeManagedObjectReference(object):
@@ -588,6 +589,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         self._volumeops.get_hosts().AndReturn(retrieve_result)
         m.StubOutWithMock(self._driver, '_create_backing')
         volume = FakeObject()
+        volume['name'] = 'vol_name'
         backing = FakeMor('VirtualMachine', 'my_back')
         mux = self._driver._create_backing(volume, host1.obj)
         mux.AndRaise(error_util.VimException('Maintenance mode'))
@@ -836,6 +838,9 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.StubOutWithMock(self._volumeops, 'get_backing')
         snapshot = FakeObject()
         snapshot['volume_name'] = 'volume_name'
+        snapshot['name'] = 'snap_name'
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'available'
         self._volumeops.get_backing(snapshot['volume_name'])
 
         m.ReplayAll()
@@ -853,6 +858,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         snapshot['volume_name'] = 'volume_name'
         snapshot['name'] = 'snapshot_name'
         snapshot['display_description'] = 'snapshot_desc'
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'available'
         backing = FakeMor('VirtualMachine', 'my_back')
         self._volumeops.get_backing(snapshot['volume_name']).AndReturn(backing)
         m.StubOutWithMock(self._volumeops, 'create_snapshot')
@@ -864,6 +871,15 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.UnsetStubs()
         m.VerifyAll()
 
+    def test_create_snapshot_when_attached(self):
+        """Test vmdk.create_snapshot when volume is attached."""
+        snapshot = FakeObject()
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'in-use'
+
+        self.assertRaises(exception.InvalidVolume,
+                          self._driver.create_snapshot, snapshot)
+
     def test_get_snapshot_from_tree(self):
         """Test _get_snapshot_from_tree."""
         volops = volumeops.VMwareVolumeOps
@@ -946,6 +962,9 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.StubOutWithMock(self._volumeops, 'get_backing')
         snapshot = FakeObject()
         snapshot['volume_name'] = 'volume_name'
+        snapshot['name'] = 'snap_name'
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'available'
         self._volumeops.get_backing(snapshot['volume_name'])
 
         m.ReplayAll()
@@ -962,6 +981,9 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         snapshot = FakeObject()
         snapshot['name'] = 'snapshot_name'
         snapshot['volume_name'] = 'volume_name'
+        snapshot['name'] = 'snap_name'
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'available'
         backing = FakeMor('VirtualMachine', 'my_back')
         self._volumeops.get_backing(snapshot['volume_name']).AndReturn(backing)
         m.StubOutWithMock(self._volumeops, 'delete_snapshot')
@@ -973,6 +995,15 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.UnsetStubs()
         m.VerifyAll()
 
+    def test_delete_snapshot_when_attached(self):
+        """Test delete_snapshot when volume is attached."""
+        snapshot = FakeObject()
+        snapshot['volume'] = FakeObject()
+        snapshot['volume']['status'] = 'in-use'
+
+        self.assertRaises(exception.InvalidVolume,
+                          self._driver.delete_snapshot, snapshot)
+
     def test_create_cloned_volume_without_backing(self):
         """Test create_cloned_volume without a backing."""
         m = self.mox
@@ -981,6 +1012,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.StubOutWithMock(self._volumeops, 'get_backing')
         volume = FakeObject()
         volume['name'] = 'volume_name'
+        volume['status'] = 'available'
         src_vref = FakeObject()
         src_vref['name'] = 'src_volume_name'
         self._volumeops.get_backing(src_vref['name'])
@@ -1090,6 +1122,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         volume['name'] = 'volume_name'
         snapshot = FakeObject()
         snapshot['volume_name'] = 'volume_name'
+        snapshot['name'] = 'snap_name'
         self._volumeops.get_backing(snapshot['volume_name'])
 
         m.ReplayAll()
@@ -1342,6 +1375,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         image_meta['disk_format'] = 'novmdk'
         volume = FakeObject()
         volume['name'] = 'vol-name'
+        volume['status'] = 'available'
 
         m.ReplayAll()
         self.assertRaises(exception.ImageUnacceptable,
@@ -1351,6 +1385,20 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         m.UnsetStubs()
         m.VerifyAll()
 
+    def test_copy_volume_to_image_when_attached(self):
+        """Test copy_volume_to_image when volume is attached."""
+        m = self.mox
+        volume = FakeObject()
+        volume['status'] = 'in-use'
+
+        m.ReplayAll()
+        self.assertRaises(exception.InvalidVolume,
+                          self._driver.copy_volume_to_image,
+                          mox.IgnoreArg(), volume,
+                          mox.IgnoreArg(), mox.IgnoreArg())
+        m.UnsetStubs()
+        m.VerifyAll()
+
     def test_copy_volume_to_image_vmdk(self):
         """Test copy_volume_to_image for a valid vmdk disk format."""
         m = self.mox
@@ -1372,6 +1420,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         volume = FakeObject()
         volume['name'] = vol_name
         volume['project_id'] = project_id
+        volume['status'] = 'available'
         # volumeops.get_backing
         backing = FakeMor("VirtualMachine", "my_vm")
         m.StubOutWithMock(self._volumeops, 'get_backing')
@@ -1382,10 +1431,6 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase):
         vmdk_file_path = '[%s] %s' % (datastore_name, file_path)
         m.StubOutWithMock(self._volumeops, 'get_vmdk_path')
         self._volumeops.get_vmdk_path(backing).AndReturn(vmdk_file_path)
-        # volumeops.create_snapshot
-        snapshot_name = 'snapshot-%s' % image_id
-        m.StubOutWithMock(self._volumeops, 'create_snapshot')
-        self._volumeops.create_snapshot(backing, snapshot_name, None, True)
         tmp_vmdk = '[datastore1] %s.vmdk' % image_id
         # volumeops.get_host
         host = FakeMor('Host', 'my_host')
@@ -1752,6 +1797,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         backing = FakeMor('VirtualMachine', 'my_back')
         src_vref = FakeObject()
         src_vref['name'] = 'src_vol_name'
+        src_vref['status'] = 'available'
         self._volumeops.get_backing(src_vref['name']).AndReturn(backing)
         volume = FakeObject()
         volume['volume_type_id'] = None
@@ -1763,7 +1809,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         self._driver.create_cloned_volume(volume, src_vref)
         m.UnsetStubs()
 
-    def test_create_lined_cloned_volume_with_backing(self):
+    def test_create_linked_cloned_volume_with_backing(self):
         """Test create_cloned_volume with clone type - linked."""
         m = self.mox
         m.StubOutWithMock(self._driver.__class__, 'volumeops')
@@ -1772,6 +1818,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         backing = FakeMor('VirtualMachine', 'my_back')
         src_vref = FakeObject()
         src_vref['name'] = 'src_vol_name'
+        src_vref['status'] = 'available'
         self._volumeops.get_backing(src_vref['name']).AndReturn(backing)
         volume = FakeObject()
         volume['id'] = 'volume_id'
@@ -1790,4 +1837,25 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         m.ReplayAll()
         self._driver.create_cloned_volume(volume, src_vref)
         m.UnsetStubs()
+
+    def test_create_linked_cloned_volume_when_attached(self):
+        """Test create_cloned_volume linked clone when volume is attached."""
+        m = self.mox
+        m.StubOutWithMock(self._driver.__class__, 'volumeops')
+        self._driver.volumeops = self._volumeops
+        m.StubOutWithMock(self._volumeops, 'get_backing')
+        backing = FakeMor('VirtualMachine', 'my_back')
+        src_vref = FakeObject()
+        src_vref['name'] = 'src_vol_name'
+        src_vref['status'] = 'in-use'
+        volume = FakeObject()
+        self._volumeops.get_backing(src_vref['name']).AndReturn(backing)
+        m.StubOutWithMock(vmdk.VMwareVcVmdkDriver, '_get_clone_type')
+        moxed = vmdk.VMwareVcVmdkDriver._get_clone_type(volume)
+        moxed.AndReturn(volumeops.LINKED_CLONE_TYPE)
+
+        m.ReplayAll()
+        self.assertRaises(exception.InvalidVolume,
+                          self._driver.create_cloned_volume, volume, src_vref)
+        m.UnsetStubs()
         m.VerifyAll()

diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py
index 7353ab6b8711f7145fd62a5e93d3fb444503289c..7f407d6868dfd99f2be0b23995faaa2cffac63f0 100644 (file)
--- a/cinder/volume/drivers/vmware/vmdk.py
+++ b/cinder/volume/drivers/vmware/vmdk.py
@@ -437,9 +437,16 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
 
         If the volume does not have a backing then simply pass, else create
         a snapshot.
+        Snapshot of only available volume is supported.
 
         :param snapshot: Snapshot object
         """
+
+        volume = snapshot['volume']
+        if volume['status'] != 'available':
+            msg = _("Snapshot of volume not supported in state: %s.")
+            LOG.error(msg % volume['status'])
+            raise exception.InvalidVolume(msg % volume['status'])
         backing = self.volumeops.get_backing(snapshot['volume_name'])
         if not backing:
             LOG.info(_("There is no backing, so will not create "
@@ -461,9 +468,16 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
 
         If the volume does not have a backing or the snapshot does not exist
         then simply pass, else delete the snapshot.
+        Snapshot deletion of only available volume is supported.
 
         :param snapshot: Snapshot object
         """
+
+        volume = snapshot['volume']
+        if volume['status'] != 'available':
+            msg = _("Delete snapshot of volume not supported in state: %s.")
+            LOG.error(msg % volume['status'])
+            raise exception.InvalidVolume(msg % volume['status'])
         backing = self.volumeops.get_backing(snapshot['volume_name'])
         if not backing:
             LOG.info(_("There is no backing, and so there is no "
@@ -655,19 +669,24 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
     def copy_volume_to_image(self, context, volume, image_service, image_meta):
         """Creates glance image from volume.
 
+        Upload of only available volume is supported.
         Steps followed are:
 
         1. Get the name of the vmdk file which the volume points to right now.
            Can be a chain of snapshots, so we need to know the last in the
            chain.
-        2. Create the snapshot. A new vmdk is created which the volume points
-           to now. The earlier vmdk becomes read-only.
-        3. Call CopyVirtualDisk which coalesces the disk chain to form a
+        2. Call CopyVirtualDisk which coalesces the disk chain to form a
            single vmdk, rather a .vmdk metadata file and a -flat.vmdk disk
            data file.
-        4. Now upload the -flat.vmdk file to the image store.
-        5. Delete the coalesced .vmdk and -flat.vmdk created.
+        3. Now upload the -flat.vmdk file to the image store.
+        4. Delete the coalesced .vmdk and -flat.vmdk created.
         """
+
+        if volume['status'] != 'available':
+            msg = _("Upload to glance of volume not supported in state: %s.")
+            LOG.error(msg % volume['status'])
+            raise exception.InvalidVolume(msg % volume['status'])
+
         LOG.debug(_("Copy Volume: %s to new image.") % volume['name'])
         VMwareEsxVmdkDriver._validate_disk_format(image_meta['disk_format'])
 
@@ -680,12 +699,8 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
         vmdk_file_path = self.volumeops.get_vmdk_path(backing)
         datastore_name = volumeops.split_datastore_path(vmdk_file_path)[0]
 
-        # Create a snapshot
+        # Create a copy of the vmdk into a tmp file
         image_id = image_meta['id']
-        snapshot_name = "snapshot-%s" % image_id
-        self.volumeops.create_snapshot(backing, snapshot_name, None, True)
-
-        # Create a copy of the snapshotted vmdk into a tmp file
         tmp_vmdk_file_path = '[%s] %s.vmdk' % (datastore_name, image_id)
         host = self.volumeops.get_host(backing)
         datacenter = self.volumeops.get_dc(host)
@@ -841,10 +856,12 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver):
         """Creates volume clone.
 
         If source volume's backing does not exist, then pass.
+        Linked clone of attached volume is not supported.
 
         :param volume: New Volume object
         :param src_vref: Source Volume object
         """
+
         backing = self.volumeops.get_backing(src_vref['name'])
         if not backing:
             LOG.info(_("There is no backing for the source volume: %(src)s. "
@@ -854,6 +871,11 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver):
         clone_type = VMwareVcVmdkDriver._get_clone_type(volume)
         snapshot = None
         if clone_type == volumeops.LINKED_CLONE_TYPE:
+            if src_vref['status'] != 'available':
+                msg = _("Linked clone of source volume not supported "
+                        "in state: %s.")
+                LOG.error(msg % src_vref['status'])
+                raise exception.InvalidVolume(msg % src_vref['status'])
             # For performing a linked clone, we snapshot the volume and
             # then create the linked clone out of this snapshot point.
             name = 'snapshot-%s' % volume['id']