From 9d43d22bf3c887542e475a212f662e73ad44e65e Mon Sep 17 00:00:00 2001 From: Yikun Jiang Date: Sat, 29 Sep 2018 09:44:22 +0800 Subject: [PATCH] Fix unexpected behavior in _clone_image_volume Current, if we upload a snapshot-volume to image, then create a new volume from this image, this new volume is created from original snapshot rather than snapshot-volume. That means if we already make some data change on a snapshot-volume, then we export this snapshot-volume to image, this image will lose all change data. This patch try to fix this, we skip the "snapshot_id" and "source_volid" when we create tmp image volume. Closes-bug: 1795098 Change-Id: Icf0dadc1ce4d008ff437ca5dcde1817d2e560e44 --- cinder/tests/unit/volume/test_image.py | 7 +++++++ cinder/volume/manager.py | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/cinder/tests/unit/volume/test_image.py b/cinder/tests/unit/volume/test_image.py index 47938572296..5bbc3a35f2e 100644 --- a/cinder/tests/unit/volume/test_image.py +++ b/cinder/tests/unit/volume/test_image.py @@ -291,6 +291,7 @@ class CopyVolumeToImageTestCase(base.BaseVolumeTestCase): # creating volume testdata self.volume_attrs['instance_uuid'] = None + self.volume_attrs['snapshot_id'] = fake.SNAPSHOT_ID db.volume_create(self.context, self.volume_attrs) def fake_create(context, volume, **kwargs): @@ -314,6 +315,12 @@ class CopyVolumeToImageTestCase(base.BaseVolumeTestCase): def test_copy_volume_to_image_with_image_volume(self): image = self._test_copy_volume_to_image_with_image_volume() self.assertTrue(image['locations'][0]['url'].startswith('cinder://')) + image_volume_id = image['locations'][0]['url'][9:] + # The image volume does NOT include the snapshot_id, and include the + # source_volid which is the uploaded-volume id. + vol_ref = db.volume_get(self.context, image_volume_id) + self.assertIsNone(vol_ref['snapshot_id']) + self.assertEqual(vol_ref['source_volid'], self.volume_id) def test_copy_volume_to_image_with_image_volume_qcow2(self): self.image_meta['disk_format'] = 'qcow2' diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 00beee9b3a7..f0866463d16 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1416,9 +1416,12 @@ class VolumeManager(manager.CleanableManager, reserve_opts = {'volumes': 1, 'gigabytes': volume.size} QUOTAS.add_volume_type_opts(ctx, reserve_opts, volume_type_id) reservations = QUOTAS.reserve(ctx, **reserve_opts) + # NOTE(yikun): Skip 'snapshot_id', 'source_volid' keys to avoid + # creating tmp img vol from wrong snapshot or wrong source vol. + skip = {'snapshot_id', 'source_volid'} + skip.update(self._VOLUME_CLONE_SKIP_PROPERTIES) try: - new_vol_values = {k: volume[k] for k in set(volume.keys()) - - self._VOLUME_CLONE_SKIP_PROPERTIES} + new_vol_values = {k: volume[k] for k in set(volume.keys()) - skip} new_vol_values['volume_type_id'] = volume_type_id new_vol_values['attach_status'] = ( fields.VolumeAttachStatus.DETACHED)