From c4cc7df0bd89d4d8348a05d8e42a7b24afdca844 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Thu, 29 Jun 2017 16:35:27 -0700 Subject: [PATCH] VMware: Fix volume cascade delete VMDK driver does not allow snapshot deletion if the volume is attached. Currently we assume the volume to be attached if it's state is not 'available', which is wrong. During cascade delete, the volume status is set to 'deleting'. Therefore, snapshot deletion fails even if the volume is not attached. Fixing this by using volume attachment info to check for attached volumes. Change-Id: I83874d6d287fe3950b003f7eac74db9a1602432e Closes-bug: #1701393 --- .../unit/volume/drivers/vmware/test_vmware_vmdk.py | 10 +++++++--- cinder/volume/drivers/vmware/vmdk.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py index 0f1f0bb0f8a..43f726fe5af 100644 --- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py +++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py @@ -273,28 +273,32 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): vops.delete_snapshot.assert_not_called() @mock.patch.object(VMDK_DRIVER, 'volumeops') - def test_delete_snapshot_with_backing(self, vops): + @mock.patch.object(VMDK_DRIVER, '_in_use', return_value=False) + def test_delete_snapshot_with_backing(self, in_use, vops): backing = mock.sentinel.backing vops.get_backing.return_value = backing - volume = self._create_volume_dict() + volume = self._create_volume_dict(status='deleting') snapshot = fake_snapshot.fake_snapshot_obj(self._context, volume=volume) self._driver.delete_snapshot(snapshot) vops.get_backing.assert_called_once_with(snapshot.volume_name) vops.get_snapshot.assert_called_once_with(backing, snapshot.name) + in_use.assert_called_once_with(snapshot.volume) vops.delete_snapshot.assert_called_once_with( backing, snapshot.name) @mock.patch.object(VMDK_DRIVER, 'volumeops') - def test_delete_snapshot_when_attached(self, vops): + @mock.patch.object(VMDK_DRIVER, '_in_use', return_value=True) + def test_delete_snapshot_when_attached(self, in_use, vops): volume = self._create_volume_dict(status='in-use') snapshot = fake_snapshot.fake_snapshot_obj(self._context, volume=volume) self.assertRaises(cinder_exceptions.InvalidSnapshot, self._driver.delete_snapshot, snapshot) + in_use.assert_called_once_with(snapshot.volume) @mock.patch.object(VMDK_DRIVER, 'volumeops') def test_delete_snapshot_without_backend_snapshot(self, vops): diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index ec85609eaac..ce00d9042c9 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -678,7 +678,7 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): resource=snapshot.volume) elif not self.volumeops.get_snapshot(backing, snapshot.name): LOG.debug("Snapshot does not exist in backend.", resource=snapshot) - elif snapshot.volume.status != 'available': + elif self._in_use(snapshot.volume): msg = _("Delete snapshot of volume not supported in " "state: %s.") % snapshot.volume.status LOG.error(msg)