From 3b0addd827de5613b4c17c802b9f936fb60491ff Mon Sep 17 00:00:00 2001
From: Peter Penchev <openstack-dev@storpool.com>
Date: Wed, 22 Jun 2022 10:04:31 +0300
Subject: [PATCH 7/9] Add the clone_across_pools driver capability

Let drivers declare that they can clone a volume into a different pool and
relax the checks when creating a volume from an image (both with and
without the image cache).

Change-Id: Idbac4dae00f9fa03fb14547a204666721bf67d96
Implements: blueprint clone-across-pools
---
 cinder/image/cache.py                         | 11 ++-
 cinder/tests/unit/image/test_cache.py         | 14 ++--
 .../volume/flows/test_create_volume_flow.py   | 73 +++++++++++++++++
 cinder/volume/flows/manager/create_volume.py  |  8 +-
 cinder/volume/manager.py                      |  3 +-
 doc/source/reference/support-matrix.ini       | 78 +++++++++++++++++++
 6 files changed, 174 insertions(+), 13 deletions(-)

diff --git a/cinder/image/cache.py b/cinder/image/cache.py
index 6d30210a4..a79451bf2 100644
--- a/cinder/image/cache.py
+++ b/cinder/image/cache.py
@@ -34,11 +34,13 @@ class ImageVolumeCache(object):
                  db,
                  volume_api,
                  max_cache_size_gb: int = 0,
-                 max_cache_size_count: int = 0):
+                 max_cache_size_count: int = 0,
+                 clone_across_pools: bool = False):
         self.db = db
         self.volume_api = volume_api
         self.max_cache_size_gb = int(max_cache_size_gb)
         self.max_cache_size_count = int(max_cache_size_count)
+        self.clone_across_pools = bool(clone_across_pools)
         self.notifier = rpc.get_notifier('volume', CONF.host)
 
     def get_by_image_volume(self,
@@ -55,11 +57,12 @@ class ImageVolumeCache(object):
         self._notify_cache_eviction(context, cache_entry['image_id'],
                                     cache_entry['host'])
 
-    @staticmethod
-    def _get_query_filters(volume_ref: objects.Volume) -> dict:
+    def _get_query_filters(self, volume_ref: objects.Volume) -> dict:
         if volume_ref.is_clustered:
             return {'cluster_name': volume_ref.cluster_name}
-        return {'host': volume_ref.host}
+        if not self.clone_across_pools:
+            return {'host': volume_ref.host}
+        return {}
 
     def get_entry(self,
                   context: context.RequestContext,
diff --git a/cinder/tests/unit/image/test_cache.py b/cinder/tests/unit/image/test_cache.py
index c3aba9b28..b8b704e0d 100644
--- a/cinder/tests/unit/image/test_cache.py
+++ b/cinder/tests/unit/image/test_cache.py
@@ -42,11 +42,12 @@ class ImageVolumeCacheTestCase(test.TestCase):
         self.volume.update(vol_params)
         self.volume_ovo = objects.Volume(self.context, **vol_params)
 
-    def _build_cache(self, max_gb=0, max_count=0):
+    def _build_cache(self, max_gb=0, max_count=0, clone_across_pools=False):
         cache = image_cache.ImageVolumeCache(self.mock_db,
                                              self.mock_volume_api,
                                              max_gb,
-                                             max_count)
+                                             max_count,
+                                             clone_across_pools)
         cache.notifier = self.notifier
         return cache
 
@@ -91,9 +92,10 @@ class ImageVolumeCacheTestCase(test.TestCase):
         self.assertEqual(entry['image_id'], msg['payload']['image_id'])
         self.assertEqual(1, len(self.notifier.notifications))
 
-    @ddt.data(True, False)
-    def test_get_entry(self, clustered):
-        cache = self._build_cache()
+    @ddt.data((True, True), (True, False), (False, True), (False, False))
+    @ddt.unpack
+    def test_get_entry(self, clustered, clone_across_pools):
+        cache = self._build_cache(clone_across_pools=clone_across_pools)
         entry = self._build_entry()
         image_meta = {
             'is_public': True,
@@ -107,7 +109,7 @@ class ImageVolumeCacheTestCase(test.TestCase):
          image_volume_cache_get_and_update_last_used.return_value) = entry
         if not clustered:
             self.volume_ovo.cluster_name = None
-            expect = {'host': self.volume.host}
+            expect = {} if clone_across_pools else {'host': self.volume.host}
         else:
             expect = {'cluster_name': self.volume.cluster_name}
         found_entry = cache.get_entry(self.context,
diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py
index fa629599a..a90e943fe 100644
--- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py
+++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py
@@ -1060,6 +1060,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
             self, volume_get_by_id, vol_update, rekey_vol, cleanup_cg):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             fake_volume_manager, fake_db, fake_driver)
@@ -1085,6 +1086,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
                                   handle_bootable, cleanup_cg):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             fake_volume_manager, fake_db, fake_driver)
@@ -1110,6 +1112,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
                                                  mock_cleanup_cg):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             fake_volume_manager, fake_db, fake_driver)
@@ -1146,6 +1149,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
                                                 mock_cleanup_cg):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_cache = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
@@ -1195,6 +1199,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
                                                     mock_cleanup_cg):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             fake_volume_manager, fake_db, fake_driver)
@@ -1254,6 +1259,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
                                 driver_error):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         backup_host = 'host@backend#pool'
         test_manager = create_volume_manager.CreateVolumeFromSpecTask(
@@ -1293,6 +1299,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
     def test_create_drive_error(self, mock_message_create):
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_volume_manager = mock.MagicMock()
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             fake_volume_manager, fake_db, fake_driver)
@@ -1494,6 +1501,7 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
             spec=utils.get_file_spec())
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             mock.MagicMock(), fake_db, fake_driver)
         fake_image_service = fake_image.FakeImageService()
@@ -1520,6 +1528,7 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
                       'cinder_encryption_key_id': None}
 
         fake_driver.clone_image.return_value = (None, False)
+        fake_db.volume_get_all.return_value = []
         fake_db.volume_get_all_by_host.return_value = [image_volume]
 
         fake_manager._create_from_image(self.ctxt,
@@ -1538,6 +1547,69 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
             self.assertFalse(fake_driver.create_cloned_volume.called)
         mock_cleanup_cg.assert_called_once_with(volume)
 
+    @mock.patch('cinder.volume.flows.manager.create_volume.'
+                'CreateVolumeFromSpecTask.'
+                '_cleanup_cg_in_volume')
+    @mock.patch('cinder.image.image_utils.TemporaryImages.fetch')
+    @mock.patch('cinder.volume.flows.manager.create_volume.'
+                'CreateVolumeFromSpecTask.'
+                '_handle_bootable_volume_glance_meta')
+    @mock.patch('cinder.image.image_utils.qemu_img_info')
+    def test_create_from_image_across(self, mock_qemu_info, handle_bootable,
+                                      mock_fetch_img, mock_cleanup_cg,
+                                      format='raw', owner=None,
+                                      location=True):
+        self.flags(allowed_direct_url_schemes=['cinder'])
+        mock_fetch_img.return_value = mock.MagicMock(
+            spec=utils.get_file_spec())
+        fake_db = mock.MagicMock()
+        fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {'clone_across_pools': True}
+        fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
+            mock.MagicMock(), fake_db, fake_driver)
+        fake_image_service = fake_image.FakeImageService()
+
+        volume = fake_volume.fake_volume_obj(self.ctxt,
+                                             host='host@backend#pool')
+        image_volume = fake_volume.fake_volume_obj(self.ctxt,
+                                                   volume_metadata={})
+        image_id = fakes.IMAGE_ID
+        image_info = imageutils.QemuImgInfo()
+        image_info.virtual_size = '1073741824'
+        mock_qemu_info.return_value = image_info
+
+        url = 'cinder://%s' % image_volume['id']
+        image_location = None
+        if location:
+            image_location = (url, [{'url': url, 'metadata': {}}])
+        image_meta = {'id': image_id,
+                      'container_format': 'bare',
+                      'disk_format': format,
+                      'size': 1024,
+                      'owner': owner or self.ctxt.project_id,
+                      'virtual_size': None,
+                      'cinder_encryption_key_id': None}
+
+        fake_driver.clone_image.return_value = (None, False)
+        fake_db.volume_get_all.return_value = [image_volume]
+        fake_db.volume_get_all_by_host.return_value = []
+
+        fake_manager._create_from_image(self.ctxt,
+                                        volume,
+                                        image_location,
+                                        image_id,
+                                        image_meta,
+                                        fake_image_service)
+        if format == 'raw' and not owner and location:
+            fake_driver.create_cloned_volume.assert_called_once_with(
+                volume, image_volume)
+            handle_bootable.assert_called_once_with(self.ctxt, volume,
+                                                    image_id=image_id,
+                                                    image_meta=image_meta)
+        else:
+            self.assertFalse(fake_driver.create_cloned_volume.called)
+        mock_cleanup_cg.assert_called_once_with(volume)
+
     LEGACY_URI = 'cinder://%s' % fakes.VOLUME_ID
     MULTISTORE_URI = 'cinder://fake-store/%s' % fakes.VOLUME_ID
 
@@ -1564,6 +1636,7 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
             spec=utils.get_file_spec())
         fake_db = mock.MagicMock()
         fake_driver = mock.MagicMock()
+        fake_driver.capabilities = {}
         fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
             mock.MagicMock(), fake_db, fake_driver)
         fake_image_service = fake_image.FakeImageService()
diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py
index 1f78b3375..5ddeb1a56 100644
--- a/cinder/volume/flows/manager/create_volume.py
+++ b/cinder/volume/flows/manager/create_volume.py
@@ -741,8 +741,12 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
         urls = list(set([direct_url]
                         + [loc.get('url') for loc in locations or []]))
         image_volume_ids = self._extract_cinder_ids(urls)
-        image_volumes = self.db.volume_get_all_by_host(
-            context, volume['host'], filters={'id': image_volume_ids})
+        if self.driver.capabilities.get('clone_across_pools'):
+            image_volumes = self.db.volume_get_all(
+                context, filters={'id': image_volume_ids})
+        else:
+            image_volumes = self.db.volume_get_all_by_host(
+                context, volume['host'], filters={'id': image_volume_ids})
 
         for image_volume in image_volumes:
             # For the case image volume is stored in the service tenant,
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 34e6e4cff..7850ed19b 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -356,7 +356,8 @@ class VolumeManager(manager.CleanableManager,
                 self.db,
                 cinder_volume.API(),
                 max_cache_size,
-                max_cache_entries
+                max_cache_entries,
+                self.driver.capabilities.get('clone_across_pools', False)
             )
             LOG.info('Image-volume cache enabled for host %(host)s.',
                      {'host': self.host})
diff --git a/doc/source/reference/support-matrix.ini b/doc/source/reference/support-matrix.ini
index 4cf4cf655..243993fe8 100644
--- a/doc/source/reference/support-matrix.ini
+++ b/doc/source/reference/support-matrix.ini
@@ -1048,3 +1048,81 @@ driver.win_iscsi=missing
 driver.win_smb=missing
 driver.yadro=complete
 driver.zadara=missing
+
+[operation.clone_across_pools]
+title=Clone a volume into a different pool
+status=optional
+notes=Vendor drivers that support cloning a volume into a different
+      storage pool, e.g. when creating a volume from a Cinder-backed
+      Glance image.
+driver.datacore=missing
+driver.datera=missing
+driver.dell_emc_powermax=missing
+driver.dell_emc_powerstore=missing
+driver.dell_emc_powerstore_nfs=missing
+driver.dell_emc_powervault=missing
+driver.dell_emc_sc=missing
+driver.dell_emc_unity=missing
+driver.dell_emc_vmax_af=missing
+driver.dell_emc_vmax_3=missing
+driver.dell_emc_vnx=missing
+driver.dell_emc_powerflex=missing
+driver.dell_emc_xtremio=missing
+driver.fujitsu_eternus=missing
+driver.fungible=missing
+driver.hitachi_vsp=missing
+driver.hpe_3par=missing
+driver.hpe_msa=missing
+driver.hpe_nimble=missing
+driver.hpe_xp=missing
+driver.huawei_t_v1=missing
+driver.huawei_t_v2=missing
+driver.huawei_v3=missing
+driver.huawei_f_v3=missing
+driver.huawei_f_v5=missing
+driver.huawei_v5=missing
+driver.huawei_18000=missing
+driver.huawei_dorado=missing
+driver.huawei_fusionstorage=missing
+driver.infinidat=missing
+driver.ibm_ds8k=missing
+driver.ibm_flashsystem=missing
+driver.ibm_gpfs=missing
+driver.ibm_storwize=missing
+driver.ibm_xiv=missing
+driver.infortrend=missing
+driver.inspur=missing
+driver.inspur_as13000=missing
+driver.kaminario=missing
+driver.kioxia_kumoscale=missing
+driver.lenovo=missing
+driver.lightbits_lightos=missing
+driver.linbit_linstor=missing
+driver.lvm=missing
+driver.macrosan=missing
+driver.nec=missing
+driver.nec_v=missing
+driver.netapp_ontap=missing
+driver.netapp_solidfire=missing
+driver.nexenta=missing
+driver.nfs=missing
+driver.opene_joviandss=missing
+driver.prophetstor=missing
+driver.pure=missing
+driver.qnap=missing
+driver.quobyte=missing
+driver.rbd=missing
+driver.rbd_iscsi=missing
+driver.sandstone=missing
+driver.seagate=missing
+driver.storpool=missing
+driver.synology=missing
+driver.toyou_netstor=missing
+driver.vrtsaccess=missing
+driver.vrtscnfs=missing
+driver.vzstorage=missing
+driver.vmware=missing
+driver.win_iscsi=missing
+driver.win_smb=missing
+driver.yadro=missing
+driver.zadara=missing
-- 
2.43.0