Fix volume lookups in SolidFire template caching

There's a number of issues with the SolidFire template caching
code.  There are a number of metadata items that aren't consistent
between Glance API V1 and V2, there's also some problems with how
the code is dealing with it's internal volume and just flat out
wrong account checking.

This patch focuses mainly on fixing up the account and volumeID
issues.  The _do_clone method currently expects to find the src
volume under the same account as the dest volume.  That's fine
for most cases, but doesn't work for either the SolidFire template
caching, or for the use of generic image caching in Cinder.

We fix that easily by modifying the _get_sf_volume call to have an
option to search all active volumes on the Cluster instead of just
those for a specified account.  For now we only change the clone
methods, as they're the only case where we fall into the case of
cross-ownership of volumes in the same context.

The other issue is around the cleanup/delete in the case of a
failed template-volume create.  In this case we were passing in
the Cinder UUID to a direct SolidFire API delete cmd, which
is wrong.  The direct call for the SolidFire API call requires
that the actual SolidFire ID be used.  So we leverage the modified
_get_sf_volume call here as well.

Another issue that was introduced was the introduction of
a connector object to the intialize_connection call.  We
got around this once already by just passing in a fake
connector that was actually a string.  That was fine, but
the initialize_connection routine also now checks the connector
for multi_attach, so it fails the _get call on a string.  We
fix that by just building a dict and using that instead of
a fake string.

Finally, the method of relying on the Glance metadata virt size
has been problematic, it's not set by default, it's not accurate
and it's a bit inefficient.  There have been a number of issues
with this method of size determination, so we'll dump that and
just leverage the image_utils methods directly.

Change-Id: Icfd6668389049d3d5686acdb832847aa2d2fce52
Closes-Bug: #1494830
Closes-Bug: #1494927

diff --git a/cinder/tests/unit/test_solidfire.py b/cinder/tests/unit/test_solidfire.py
index c3cda4d28a4a2a423f22d9aa379feec555e6ce23..a8cbdb15fa75e47a0c6332101eb5c6893537c183 100644 (file)
--- a/cinder/tests/unit/test_solidfire.py
+++ b/cinder/tests/unit/test_solidfire.py
@@ -386,6 +386,10 @@ class SolidFireVolumeTestCase(test.TestCase):
         _mock_issue_api_request.return_value = self.mock_stats_data
         _mock_create_template_account.return_value = 1
         _fake_get_snaps = [{'snapshotID': 5, 'name': 'testvol'}]
+        _fake_get_volume = (
+            {'volumeID': 99,
+             'name': 'UUID-a720b3c0-d1f0-11e1-9b23-0800200c9a66',
+             'attributes': {}})
 
         testvol = {'project_id': 'testprjid',
                    'name': 'testvol',
@@ -405,6 +409,9 @@ class SolidFireVolumeTestCase(test.TestCase):
         with mock.patch.object(sfv,
                                '_get_sf_snapshots',
                                return_value=_fake_get_snaps), \
+                mock.patch.object(sfv,
+                                  '_get_sf_volume',
+                                  return_value=_fake_get_volume), \
                 mock.patch.object(sfv,
                                   '_issue_api_request',
                                   side_effect=self.fake_issue_api_request), \
@@ -985,33 +992,14 @@ class SolidFireVolumeTestCase(test.TestCase):
                                                            'fake',
                                                            _fake_image_meta,
                                                            'fake'))
-
-    @mock.patch.object(solidfire.SolidFireDriver, '_issue_api_request')
-    @mock.patch.object(solidfire.SolidFireDriver, '_create_template_account')
-    def test_clone_image_virt_size_not_set(self,
-                                           _mock_create_template_account,
-                                           _mock_issue_api_request):
-        _mock_issue_api_request.return_value = self.mock_stats_data
-        _mock_create_template_account.return_value = 1
-
-        self.configuration.sf_allow_template_caching = True
-        sfv = solidfire.SolidFireDriver(configuration=self.configuration)
-
-        # Don't run clone_image if virtual_size property not on image
-        _fake_image_meta = {'id': '17c550bb-a411-44c0-9aaf-0d96dd47f501',
-                            'updated_at': datetime.datetime(2013, 9,
-                                                            28, 15,
-                                                            27, 36,
-                                                            325355),
-                            'is_public': True,
-                            'owner': 'testprjid'}
-
-        self.assertEqual((None, False),
-                         sfv.clone_image(self.ctxt,
-                                         self.mock_volume,
-                                         'fake',
-                                         _fake_image_meta,
-                                         'fake'))
+            # And using the new V2 visibility tag
+            _fake_image_meta['visibility'] = 'public'
+            _fake_image_meta['owner'] = 'wrong-owner'
+            self.assertEqual(('fo', True), sfv.clone_image(self.ctxt,
+                                                           self.mock_volume,
+                                                           'fake',
+                                                           _fake_image_meta,
+                                                           'fake'))
 
     def test_create_template_no_account(self):
         sfv = solidfire.SolidFireDriver(configuration=self.configuration)

