From ace220366a7e5cad41e6ac038e1f0cf660306a92 Mon Sep 17 00:00:00 2001 From: int32bit Date: Sat, 12 Nov 2016 17:27:43 +0800 Subject: [PATCH] Add host check while create snapshot We may create a volume but the volume was not been scheduled for some reasons(e.g. The disk space is unavailable, cinder-volume service is down, etc...). Although this volume is in error status, we can force create snapshot using `--force` flag, that's make no sense. What's worse, it may carry some unexpected error: We can delete this error volume before cleanup the snapshot, but we can't delete the snapshot any more via API for its prarent volume not found. This patch just raises an InvalidVolume exception when we create a snapshot on a volume without host. In addtion, the volume state is mutable so I think it is not 100% reliable, the user may reset state w/o checking host state. I think we need check host state before go forward to create snapshot on it. Change-Id: Ifb9388cc76c1ff8d2e7f5b24710acbe95b602825 Closes-Bug: #1641294 --- cinder/tests/unit/api/v1/test_snapshots.py | 6 +++--- cinder/tests/unit/test_volume.py | 13 +++++++++++++ cinder/volume/api.py | 5 +++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/cinder/tests/unit/api/v1/test_snapshots.py b/cinder/tests/unit/api/v1/test_snapshots.py index 3f95282e945..e3c593d99ba 100644 --- a/cinder/tests/unit/api/v1/test_snapshots.py +++ b/cinder/tests/unit/api/v1/test_snapshots.py @@ -108,7 +108,7 @@ class SnapshotApiTest(test.TestCase): self.stubs.Set(volume.api.API, "create_snapshot_force", stub_snapshot_create) - self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) + self.mock_object(volume.api.API, 'get', stubs.stub_volume_api_get) snapshot = {"volume_id": fake.VOLUME_ID, "force": force_param, "display_name": "Snapshot Test Name", @@ -128,7 +128,7 @@ class SnapshotApiTest(test.TestCase): self.stubs.Set(volume.api.API, "create_snapshot_force", stub_snapshot_create) - self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) + self.mock_object(volume.api.API, 'get', stubs.stub_volume_api_get) snapshot = {"volume_id": fake.VOLUME_ID, "force": force_param, "display_name": "Snapshot Test Name", @@ -145,7 +145,7 @@ class SnapshotApiTest(test.TestCase): self.stubs.Set(volume.api.API, "create_snapshot_force", stub_snapshot_create) - self.stubs.Set(volume.api.API, 'get', stubs.stub_volume_get) + self.mock_object(volume.api.API, 'get', stubs.stub_volume_api_get) snapshot = {"volume_id": fake.SNAPSHOT_ID, "force": "**&&^^%%$$##@@", "display_name": "Snapshot Test Name", diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 11adb8adb10..742a0791fad 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -3097,6 +3097,19 @@ class VolumeTestCase(base.BaseVolumeTestCase): 'fake_description', fake.CONSISTENCY_GROUP_ID) + def test_create_snapshot_failed_host_is_None(self): + """Test exception handling when create snapshot and host is None.""" + test_volume = tests_utils.create_volume( + self.context, + host=None) + volume_api = cinder.volume.api.API() + self.assertRaises(exception.InvalidVolume, + volume_api.create_snapshot, + self.context, + test_volume, + 'fake_name', + 'fake_description') + def test_cannot_delete_volume_in_use(self): """Test volume can't be deleted in in-use status.""" self._test_cannot_delete_volume('in-use') diff --git a/cinder/volume/api.py b/cinder/volume/api.py index d964eeb88f6..eff21522e60 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -771,6 +771,11 @@ class API(base.Base): group_snapshot_id=None): check_policy(context, 'create_snapshot', volume) + if not volume.host: + msg = _("The snapshot cannot be created because volume has " + "not been scheduled to any host.") + raise exception.InvalidVolume(reason=msg) + if volume['status'] == 'maintenance': LOG.info(_LI('Unable to create the snapshot for volume, ' 'because it is in maintenance.'), resource=volume)