From 16f1dc862e9771f7b26d094bdcf41791c9fc5a8c Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Tue, 29 Dec 2015 12:35:52 +0900 Subject: [PATCH] Fix image volume creation error When "image_upload_use_cinder_backend = True" is specified to the volume backend and the Glance cinder store is enabled, upload-to-image command fails with the following error: Create clone_image_volume: xxx for image xxx failed (Exception: An object of type VolumeAttachmentList is required in field volume_attachment, not a ) This is due to unnecessary attributes specified in the volume params, such as 'volume_attachment' or 'consistencygroup'. This patch removes the unnecessary attributes from the params. Change-Id: Ia416cb6686312e1591de7fe40a7ce7d6aa2fce7d Closes-Bug: #1529762 Related-Bug: #1528087 --- cinder/tests/unit/test_volume.py | 34 ++++++++++++++++++++++++++++++++ cinder/volume/manager.py | 27 +++++++++++-------------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index adb9c7004..ccf9ba3bf 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -3489,6 +3489,40 @@ class VolumeTestCase(BaseVolumeTestCase): self.context, volume_id) + @mock.patch('cinder.volume.drivers.lvm.LVMVolumeDriver.' + 'create_cloned_volume') + @mock.patch('cinder.quota.QUOTAS.rollback') + @mock.patch('cinder.quota.QUOTAS.commit') + @mock.patch('cinder.quota.QUOTAS.reserve', return_value=["RESERVATION"]) + def test_clone_image_volume(self, mock_reserve, mock_commit, + mock_rollback, mock_cloned_volume): + vol = tests_utils.create_volume(self.context, + **self.volume_params) + # unnecessary attributes should be removed from image volume + vol.consistencygroup = None + result = self.volume._clone_image_volume(self.context, vol, + {'id': 'fake'}) + + self.assertNotEqual(False, result) + mock_reserve.assert_called_once_with(self.context, volumes=1, + gigabytes=vol.size) + mock_commit.assert_called_once_with(self.context, ["RESERVATION"], + project_id=vol.project_id) + + @mock.patch('cinder.quota.QUOTAS.rollback') + @mock.patch('cinder.quota.QUOTAS.commit') + @mock.patch('cinder.quota.QUOTAS.reserve', return_value=["RESERVATION"]) + def test_clone_image_volume_creation_failure(self, mock_reserve, + mock_commit, mock_rollback): + vol = tests_utils.create_volume(self.context, **self.volume_params) + with mock.patch.object(objects, 'Volume', side_effect=ValueError): + self.assertFalse(self.volume._clone_image_volume(self.context, vol, + {'id': 'fake'})) + + mock_reserve.assert_called_once_with(self.context, volumes=1, + gigabytes=vol.size) + mock_rollback.assert_called_once_with(self.context, ["RESERVATION"]) + @mock.patch('cinder.image.image_utils.TemporaryImages.fetch') @mock.patch('cinder.volume.flows.manager.create_volume.' 'CreateVolumeFromSpecTask._clone_image_volume') diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 40b9c2bd3..42fa984fc 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -207,6 +207,15 @@ class VolumeManager(manager.SchedulerDependentManager): target = messaging.Target(version=RPC_API_VERSION) + # On cloning a volume, we shouldn't copy volume_type, consistencygroup + # and volume_attachment, because the db sets that according to [field]_id, + # which we do copy. We also skip some other values that are set during + # creation of Volume object. + _VOLUME_CLONE_SKIP_PROPERTIES = { + 'id', '_name_id', 'name_id', 'name', 'status', + 'attach_status', 'migration_status', 'volume_type', + 'consistencygroup', 'volume_attachment'} + def __init__(self, volume_driver=None, service_name=None, *args, **kwargs): """Load the driver from the one specified in args, or from flags.""" @@ -1080,13 +1089,8 @@ class VolumeManager(manager.SchedulerDependentManager): QUOTAS.add_volume_type_opts(ctx, reserve_opts, volume_type_id) reservations = QUOTAS.reserve(ctx, **reserve_opts) try: - new_vol_values = dict(volume.items()) - new_vol_values.pop('id', None) - new_vol_values.pop('_name_id', None) - new_vol_values.pop('name_id', None) - new_vol_values.pop('volume_type', None) - new_vol_values.pop('name', None) - + new_vol_values = {k: volume[k] for k in set(volume.keys()) - + self._VOLUME_CLONE_SKIP_PROPERTIES} new_vol_values['volume_type_id'] = volume_type_id new_vol_values['attach_status'] = 'detached' new_vol_values['status'] = 'creating' @@ -1620,14 +1624,7 @@ class VolumeManager(manager.SchedulerDependentManager): rpcapi = volume_rpcapi.VolumeAPI() # Create new volume on remote host - - skip = {'id', '_name_id', 'name_id', 'name', 'host', 'status', - 'attach_status', 'migration_status', 'volume_type', - 'consistencygroup', 'volume_attachment'} - # We don't copy volume_type, consistencygroup and volume_attachment, - # because the db sets that according to [field]_id, which we do copy. - # We also skip some other values that are either set manually later or - # during creation of Volume object. + skip = self._VOLUME_CLONE_SKIP_PROPERTIES | {'host'} new_vol_values = {k: volume[k] for k in set(volume.keys()) - skip} if new_type_id: new_vol_values['volume_type_id'] = new_type_id -- 2.45.2