diff --git a/cinder/volume/drivers/solidfire.py b/cinder/volume/drivers/solidfire.py
index 97b8dd56b594d2c24cea29ab44145a33426f797d..63ee271e9bb8431a3f684c904a580810d1284e65 100644 (file)
--- a/cinder/volume/drivers/solidfire.py
+++ b/cinder/volume/drivers/solidfire.py
@@ -32,7 +32,7 @@ import six
 
 from cinder import context
 from cinder import exception
-from cinder.i18n import _, _LE, _LI, _LW
+from cinder.i18n import _, _LE, _LW
 from cinder.image import image_utils
 from cinder.volume.drivers.san import san
 from cinder.volume import qos_specs
@@ -439,6 +439,7 @@ class SolidFireDriver(san.SanISCSIDriver):
                          src_project_id,
                          vref):
         """Create a clone of an existing volume or snapshot."""
+
         attributes = {}
         qos = {}
 
@@ -470,8 +471,7 @@ class SolidFireDriver(san.SanISCSIDriver):
             params['volumeID'] = int(snap['volumeID'])
             params['newSize'] = int(vref['size'] * units.Gi)
         else:
-            sf_vol = self._get_sf_volume(
-                src_uuid, {'accountID': sf_account['accountID']})
+            sf_vol = self._get_sf_volume(src_uuid)
             if sf_vol is None:
                 raise exception.VolumeNotFound(volume_id=src_uuid)
             params['volumeID'] = int(sf_vol['volumeID'])
@@ -591,9 +591,13 @@ class SolidFireDriver(san.SanISCSIDriver):
                 qos[key] = int(value)
         return qos
 
-    def _get_sf_volume(self, uuid, params):
-        vols = self._issue_api_request(
-            'ListVolumesForAccount', params)['result']['volumes']
+    def _get_sf_volume(self, uuid, params=None):
+        if params:
+            vols = self._issue_api_request(
+                'ListVolumesForAccount', params)['result']['volumes']
+        else:
+            vols = self._issue_api_request(
+                'ListActiveVolumes', params)['result']['volumes']
 
         found_count = 0
         sf_volref = None
@@ -635,63 +639,79 @@ class SolidFireDriver(san.SanISCSIDriver):
     def _create_image_volume(self, context,
                              image_meta, image_service,
                              image_id):
