exp: add our clone_volume() and clone_image() patches.
Change-Id: Idffa91bf8f1670ce8fb47d20468df2093865414e
diff --git a/patches/openstack/cinder/storpool-clone-volume.patch b/patches/openstack/cinder/storpool-clone-volume.patch
new file mode 100644
index 0000000..8b8c55c
--- /dev/null
+++ b/patches/openstack/cinder/storpool-clone-volume.patch
@@ -0,0 +1,425 @@
+commit 5563b42f8ba435bc45e08399c77ac350156d9aa2
+Author: Peter Penchev <openstack-dev@storpool.com>
+Date: Wed Apr 20 15:47:39 2022 +0300
+
+ StorPool: create_cloned_volume() improvements
+
+ If the source and destination volumes are in the same StorPool template
+ (as defined by either the volume type or the global config setting),
+ forego the need to create the transient snapshot at all and use
+ StorPool's "base this volume on that one" API call (which does the same
+ thing internally, but much more efficiently and atomically).
+
+ If the destination volume should be in a different StorPool template,
+ then make sure that the transient snapshot is also in that template so
+ that, if other volumes are cloned from the same source volume later,
+ they can all use the same data underneath (the internal workings of
+ StorPool will detect that all those snapshots are exactly the same and
+ not duplicate any data in the destination template). This will avoid
+ data duplication, sometimes with drastic results.
+
+ Bump the minimum required version of the "storpool" third-party library
+ for snapshotUpdate(template=...) support.
+
+ Change-Id: Ib9bb76cf2e2f2b035b92e596b1ef185558b190d6
+
+diff --git a/cinder/tests/unit/volume/drivers/test_storpool.py b/cinder/tests/unit/volume/drivers/test_storpool.py
+index 843283db4..51db7f292 100644
+--- a/cinder/tests/unit/volume/drivers/test_storpool.py
++++ b/cinder/tests/unit/volume/drivers/test_storpool.py
+@@ -1,4 +1,4 @@
+-# Copyright 2014 - 2017, 2019 - 2021 StorPool
++# Copyright 2014 - 2017, 2019 - 2022 StorPool
+ # All Rights Reserved.
+ #
+ # Licensed under the Apache License, Version 2.0 (the "License"); you may
+@@ -14,6 +14,7 @@
+ # under the License.
+
+
++import itertools
+ import re
+ import sys
+ from unittest import mock
+@@ -110,13 +111,33 @@ class MockAPI(object):
+ def snapshotCreate(self, vname, snap):
+ snapshots[snap['name']] = dict(volumes[vname])
+
++ def snapshotUpdate(self, snap, data):
++ sdata = snapshots[snap]
++ sdata.update(data)
++
+ def snapshotDelete(self, name):
+ del snapshots[name]
+
+ def volumeCreate(self, v):
+- if v['name'] in volumes:
++ name = v['name']
++ if name in volumes:
+ raise MockApiError('volume already exists')
+- volumes[v['name']] = v
++ data = dict(v)
++
++ if 'parent' in v and 'template' not in v:
++ sdata = snapshots[v['parent']]
++ if 'template' in sdata:
++ data['template'] = sdata['template']
++
++ if 'baseOn' in v and 'template' not in v:
++ vdata = volumes[v['baseOn']]
++ if 'template' in vdata:
++ data['template'] = vdata['template']
++
++ if 'template' not in data:
++ data['template'] = None
++
++ volumes[name] = data
+
+ def volumeDelete(self, name):
+ del volumes[name]
+@@ -171,6 +192,23 @@ fakeStorPool.spopenstack.AttachDB = MockAttachDB
+ fakeStorPool.sptypes.VolumeUpdateDesc = MockVolumeUpdateDesc
+
+
++class MockVolumeDB(object):
++ """Simulate a Cinder database with a volume_get() method."""
++
++ def __init__(self, vol_types=None):
++ """Store the specified volume types mapping if necessary."""
++ self.vol_types = vol_types if vol_types is not None else {}
++
++ def volume_get(self, _context, vid):
++ """Get a volume-like structure, only the fields we care about."""
++ # Still, try to at least make sure we know about that volume
++ return {
++ 'id': vid,
++ 'size': volumes[volumeName(vid)]['size'],
++ 'volume_type': self.vol_types.get(vid),
++ }
++
++
+ @ddt.ddt
+ class StorPoolTestCase(test.TestCase):
+
+@@ -260,6 +298,11 @@ class StorPoolTestCase(test.TestCase):
+ self.assertListEqual(sorted([volumeName(n) for n in names]),
+ sorted(volumes.keys()))
+
++ def assertSnapshotNames(self, specs):
++ self.assertListEqual(
++ sorted(snapshotName(spec[0], spec[1]) for spec in specs),
++ sorted(snapshots.keys()))
++
+ @mock_volume_types
+ def test_create_delete_volume(self):
+ self.assertVolumeNames([])
+@@ -272,7 +315,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('1',))
+ v = volumes[volumeName('1')]
+ self.assertEqual(1 * units.Gi, v['size'])
+- self.assertNotIn('template', v.keys())
++ self.assertIsNone(v['template'])
+ self.assertEqual(3, v['replication'])
+
+ caught = False
+@@ -292,7 +335,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('1',))
+ v = volumes[volumeName('1')]
+ self.assertEqual(2 * units.Gi, v['size'])
+- self.assertNotIn('template', v.keys())
++ self.assertIsNone(v['template'])
+ self.assertEqual(3, v['replication'])
+
+ self.driver.create_volume({'id': '2', 'name': 'v2', 'size': 3,
+@@ -300,7 +343,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('1', '2'))
+ v = volumes[volumeName('2')]
+ self.assertEqual(3 * units.Gi, v['size'])
+- self.assertNotIn('template', v.keys())
++ self.assertIsNone(v['template'])
+ self.assertEqual(3, v['replication'])
+
+ self.driver.create_volume({'id': '3', 'name': 'v2', 'size': 4,
+@@ -322,7 +365,7 @@ class StorPoolTestCase(test.TestCase):
+ # Make sure the dictionary is not corrupted somehow...
+ v = volumes[volumeName('1')]
+ self.assertEqual(2 * units.Gi, v['size'])
+- self.assertNotIn('template', v.keys())
++ self.assertIsNone(v['template'])
+ self.assertEqual(3, v['replication'])
+
+ for vid in ('1', '2', '3', '4'):
+@@ -386,16 +429,17 @@ class StorPoolTestCase(test.TestCase):
+ self.driver.extend_volume({'id': '1'}, 2)
+ self.assertEqual(2 * units.Gi, volumes[volumeName('1')]['size'])
+
+- self.driver.create_cloned_volume({'id': '2', 'name': 'clo', 'size': 3},
+- {'id': 1})
++ with mock.patch.object(self.driver, 'db', new=MockVolumeDB()):
++ self.driver.create_cloned_volume(
++ {'id': '2', 'name': 'clo', 'size': 3, 'volume_type': None},
++ {'id': 1})
+ self.assertVolumeNames(('1', '2'))
+ self.assertDictEqual({}, snapshots)
+- # Note: this would not be true in a real environment (the snapshot will
+- # have been deleted, the volume would have no parent), but with this
+- # fake implementation it helps us make sure that the second volume was
+- # created with the proper options.
+- self.assertEqual(volumes[volumeName('2')]['parent'],
+- snapshotName('clone', '2'))
++ # We do not provide a StorPool template name in either of the volumes'
++ # types, so create_cloned_volume() should take the baseOn shortcut.
++ vol2 = volumes[volumeName('2')]
++ self.assertEqual(vol2['baseOn'], volumeName('1'))
++ self.assertNotIn('parent', vol2)
+
+ self.driver.delete_volume({'id': 1})
+ self.driver.delete_volume({'id': 2})
+@@ -403,6 +447,78 @@ class StorPoolTestCase(test.TestCase):
+ self.assertDictEqual({}, volumes)
+ self.assertDictEqual({}, snapshots)
+
++ @ddt.data(*itertools.product(
++ [None] + [{'id': key} for key in sorted(volume_types.keys())],
++ [None] + [{'id': key} for key in sorted(volume_types.keys())]))
++ @ddt.unpack
++ @mock_volume_types
++ def test_create_cloned_volume(self, src_type, dst_type):
++ self.assertDictEqual({}, volumes)
++ self.assertDictEqual({}, snapshots)
++
++ src_template = (
++ None
++ if src_type is None
++ else volume_types[src_type['id']].get('storpool_template')
++ )
++ dst_template = (
++ None
++ if dst_type is None
++ else volume_types[dst_type['id']].get('storpool_template')
++ )
++ src_name = 's-none' if src_template is None else 's-' + src_template
++ dst_name = 'd-none' if dst_template is None else 'd-' + dst_template
++
++ snap_name = snapshotName('clone', '2')
++
++ vdata1 = {
++ 'id': '1',
++ 'name': src_name,
++ 'size': 1,
++ 'volume_type': src_type,
++ }
++ self.assertEqual(
++ self.driver._template_from_volume(vdata1),
++ src_template)
++ self.driver.create_volume(vdata1)
++ self.assertVolumeNames(('1',))
++
++ vdata2 = {
++ 'id': 2,
++ 'name': dst_name,
++ 'size': 1,
++ 'volume_type': dst_type,
++ }
++ self.assertEqual(
++ self.driver._template_from_volume(vdata2),
++ dst_template)
++ with mock.patch.object(self.driver, 'db',
++ new=MockVolumeDB(vol_types={'1': src_type})):
++ self.driver.create_cloned_volume(vdata2, {'id': '1'})
++ self.assertVolumeNames(('1', '2'))
++ vol2 = volumes[volumeName('2')]
++ self.assertEqual(vol2['template'], dst_template)
++
++ if src_template == dst_template:
++ self.assertEqual(vol2['baseOn'], volumeName('1'))
++ self.assertNotIn('parent', vol2)
++
++ self.assertDictEqual({}, snapshots)
++ else:
++ self.assertNotIn('baseOn', vol2)
++ self.assertEqual(vol2['parent'], snap_name)
++
++ self.assertSnapshotNames((('clone', '2'),))
++ self.assertEqual(snapshots[snap_name]['template'], dst_template)
++
++ self.driver.delete_volume({'id': '1'})
++ self.driver.delete_volume({'id': '2'})
++ if src_template != dst_template:
++ del snapshots[snap_name]
++
++ self.assertDictEqual({}, volumes)
++ self.assertDictEqual({}, snapshots)
++
+ @mock_volume_types
+ def test_config_replication(self):
+ self.assertVolumeNames([])
+@@ -422,7 +538,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('cfgrepl1',))
+ v = volumes[volumeName('cfgrepl1')]
+ self.assertEqual(3, v['replication'])
+- self.assertNotIn('template', v)
++ self.assertIsNone(v['template'])
+ self.driver.delete_volume({'id': 'cfgrepl1'})
+
+ self.driver.configuration.storpool_replication = 2
+@@ -436,7 +552,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('cfgrepl2',))
+ v = volumes[volumeName('cfgrepl2')]
+ self.assertEqual(2, v['replication'])
+- self.assertNotIn('template', v)
++ self.assertIsNone(v['template'])
+ self.driver.delete_volume({'id': 'cfgrepl2'})
+
+ self.driver.create_volume({'id': 'cfgrepl3', 'name': 'v1', 'size': 1,
+@@ -468,7 +584,7 @@ class StorPoolTestCase(test.TestCase):
+ self.assertVolumeNames(('cfgtempl1',))
+ v = volumes[volumeName('cfgtempl1')]
+ self.assertEqual(3, v['replication'])
+- self.assertNotIn('template', v)
++ self.assertIsNone(v['template'])
+ self.driver.delete_volume({'id': 'cfgtempl1'})
+
+ self.driver.create_volume({'id': 'cfgtempl2', 'name': 'v1', 'size': 1,
+diff --git a/cinder/volume/drivers/storpool.py b/cinder/volume/drivers/storpool.py
+index 58ceed1be..36ad98ded 100644
+--- a/cinder/volume/drivers/storpool.py
++++ b/cinder/volume/drivers/storpool.py
+@@ -1,4 +1,4 @@
+-# Copyright (c) 2014 - 2021 StorPool
++# Copyright (c) 2014 - 2022 StorPool
+ # All Rights Reserved.
+ #
+ # Licensed under the Apache License, Version 2.0 (the "License"); you may
+@@ -19,11 +19,13 @@ import platform
+
+ from oslo_config import cfg
+ from oslo_log import log as logging
++from oslo_utils import excutils
+ from oslo_utils import importutils
+ from oslo_utils import units
+ import six
+
+ from cinder.common import constants
++from cinder import context
+ from cinder import exception
+ from cinder.i18n import _
+ from cinder import interface
+@@ -94,6 +96,7 @@ class StorPoolDriver(driver.VolumeDriver):
+ 2.0.0 - Drop _attach_volume() and _detach_volume(), our os-brick
+ connector will handle this.
+ - Drop backup_volume()
++ - Avoid data duplication in create_cloned_volume()
+ """
+
+ VERSION = '2.0.0'
+@@ -200,30 +203,80 @@ class StorPoolDriver(driver.VolumeDriver):
+
+ def create_cloned_volume(self, volume, src_vref):
+ refname = self._attach.volumeName(src_vref['id'])
++ size = int(volume['size']) * units.Gi
++ volname = self._attach.volumeName(volume['id'])
++
++ src_volume = self.db.volume_get(
++ context.get_admin_context(),
++ src_vref['id'],
++ )
++ src_template = self._template_from_volume(src_volume)
++
++ template = self._template_from_volume(volume)
++ LOG.debug('clone volume id %(vol_id)s template %(template)s', {
++ 'vol_id': repr(volume['id']),
++ 'template': repr(template),
++ })
++ if template == src_template:
++ LOG.info('Using baseOn to clone a volume into the same template')
++ try:
++ self._attach.api().volumeCreate({
++ 'name': volname,
++ 'size': size,
++ 'baseOn': refname,
++ })
++ except spapi.ApiError as e:
++ raise self._backendException(e)
++
++ return None
++
+ snapname = self._attach.snapshotName('clone', volume['id'])
++ LOG.info(
++ 'A transient snapshot for a %(src)s -> %(dst)s template change',
++ {'src': src_template, 'dst': template})
+ try:
+ self._attach.api().snapshotCreate(refname, {'name': snapname})
+ except spapi.ApiError as e:
+- raise self._backendException(e)
++ if e.name != 'objectExists':
++ raise self._backendException(e)
+
+- size = int(volume['size']) * units.Gi
+- volname = self._attach.volumeName(volume['id'])
+ try:
+- self._attach.api().volumeCreate({
+- 'name': volname,
+- 'size': size,
+- 'parent': snapname
+- })
+- except spapi.ApiError as e:
+- raise self._backendException(e)
+- finally:
+ try:
+- self._attach.api().snapshotDelete(snapname)
++ self._attach.api().snapshotUpdate(
++ snapname,
++ {'template': template},
++ )
++ except spapi.ApiError as e:
++ raise self._backendException(e)
++
++ try:
++ self._attach.api().volumeCreate({
++ 'name': volname,
++ 'size': size,
++ 'parent': snapname
++ })
+ except spapi.ApiError as e:
+- # ARGH!
+- LOG.error("Could not delete the temp snapshot %(name)s: "
+- "%(msg)s",
+- {'name': snapname, 'msg': e})
++ raise self._backendException(e)
++
++ try:
++ self._attach.api().snapshotUpdate(
++ snapname,
++ {'tags': {'transient': '1.0'}},
++ )
++ except spapi.ApiError as e:
++ raise self._backendException(e)
++ except Exception:
++ with excutils.save_and_reraise_exception():
++ try:
++ LOG.warning(
++ 'Something went wrong, removing the transient snapshot'
++ )
++ self._attach.api().snapshotDelete(snapname)
++ except spapi.ApiError as e:
++ LOG.error(
++ 'Could not delete the %(name)s snapshot: %(err)s',
++ {'name': snapname, 'err': str(e)}
++ )
+
+ def create_export(self, context, volume, connector):
+ pass
+diff --git a/driver-requirements.txt b/driver-requirements.txt
+index cc8da8eb5..ec05b01c6 100644
+--- a/driver-requirements.txt
++++ b/driver-requirements.txt
+@@ -40,7 +40,7 @@ infi.dtypes.wwn # PSF
+ infi.dtypes.iqn # PSF
+
+ # Storpool
+-storpool>=4.0.0 # Apache-2.0
++storpool>=7.1.0 # Apache-2.0
+ storpool.spopenstack>=2.2.1 # Apache-2.0
+
+ # Datera