From ecb1af2836444855774a19e2b2591990eed44217 Mon Sep 17 00:00:00 2001 From: Dmitry Burmistrov Date: Tue, 3 Dec 2013 16:12:39 +0400 Subject: [PATCH] Add patch for convert non raw images according to OSCI-943 Change-Id: I82cf1633267d85e591228c2e81b0be95d88168f3 --- ...-clone-non-raw-images-in-rbd-backend.patch | 496 ++++++++++++++++++ debian/patches/series | 1 + ...-clone-non-raw-images-in-rbd-backend.patch | 496 ++++++++++++++++++ rpm/SPECS/openstack-cinder.spec | 2 + 4 files changed, 995 insertions(+) create mode 100755 debian/patches/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch create mode 100755 rpm/SOURCES/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch diff --git a/debian/patches/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch b/debian/patches/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch new file mode 100755 index 000000000..0b1f0850b --- /dev/null +++ b/debian/patches/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch @@ -0,0 +1,496 @@ +From fd7e9dd59ffa346bed5a11e5312f4bb1bf114ab4 Mon Sep 17 00:00:00 2001 +From: Dmitry Borodaenko +Date: Wed, 27 Nov 2013 14:33:00 -0800 +Subject: [PATCH] Do not clone non-raw images in rbd backend + +RBD backend only supports booting from images in raw format. A volume +that was cloned from an image in any other format is not bootable. The +RBD driver will consider non-raw images to be uncloneable to trigger +automatic conversion to raw format. + +Includes conversion of the corresponding unit test to use mock (instead +of mox) and expanded comments and error messages from patchset #58893 by +Edward Hope-Morley. + +Change-Id: I5725d2f7576bc1b3e9b874ba944ad17d33a6e2cb +Closes-Bug: #1246219 +Closes-Bug: #1247998 +--- + cinder/tests/test_gpfs.py | 6 +- + cinder/tests/test_netapp_nfs.py | 12 +- + cinder/tests/test_rbd.py | 168 +++++++++++++++++--------- + cinder/tests/test_volume.py | 2 +- + cinder/volume/driver.py | 7 +- + cinder/volume/drivers/gpfs.py | 2 +- + cinder/volume/drivers/lvm.py | 2 +- + cinder/volume/drivers/netapp/nfs.py | 2 +- + cinder/volume/drivers/rbd.py | 14 ++- + cinder/volume/drivers/scality.py | 2 +- + cinder/volume/flows/create_volume/__init__.py | 2 +- + 11 files changed, 144 insertions(+), 75 deletions(-) + +diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py +index 4fdb788..1f47c6b 100644 +--- a/cinder/tests/test_gpfs.py ++++ b/cinder/tests/test_gpfs.py +@@ -288,7 +288,8 @@ class GPFSDriverTestCase(test.TestCase): + CONF.gpfs_images_share_mode = 'copy_on_write' + self.driver.clone_image(volume, + None, +- self.image_id) ++ self.image_id, ++ {}) + + self.assertTrue(os.path.exists(volumepath)) + self.volume.delete_volume(self.context, volume['id']) +@@ -309,7 +310,8 @@ class GPFSDriverTestCase(test.TestCase): + CONF.gpfs_images_share_mode = 'copy' + self.driver.clone_image(volume, + None, +- self.image_id) ++ self.image_id, ++ {}) + + self.assertTrue(os.path.exists(volumepath)) + self.volume.delete_volume(self.context, volume['id']) +diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py +index 950efc8..042280e 100644 +--- a/cinder/tests/test_netapp_nfs.py ++++ b/cinder/tests/test_netapp_nfs.py +@@ -469,7 +469,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + drv._post_clone_image(volume) + + mox.ReplayAll() +- drv. clone_image(volume, ('image_location', None), 'image_id') ++ drv.clone_image(volume, ('image_location', None), 'image_id', {}) + mox.VerifyAll() + + def get_img_info(self, format): +@@ -493,7 +493,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + (prop, cloned) = drv. clone_image( +- volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + if not cloned and not prop['provider_location']: + pass +@@ -529,7 +529,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + drv. clone_image( +- volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + + def test_clone_image_cloneableshare_notraw(self): +@@ -566,7 +566,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + + def test_clone_image_file_not_discovered(self): +@@ -605,7 +605,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + vol_dict, result = drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + self.assertFalse(result) + self.assertFalse(vol_dict['bootable']) +@@ -652,7 +652,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + vol_dict, result = drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + self.assertFalse(result) + self.assertFalse(vol_dict['bootable']) +diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py +index 60de09b..054bbb5 100644 +--- a/cinder/tests/test_rbd.py ++++ b/cinder/tests/test_rbd.py +@@ -34,6 +34,7 @@ from cinder.tests.test_volume import DriverTestCase + from cinder import units + from cinder.volume import configuration as conf + import cinder.volume.drivers.rbd as driver ++from cinder.volume.flows import create_volume + + + LOG = logging.getLogger(__name__) +@@ -247,7 +248,8 @@ class RBDTestCase(test.TestCase): + self.assertRaises(exception.ImageUnacceptable, + self.driver._parse_location, + loc) +- self.assertFalse(self.driver._is_cloneable(loc)) ++ self.assertFalse( ++ self.driver._is_cloneable(loc, {'disk_format': 'raw'})) + + def test_cloneable(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') +@@ -264,12 +266,14 @@ class RBDTestCase(test.TestCase): + + self.mox.ReplayAll() + +- self.assertTrue(self.driver._is_cloneable(location)) ++ self.assertTrue( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) + + def test_uncloneable_different_fsid(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') + location = 'rbd://def/pool/image/snap' +- self.assertFalse(self.driver._is_cloneable(location)) ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) + + def test_uncloneable_unreadable(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') +@@ -284,7 +288,16 @@ class RBDTestCase(test.TestCase): + + self.mox.ReplayAll() + +- self.assertFalse(self.driver._is_cloneable(location)) ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) ++ ++ def test_uncloneable_bad_format(self): ++ self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') ++ location = 'rbd://abc/pool/image/snap' ++ formats = ['qcow2', 'vmdk', 'vdi'] ++ for f in formats: ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': f})) + + def _copy_image(self): + @contextlib.contextmanager +@@ -504,26 +517,37 @@ class ManagedRBDTestCase(DriverTestCase): + super(ManagedRBDTestCase, self).setUp() + fake_image.stub_out_image_service(self.stubs) + self.volume.driver.set_initialized() ++ self.called = [] + +- def _clone_volume_from_image(self, expected_status, +- clone_works=True): ++ def _create_volume_from_image(self, expected_status, raw=False, ++ clone_error=False): + """Try to clone a volume from an image, and check the status + afterwards. ++ ++ NOTE: if clone_error is True we force the image type to raw otherwise ++ clone_image is not called + """ +- def fake_clone_image(volume, image_location, image_id): +- return {'provider_location': None}, True ++ def mock_clone_image(volume, image_location, image_id, image_meta): ++ self.called.append('clone_image') ++ if clone_error: ++ raise exception.CinderException() ++ else: ++ return {'provider_location': None}, True + +- def fake_clone_error(volume, image_location, image_id): +- raise exception.CinderException() ++ if clone_error: ++ raw = True + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- if clone_works: +- self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_image) ++ # See tests.image.fake for image types. ++ if raw: ++ expected_calls = ['clone_image'] ++ image_id = '155d900f-4e14-4e4c-a73d-069cbf4541e6' + else: +- self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_error) ++ expected_calls = ['clone_image', 'create_volume', ++ 'copy_image_to_volume'] ++ image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' + +- image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' + volume_id = 1 ++ + # creating volume testdata + db.volume_create(self.context, + {'id': volume_id, +@@ -533,58 +557,88 @@ class ManagedRBDTestCase(DriverTestCase): + 'status': 'creating', + 'instance_uuid': None, + 'host': 'dummy'}) +- try: +- if clone_works: +- self.volume.create_volume(self.context, +- volume_id, +- image_id=image_id) +- else: +- self.assertRaises(exception.CinderException, +- self.volume.create_volume, +- self.context, +- volume_id, +- image_id=image_id) +- +- volume = db.volume_get(self.context, volume_id) +- self.assertEqual(volume['status'], expected_status) +- finally: +- # cleanup +- db.volume_destroy(self.context, volume_id) ++ ++ with mock.patch.object(self.volume.driver, 'create_volume') as \ ++ mock_create_volume: ++ with mock.patch.object(self.volume.driver, 'clone_image', ++ mock_clone_image): ++ with mock.patch.object(create_volume.CreateVolumeFromSpecTask, ++ '_copy_image_to_volume') as \ ++ mock_copy_image_to_volume: ++ self.volume.driver._is_cloneable = mock.Mock() ++ self.volume.driver._is_cloneable.return_value = True ++ ++ try: ++ if not clone_error: ++ self.volume.create_volume(self.context, ++ volume_id, ++ image_id=image_id) ++ else: ++ self.assertRaises(exception.CinderException, ++ self.volume.create_volume, ++ self.context, ++ volume_id, ++ image_id=image_id) ++ ++ volume = db.volume_get(self.context, volume_id) ++ self.assertEqual(volume['status'], expected_status) ++ finally: ++ # cleanup ++ db.volume_destroy(self.context, volume_id) ++ ++ if raw: ++ self.assertEquals(self.called, ['clone_image']) ++ ++ mock_create_volume.assert_called() ++ mock_copy_image_to_volume.assert_called() + + def test_create_vol_from_image_status_available(self): +- """Verify that before cloning, an image is in the available state.""" +- self._clone_volume_from_image('available', True) ++ """Clone raw image then verify volume is in available state.""" ++ self._create_volume_from_image('available', raw=True) ++ ++ def test_create_vol_from_non_raw_image_status_available(self): ++ """Clone non-raw image then verify volume is in available state.""" ++ self._create_volume_from_image('available', raw=False) + + def test_create_vol_from_image_status_error(self): +- """Verify that before cloning, an image is in the available state.""" +- self._clone_volume_from_image('error', False) ++ """Clone raw image with failure then verify volume is in error ++ state. ++ """ ++ self._create_volume_from_image('error', raw=True, clone_error=True) + + def test_clone_image(self): +- # Test Failure Case(s) +- expected = ({}, False) ++ driver = self.volume.driver + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: False) +- image_loc = (object(), object()) +- actual = self.volume.driver.clone_image(object(), image_loc, object()) +- self.assertEqual(expected, actual) ++ # Test uncloneable case(s) ++ with mock.patch.object(driver, '_is_cloneable', ++ lambda *args: False) as mock_is_cloneable: ++ image_loc = (mock.Mock(), mock.Mock()) ++ actual = driver.clone_image(mock.Mock(), image_loc, ++ mock.Mock(), {}) ++ self.assertEqual(({}, False), actual) + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- self.assertEqual(expected, +- self.volume.driver.clone_image(object(), None, None)) ++ self.assertEqual(({}, False), ++ driver.clone_image(object(), None, None, {})) + +- # Test Success Case(s) ++ # Test success case(s) + expected = ({'provider_location': None}, True) +- +- self.stubs.Set(self.volume.driver, '_parse_location', +- lambda x: ('a', 'b', 'c', 'd')) +- +- self.stubs.Set(self.volume.driver, '_clone', lambda *args: None) +- self.stubs.Set(self.volume.driver, '_resize', lambda *args: None) +- actual = self.volume.driver.clone_image(object(), image_loc, object()) +- self.assertEqual(expected, actual) ++ mpo = mock.patch.object ++ with mpo(driver, '_is_cloneable', lambda *args: True): ++ with mpo(driver, '_parse_location', ++ lambda x: ('a', 'b', 'c', 'd')): ++ with mpo(driver, '_clone') as mock_clone: ++ with mpo(driver, '_resize') as mock_resize: ++ actual = driver.clone_image(mock.Mock(), image_loc, ++ mock.Mock(), ++ {'disk_format': 'raw'}) ++ self.assertEqual(expected, actual) ++ mock_clone.assert_called() ++ mock_resize.assert_called() + + def test_clone_success(self): +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- self.stubs.Set(self.volume.driver, 'clone_image', lambda a, b, c: True) ++ self.stubs.Set(self.volume.driver, '_is_cloneable', lambda *args: True) ++ self.stubs.Set(self.volume.driver, 'clone_image', ++ lambda a, b, c, d: True) + image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' +- self.assertTrue(self.volume.driver.clone_image({}, image_id, image_id)) ++ self.assertTrue(self.volume.driver.clone_image({}, ++ image_id, image_id, {})) +diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py +index 4a0d475..928799f 100644 +--- a/cinder/tests/test_volume.py ++++ b/cinder/tests/test_volume.py +@@ -1216,7 +1216,7 @@ class VolumeTestCase(BaseVolumeTestCase): + def fake_fetch_to_raw(ctx, image_service, image_id, path, size=None): + pass + +- def fake_clone_image(volume_ref, image_location, image_id): ++ def fake_clone_image(volume_ref, image_location, image_id, image_meta): + return {'provider_location': None}, True + + dst_fd, dst_path = tempfile.mkstemp() +diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py +index 21dd12b..dd2be3c 100644 +--- a/cinder/volume/driver.py ++++ b/cinder/volume/driver.py +@@ -396,7 +396,7 @@ class VolumeDriver(object): + connector.disconnect_volume(attach_info['conn']['data'], + attach_info['device']) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +@@ -407,6 +407,11 @@ class VolumeDriver(object): + It can be used by the driver to introspect internal + stores or registry to do an efficient image clone. + ++ image_meta is a dictionary that includes 'disk_format' (e.g. ++ raw, qcow2) and other image attributes that allow drivers to ++ decide whether they can clone the image without first requiring ++ conversion. ++ + Returns a dict of volume properties eg. provider_location, + boolean indicating whether cloning occurred + """ +diff --git a/cinder/volume/drivers/gpfs.py b/cinder/volume/drivers/gpfs.py +index 9a1a397..8792ad8 100644 +--- a/cinder/volume/drivers/gpfs.py ++++ b/cinder/volume/drivers/gpfs.py +@@ -463,7 +463,7 @@ class GPFSDriver(driver.VolumeDriver): + return '100M' + return '%sG' % size_in_g + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + return self._clone_image(volume, image_location, image_id) + + def _is_cloneable(self, image_id): +diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py +index 094436f..c6023eb 100644 +--- a/cinder/volume/drivers/lvm.py ++++ b/cinder/volume/drivers/lvm.py +@@ -322,7 +322,7 @@ class LVMVolumeDriver(driver.VolumeDriver): + finally: + self.delete_snapshot(temp_snapshot) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + return None, False + + def backup_volume(self, context, backup, backup_service): +diff --git a/cinder/volume/drivers/netapp/nfs.py b/cinder/volume/drivers/netapp/nfs.py +index 602a1dc..9463137 100644 +--- a/cinder/volume/drivers/netapp/nfs.py ++++ b/cinder/volume/drivers/netapp/nfs.py +@@ -374,7 +374,7 @@ class NetAppNFSDriver(nfs.NfsDriver): + LOG.warning(_('Exception during deleting %s'), ex.__str__()) + return False + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py +index 775ab16..7ed59d0 100644 +--- a/cinder/volume/drivers/rbd.py ++++ b/cinder/volume/drivers/rbd.py +@@ -706,7 +706,7 @@ class RBDDriver(driver.VolumeDriver): + with RADOSClient(self) as client: + return client.cluster.get_fsid() + +- def _is_cloneable(self, image_location): ++ def _is_cloneable(self, image_location, image_meta): + try: + fsid, pool, image, snapshot = self._parse_location(image_location) + except exception.ImageUnacceptable as e: +@@ -718,6 +718,13 @@ class RBDDriver(driver.VolumeDriver): + LOG.debug(reason) + return False + ++ if image_meta['disk_format'] != 'raw': ++ reason = _("rbd image clone requires image format to be " ++ "'raw' but image {0} is '{1}'").format( ++ image_location, image_meta['disk_format']) ++ LOG.debug(reason) ++ return False ++ + # check that we can read the image + try: + with RBDVolumeProxy(self, image, +@@ -730,9 +737,10 @@ class RBDDriver(driver.VolumeDriver): + dict(loc=image_location, err=e)) + return False + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + image_location = image_location[0] if image_location else None +- if image_location is None or not self._is_cloneable(image_location): ++ if image_location is None or not self._is_cloneable( ++ image_location, image_meta): + return ({}, False) + prefix, pool, image, snapshot = self._parse_location(image_location) + self._clone(volume, pool, image, snapshot) +diff --git a/cinder/volume/drivers/scality.py b/cinder/volume/drivers/scality.py +index 4cf49c6..abd6c29 100644 +--- a/cinder/volume/drivers/scality.py ++++ b/cinder/volume/drivers/scality.py +@@ -250,7 +250,7 @@ class ScalityDriver(driver.VolumeDriver): + image_meta, + self.local_path(volume)) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +diff --git a/cinder/volume/flows/create_volume/__init__.py b/cinder/volume/flows/create_volume/__init__.py +index bb7acd3..e5aa371 100644 +--- a/cinder/volume/flows/create_volume/__init__.py ++++ b/cinder/volume/flows/create_volume/__init__.py +@@ -1438,7 +1438,7 @@ class CreateVolumeFromSpecTask(base.CinderTask): + # dict containing provider_location for cloned volume + # and clone status. + model_update, cloned = self.driver.clone_image( +- volume_ref, image_location, image_id) ++ volume_ref, image_location, image_id, image_meta) + if not cloned: + # TODO(harlowja): what needs to be rolled back in the clone if this + # volume create fails?? Likely this should be a subflow or broken +-- +1.8.5.1 + diff --git a/debian/patches/series b/debian/patches/series index 82e379d48..be203b8c7 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,2 +1,3 @@ fix-sqlalchemy-requirements.patch fix-babel-requirements.patch +MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch diff --git a/rpm/SOURCES/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch b/rpm/SOURCES/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch new file mode 100755 index 000000000..0b1f0850b --- /dev/null +++ b/rpm/SOURCES/MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch @@ -0,0 +1,496 @@ +From fd7e9dd59ffa346bed5a11e5312f4bb1bf114ab4 Mon Sep 17 00:00:00 2001 +From: Dmitry Borodaenko +Date: Wed, 27 Nov 2013 14:33:00 -0800 +Subject: [PATCH] Do not clone non-raw images in rbd backend + +RBD backend only supports booting from images in raw format. A volume +that was cloned from an image in any other format is not bootable. The +RBD driver will consider non-raw images to be uncloneable to trigger +automatic conversion to raw format. + +Includes conversion of the corresponding unit test to use mock (instead +of mox) and expanded comments and error messages from patchset #58893 by +Edward Hope-Morley. + +Change-Id: I5725d2f7576bc1b3e9b874ba944ad17d33a6e2cb +Closes-Bug: #1246219 +Closes-Bug: #1247998 +--- + cinder/tests/test_gpfs.py | 6 +- + cinder/tests/test_netapp_nfs.py | 12 +- + cinder/tests/test_rbd.py | 168 +++++++++++++++++--------- + cinder/tests/test_volume.py | 2 +- + cinder/volume/driver.py | 7 +- + cinder/volume/drivers/gpfs.py | 2 +- + cinder/volume/drivers/lvm.py | 2 +- + cinder/volume/drivers/netapp/nfs.py | 2 +- + cinder/volume/drivers/rbd.py | 14 ++- + cinder/volume/drivers/scality.py | 2 +- + cinder/volume/flows/create_volume/__init__.py | 2 +- + 11 files changed, 144 insertions(+), 75 deletions(-) + +diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py +index 4fdb788..1f47c6b 100644 +--- a/cinder/tests/test_gpfs.py ++++ b/cinder/tests/test_gpfs.py +@@ -288,7 +288,8 @@ class GPFSDriverTestCase(test.TestCase): + CONF.gpfs_images_share_mode = 'copy_on_write' + self.driver.clone_image(volume, + None, +- self.image_id) ++ self.image_id, ++ {}) + + self.assertTrue(os.path.exists(volumepath)) + self.volume.delete_volume(self.context, volume['id']) +@@ -309,7 +310,8 @@ class GPFSDriverTestCase(test.TestCase): + CONF.gpfs_images_share_mode = 'copy' + self.driver.clone_image(volume, + None, +- self.image_id) ++ self.image_id, ++ {}) + + self.assertTrue(os.path.exists(volumepath)) + self.volume.delete_volume(self.context, volume['id']) +diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py +index 950efc8..042280e 100644 +--- a/cinder/tests/test_netapp_nfs.py ++++ b/cinder/tests/test_netapp_nfs.py +@@ -469,7 +469,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + drv._post_clone_image(volume) + + mox.ReplayAll() +- drv. clone_image(volume, ('image_location', None), 'image_id') ++ drv.clone_image(volume, ('image_location', None), 'image_id', {}) + mox.VerifyAll() + + def get_img_info(self, format): +@@ -493,7 +493,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + (prop, cloned) = drv. clone_image( +- volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + if not cloned and not prop['provider_location']: + pass +@@ -529,7 +529,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + drv. clone_image( +- volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + + def test_clone_image_cloneableshare_notraw(self): +@@ -566,7 +566,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + + def test_clone_image_file_not_discovered(self): +@@ -605,7 +605,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + vol_dict, result = drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + self.assertFalse(result) + self.assertFalse(vol_dict['bootable']) +@@ -652,7 +652,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase): + + mox.ReplayAll() + vol_dict, result = drv. clone_image( +- volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id') ++ volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {}) + mox.VerifyAll() + self.assertFalse(result) + self.assertFalse(vol_dict['bootable']) +diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py +index 60de09b..054bbb5 100644 +--- a/cinder/tests/test_rbd.py ++++ b/cinder/tests/test_rbd.py +@@ -34,6 +34,7 @@ from cinder.tests.test_volume import DriverTestCase + from cinder import units + from cinder.volume import configuration as conf + import cinder.volume.drivers.rbd as driver ++from cinder.volume.flows import create_volume + + + LOG = logging.getLogger(__name__) +@@ -247,7 +248,8 @@ class RBDTestCase(test.TestCase): + self.assertRaises(exception.ImageUnacceptable, + self.driver._parse_location, + loc) +- self.assertFalse(self.driver._is_cloneable(loc)) ++ self.assertFalse( ++ self.driver._is_cloneable(loc, {'disk_format': 'raw'})) + + def test_cloneable(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') +@@ -264,12 +266,14 @@ class RBDTestCase(test.TestCase): + + self.mox.ReplayAll() + +- self.assertTrue(self.driver._is_cloneable(location)) ++ self.assertTrue( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) + + def test_uncloneable_different_fsid(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') + location = 'rbd://def/pool/image/snap' +- self.assertFalse(self.driver._is_cloneable(location)) ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) + + def test_uncloneable_unreadable(self): + self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') +@@ -284,7 +288,16 @@ class RBDTestCase(test.TestCase): + + self.mox.ReplayAll() + +- self.assertFalse(self.driver._is_cloneable(location)) ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': 'raw'})) ++ ++ def test_uncloneable_bad_format(self): ++ self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc') ++ location = 'rbd://abc/pool/image/snap' ++ formats = ['qcow2', 'vmdk', 'vdi'] ++ for f in formats: ++ self.assertFalse( ++ self.driver._is_cloneable(location, {'disk_format': f})) + + def _copy_image(self): + @contextlib.contextmanager +@@ -504,26 +517,37 @@ class ManagedRBDTestCase(DriverTestCase): + super(ManagedRBDTestCase, self).setUp() + fake_image.stub_out_image_service(self.stubs) + self.volume.driver.set_initialized() ++ self.called = [] + +- def _clone_volume_from_image(self, expected_status, +- clone_works=True): ++ def _create_volume_from_image(self, expected_status, raw=False, ++ clone_error=False): + """Try to clone a volume from an image, and check the status + afterwards. ++ ++ NOTE: if clone_error is True we force the image type to raw otherwise ++ clone_image is not called + """ +- def fake_clone_image(volume, image_location, image_id): +- return {'provider_location': None}, True ++ def mock_clone_image(volume, image_location, image_id, image_meta): ++ self.called.append('clone_image') ++ if clone_error: ++ raise exception.CinderException() ++ else: ++ return {'provider_location': None}, True + +- def fake_clone_error(volume, image_location, image_id): +- raise exception.CinderException() ++ if clone_error: ++ raw = True + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- if clone_works: +- self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_image) ++ # See tests.image.fake for image types. ++ if raw: ++ expected_calls = ['clone_image'] ++ image_id = '155d900f-4e14-4e4c-a73d-069cbf4541e6' + else: +- self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_error) ++ expected_calls = ['clone_image', 'create_volume', ++ 'copy_image_to_volume'] ++ image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' + +- image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' + volume_id = 1 ++ + # creating volume testdata + db.volume_create(self.context, + {'id': volume_id, +@@ -533,58 +557,88 @@ class ManagedRBDTestCase(DriverTestCase): + 'status': 'creating', + 'instance_uuid': None, + 'host': 'dummy'}) +- try: +- if clone_works: +- self.volume.create_volume(self.context, +- volume_id, +- image_id=image_id) +- else: +- self.assertRaises(exception.CinderException, +- self.volume.create_volume, +- self.context, +- volume_id, +- image_id=image_id) +- +- volume = db.volume_get(self.context, volume_id) +- self.assertEqual(volume['status'], expected_status) +- finally: +- # cleanup +- db.volume_destroy(self.context, volume_id) ++ ++ with mock.patch.object(self.volume.driver, 'create_volume') as \ ++ mock_create_volume: ++ with mock.patch.object(self.volume.driver, 'clone_image', ++ mock_clone_image): ++ with mock.patch.object(create_volume.CreateVolumeFromSpecTask, ++ '_copy_image_to_volume') as \ ++ mock_copy_image_to_volume: ++ self.volume.driver._is_cloneable = mock.Mock() ++ self.volume.driver._is_cloneable.return_value = True ++ ++ try: ++ if not clone_error: ++ self.volume.create_volume(self.context, ++ volume_id, ++ image_id=image_id) ++ else: ++ self.assertRaises(exception.CinderException, ++ self.volume.create_volume, ++ self.context, ++ volume_id, ++ image_id=image_id) ++ ++ volume = db.volume_get(self.context, volume_id) ++ self.assertEqual(volume['status'], expected_status) ++ finally: ++ # cleanup ++ db.volume_destroy(self.context, volume_id) ++ ++ if raw: ++ self.assertEquals(self.called, ['clone_image']) ++ ++ mock_create_volume.assert_called() ++ mock_copy_image_to_volume.assert_called() + + def test_create_vol_from_image_status_available(self): +- """Verify that before cloning, an image is in the available state.""" +- self._clone_volume_from_image('available', True) ++ """Clone raw image then verify volume is in available state.""" ++ self._create_volume_from_image('available', raw=True) ++ ++ def test_create_vol_from_non_raw_image_status_available(self): ++ """Clone non-raw image then verify volume is in available state.""" ++ self._create_volume_from_image('available', raw=False) + + def test_create_vol_from_image_status_error(self): +- """Verify that before cloning, an image is in the available state.""" +- self._clone_volume_from_image('error', False) ++ """Clone raw image with failure then verify volume is in error ++ state. ++ """ ++ self._create_volume_from_image('error', raw=True, clone_error=True) + + def test_clone_image(self): +- # Test Failure Case(s) +- expected = ({}, False) ++ driver = self.volume.driver + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: False) +- image_loc = (object(), object()) +- actual = self.volume.driver.clone_image(object(), image_loc, object()) +- self.assertEqual(expected, actual) ++ # Test uncloneable case(s) ++ with mock.patch.object(driver, '_is_cloneable', ++ lambda *args: False) as mock_is_cloneable: ++ image_loc = (mock.Mock(), mock.Mock()) ++ actual = driver.clone_image(mock.Mock(), image_loc, ++ mock.Mock(), {}) ++ self.assertEqual(({}, False), actual) + +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- self.assertEqual(expected, +- self.volume.driver.clone_image(object(), None, None)) ++ self.assertEqual(({}, False), ++ driver.clone_image(object(), None, None, {})) + +- # Test Success Case(s) ++ # Test success case(s) + expected = ({'provider_location': None}, True) +- +- self.stubs.Set(self.volume.driver, '_parse_location', +- lambda x: ('a', 'b', 'c', 'd')) +- +- self.stubs.Set(self.volume.driver, '_clone', lambda *args: None) +- self.stubs.Set(self.volume.driver, '_resize', lambda *args: None) +- actual = self.volume.driver.clone_image(object(), image_loc, object()) +- self.assertEqual(expected, actual) ++ mpo = mock.patch.object ++ with mpo(driver, '_is_cloneable', lambda *args: True): ++ with mpo(driver, '_parse_location', ++ lambda x: ('a', 'b', 'c', 'd')): ++ with mpo(driver, '_clone') as mock_clone: ++ with mpo(driver, '_resize') as mock_resize: ++ actual = driver.clone_image(mock.Mock(), image_loc, ++ mock.Mock(), ++ {'disk_format': 'raw'}) ++ self.assertEqual(expected, actual) ++ mock_clone.assert_called() ++ mock_resize.assert_called() + + def test_clone_success(self): +- self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True) +- self.stubs.Set(self.volume.driver, 'clone_image', lambda a, b, c: True) ++ self.stubs.Set(self.volume.driver, '_is_cloneable', lambda *args: True) ++ self.stubs.Set(self.volume.driver, 'clone_image', ++ lambda a, b, c, d: True) + image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' +- self.assertTrue(self.volume.driver.clone_image({}, image_id, image_id)) ++ self.assertTrue(self.volume.driver.clone_image({}, ++ image_id, image_id, {})) +diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py +index 4a0d475..928799f 100644 +--- a/cinder/tests/test_volume.py ++++ b/cinder/tests/test_volume.py +@@ -1216,7 +1216,7 @@ class VolumeTestCase(BaseVolumeTestCase): + def fake_fetch_to_raw(ctx, image_service, image_id, path, size=None): + pass + +- def fake_clone_image(volume_ref, image_location, image_id): ++ def fake_clone_image(volume_ref, image_location, image_id, image_meta): + return {'provider_location': None}, True + + dst_fd, dst_path = tempfile.mkstemp() +diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py +index 21dd12b..dd2be3c 100644 +--- a/cinder/volume/driver.py ++++ b/cinder/volume/driver.py +@@ -396,7 +396,7 @@ class VolumeDriver(object): + connector.disconnect_volume(attach_info['conn']['data'], + attach_info['device']) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +@@ -407,6 +407,11 @@ class VolumeDriver(object): + It can be used by the driver to introspect internal + stores or registry to do an efficient image clone. + ++ image_meta is a dictionary that includes 'disk_format' (e.g. ++ raw, qcow2) and other image attributes that allow drivers to ++ decide whether they can clone the image without first requiring ++ conversion. ++ + Returns a dict of volume properties eg. provider_location, + boolean indicating whether cloning occurred + """ +diff --git a/cinder/volume/drivers/gpfs.py b/cinder/volume/drivers/gpfs.py +index 9a1a397..8792ad8 100644 +--- a/cinder/volume/drivers/gpfs.py ++++ b/cinder/volume/drivers/gpfs.py +@@ -463,7 +463,7 @@ class GPFSDriver(driver.VolumeDriver): + return '100M' + return '%sG' % size_in_g + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + return self._clone_image(volume, image_location, image_id) + + def _is_cloneable(self, image_id): +diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py +index 094436f..c6023eb 100644 +--- a/cinder/volume/drivers/lvm.py ++++ b/cinder/volume/drivers/lvm.py +@@ -322,7 +322,7 @@ class LVMVolumeDriver(driver.VolumeDriver): + finally: + self.delete_snapshot(temp_snapshot) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + return None, False + + def backup_volume(self, context, backup, backup_service): +diff --git a/cinder/volume/drivers/netapp/nfs.py b/cinder/volume/drivers/netapp/nfs.py +index 602a1dc..9463137 100644 +--- a/cinder/volume/drivers/netapp/nfs.py ++++ b/cinder/volume/drivers/netapp/nfs.py +@@ -374,7 +374,7 @@ class NetAppNFSDriver(nfs.NfsDriver): + LOG.warning(_('Exception during deleting %s'), ex.__str__()) + return False + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py +index 775ab16..7ed59d0 100644 +--- a/cinder/volume/drivers/rbd.py ++++ b/cinder/volume/drivers/rbd.py +@@ -706,7 +706,7 @@ class RBDDriver(driver.VolumeDriver): + with RADOSClient(self) as client: + return client.cluster.get_fsid() + +- def _is_cloneable(self, image_location): ++ def _is_cloneable(self, image_location, image_meta): + try: + fsid, pool, image, snapshot = self._parse_location(image_location) + except exception.ImageUnacceptable as e: +@@ -718,6 +718,13 @@ class RBDDriver(driver.VolumeDriver): + LOG.debug(reason) + return False + ++ if image_meta['disk_format'] != 'raw': ++ reason = _("rbd image clone requires image format to be " ++ "'raw' but image {0} is '{1}'").format( ++ image_location, image_meta['disk_format']) ++ LOG.debug(reason) ++ return False ++ + # check that we can read the image + try: + with RBDVolumeProxy(self, image, +@@ -730,9 +737,10 @@ class RBDDriver(driver.VolumeDriver): + dict(loc=image_location, err=e)) + return False + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + image_location = image_location[0] if image_location else None +- if image_location is None or not self._is_cloneable(image_location): ++ if image_location is None or not self._is_cloneable( ++ image_location, image_meta): + return ({}, False) + prefix, pool, image, snapshot = self._parse_location(image_location) + self._clone(volume, pool, image, snapshot) +diff --git a/cinder/volume/drivers/scality.py b/cinder/volume/drivers/scality.py +index 4cf49c6..abd6c29 100644 +--- a/cinder/volume/drivers/scality.py ++++ b/cinder/volume/drivers/scality.py +@@ -250,7 +250,7 @@ class ScalityDriver(driver.VolumeDriver): + image_meta, + self.local_path(volume)) + +- def clone_image(self, volume, image_location, image_id): ++ def clone_image(self, volume, image_location, image_id, image_meta): + """Create a volume efficiently from an existing image. + + image_location is a string whose format depends on the +diff --git a/cinder/volume/flows/create_volume/__init__.py b/cinder/volume/flows/create_volume/__init__.py +index bb7acd3..e5aa371 100644 +--- a/cinder/volume/flows/create_volume/__init__.py ++++ b/cinder/volume/flows/create_volume/__init__.py +@@ -1438,7 +1438,7 @@ class CreateVolumeFromSpecTask(base.CinderTask): + # dict containing provider_location for cloned volume + # and clone status. + model_update, cloned = self.driver.clone_image( +- volume_ref, image_location, image_id) ++ volume_ref, image_location, image_id, image_meta) + if not cloned: + # TODO(harlowja): what needs to be rolled back in the clone if this + # volume create fails?? Likely this should be a subflow or broken +-- +1.8.5.1 + diff --git a/rpm/SPECS/openstack-cinder.spec b/rpm/SPECS/openstack-cinder.spec index a973c2627..e39dd511f 100644 --- a/rpm/SPECS/openstack-cinder.spec +++ b/rpm/SPECS/openstack-cinder.spec @@ -31,6 +31,7 @@ Patch0001: 0001-Ensure-we-don-t-access-the-net-when-building-docs.patch Patch0002: 0002-Use-updated-parallel-install-versions-of-epel-packag.patch Patch0003: 0003-Remove-runtime-dep-on-python-pbr-python-d2to1.patch Patch0004: 0004-Revert-Use-oslo.sphinx-and-remove-local-copy-of-doc-.patch +Patch0005: MIRA-Do-not-clone-non-raw-images-in-rbd-backend.patch BuildArch: noarch BuildRequires: intltool @@ -147,6 +148,7 @@ This package contains documentation files for cinder. %patch0002 -p1 %patch0003 -p1 %patch0004 -p1 +%patch0005 -p1 find . \( -name .gitignore -o -name .placeholder \) -delete -- 2.45.2