-        # NOTE(jdg): It's callers responsibility to ensure that
-        # the optional properties.virtual_size is set on the image
-        # before we get here
-        virt_size = int(image_meta['properties'].get('virtual_size'))
-        min_sz_in_bytes = (
-            math.ceil(virt_size / float(units.Gi)) * float(units.Gi))
-        min_sz_in_gig = math.ceil(min_sz_in_bytes / float(units.Gi))
-
-        attributes = {}
-        attributes['image_info'] = {}
-        attributes['image_info']['image_updated_at'] = (
-            image_meta['updated_at'].isoformat())
-        attributes['image_info']['image_name'] = (
-            image_meta['name'])
-        attributes['image_info']['image_created_at'] = (
-            image_meta['created_at'].isoformat())
-        attributes['image_info']['image_id'] = image_meta['id']
-        params = {'name': 'OpenStackIMG-%s' % image_id,
-                  'accountID': self.template_account_id,
-                  'sliceCount': 1,
-                  'totalSize': int(min_sz_in_bytes),
-                  'enable512e': self.configuration.sf_emulate_512,
-                  'attributes': attributes,
-                  'qos': {}}
-
-        sf_account = self._issue_api_request(
-            'GetAccountByID',
-            {'accountID': self.template_account_id})['result']['account']
-
-        template_vol = self._do_volume_create(sf_account, params)
-        tvol = {}
-        tvol['id'] = image_id
-        tvol['provider_location'] = template_vol['provider_location']
-        tvol['provider_auth'] = template_vol['provider_auth']
-
-        connector = 'na'
-        conn = self.initialize_connection(tvol, connector)
-        attach_info = super(SolidFireDriver, self)._connect_device(conn)
-        properties = 'na'
-
-        try:
-            image_utils.fetch_to_raw(context,
-                                     image_service,
-                                     image_id,
-                                     attach_info['device']['path'],
-                                     self.configuration.volume_dd_blocksize,
-                                     size=min_sz_in_gig)
-        except Exception as exc:
-            params['volumeID'] = template_vol['volumeID']
-            LOG.error(_LE('Failed image conversion during cache creation: %s'),
-                      exc)
-            LOG.debug('Removing SolidFire Cache Volume (SF ID): %s',
-                      template_vol['volumeID'])
-
-            self._detach_volume(context, attach_info, tvol, properties)
-            self._issue_api_request('DeleteVolume', params)
-            return
+        with image_utils.TemporaryImages.fetch(image_service,
+                                               context,
+                                               image_id) as tmp_image:
+            data = image_utils.qemu_img_info(tmp_image)
+            fmt = data.file_format
+            if fmt is None:
+                raise exception.ImageUnacceptable(
+                    reason=_("'qemu-img info' parsing failed."),
+                    image_id=image_id)
+
+            backing_file = data.backing_file
+            if backing_file is not None:
+                raise exception.ImageUnacceptable(
+                    image_id=image_id,
+                    reason=_("fmt=%(fmt)s backed by:%(backing_file)s")
+                    % {'fmt': fmt, 'backing_file': backing_file, })
+
+            virtual_size = int(math.ceil(float(data.virtual_size) / units.Gi))
+            attributes = {}
+            attributes['image_info'] = {}
+            attributes['image_info']['image_updated_at'] = (
+                image_meta['updated_at'].isoformat())
+            attributes['image_info']['image_name'] = (
+                image_meta['name'])
+            attributes['image_info']['image_created_at'] = (
+                image_meta['created_at'].isoformat())
+            attributes['image_info']['image_id'] = image_meta['id']
+            params = {'name': 'OpenStackIMG-%s' % image_id,
+                      'accountID': self.template_account_id,
+                      'sliceCount': 1,
+                      'totalSize': int(virtual_size * units.Gi),
+                      'enable512e': self.configuration.sf_emulate_512,
+                      'attributes': attributes,
+                      'qos': {}}
+
+            sf_account = self._issue_api_request(
+                'GetAccountByID',
+                {'accountID': self.template_account_id})['result']['account']
+            template_vol = self._do_volume_create(sf_account, params)
+
+            tvol = {}
+            tvol['id'] = image_id
+            tvol['provider_location'] = template_vol['provider_location']
+            tvol['provider_auth'] = template_vol['provider_auth']
+
+            connector = {'multipath': False}
+            conn = self.initialize_connection(tvol, connector)
+            attach_info = super(SolidFireDriver, self)._connect_device(conn)
+            properties = 'na'
+            try:
+                image_utils.convert_image(tmp_image,
+                                          attach_info['device']['path'],
+                                          'raw',
+                                          run_as_root=True)
+                data = image_utils.qemu_img_info(attach_info['device']['path'],
+                                                 run_as_root=True)
+                if data.file_format != 'raw':
+                    raise exception.ImageUnacceptable(
+                        image_id=image_id,
+                        reason=_("Converted to %(vol_format)s, but format is "
+                                 "now %(file_format)s") % {'vol_format': 'raw',
+                                                           'file_format': data.
+                                                           file_format})
+            except Exception as exc:
+                vol = self._get_sf_volume(image_id)
+                LOG.error(_LE('Failed image conversion during '
+                              'cache creation: %s'),
+                          exc)
+                LOG.debug('Removing SolidFire Cache Volume (SF ID): %s',
+                          vol['volumeID'])
+                self._detach_volume(context, attach_info, tvol, properties)
+                self._issue_api_request('DeleteVolume', params)
+                return
 
         self._detach_volume(context, attach_info, tvol, properties)
         sf_vol = self._get_sf_volume(image_id, params)
@@ -793,25 +813,28 @@ class SolidFireDriver(san.SanISCSIDriver):
     def clone_image(self, context,
                     volume, image_location,
                     image_meta, image_service):
-
+        public = False
         # Check out pre-requisites:
         # Is template caching enabled?
         if not self.configuration.sf_allow_template_caching:
             return None, False
 
-        # Is the image owned by this tenant or public?
-        if ((not image_meta.get('is_public', False)) and
-                (image_meta['owner'] != volume['project_id'])):
-                LOG.warning(_LW("Requested image is not "
-                                "accessible by current Tenant."))
-                return None, False
-
-        # Is virtual_size property set on the image?
-        if ((not image_meta.get('properties', None)) or
-                (not image_meta['properties'].get('virtual_size', None))):
-            LOG.info(_LI('Unable to create cache volume because image: %s '
-                         'does not include properties.virtual_size'),
-                     image_meta['id'])
+        # NOTE(jdg): Glance V2 moved from is_public to visibility
+        # so we check both, as we don't necessarily know or want
+        # to care which we're using.  Will need to look at
+        # future handling of things like shared and community
+        # but for now, it's owner or public and that's it
+        visibility = image_meta.get('visibility', None)
+        if visibility and visibility == 'public':
+            public = True
+        elif image_meta.get('is_public', False):
+            public = True
+        else:
+            if image_meta['owner'] == volume['project_id']:
+                public = True
+        if not public:
+            LOG.warning(_LW("Requested image is not "
+                            "accessible by current Tenant."))
             return None, False
 
         try: