openstack/cinder
Code Issues Proposed changes

SMBFS: fix parsing volume type extra specs and metadata

Browse Source 
When creating a new volume, the SMBFS volume driver can accept the
volume backing image file format via metadata or extra specs.

The issue is that the volume object the driver receives can be
either a SQLAlchemy model object, either a versioned object.

While there was an effort to keep versioned objects backwards
compatible with the SqlA objects, there still are some
inconsistencies regarding the volume type extra specs.

For this reason, the SMBFS driver fail to parse volume type extra
specs in case of versioned objects.

This patch fixes this issue, by making the driver able to parse
versioned objects as well.

Change-Id: I0385a5ba290ab03b676145d35206b1ed12899473
Closes-Bug: #1529877
This commit is contained in:
Lucian Petrut 2016-03-10 14:22:16 +02:00
parent e26155183e
commit 4666091faf
2 changed files with 88 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
cinder/tests/unit/test_smbfs.py
View File

@ -16,12 +16,17 @@ import copy
import functools
import os
import ddt
import mock
from oslo_utils import fileutils
from cinder import context
from cinder import db
from cinder import exception
from cinder.image import image_utils
from cinder import objects
from cinder import test
from cinder.tests.unit import fake_volume
from cinder.volume.drivers import remotefs
from cinder.volume.drivers import smbfs
@ -40,6 +45,7 @@ def requires_allocation_data_update(expected_size):
return wrapper
@ddt.ddt
class SmbFsTestCase(test.TestCase):
_FAKE_SHARE = '//1.2.3.4/share1'
@ -69,16 +75,16 @@ class SmbFsTestCase(test.TestCase):
_FAKE_ALLOCATION_DATA_PATH = os.path.join('fake_dir',
'fake_allocation_data')
_FAKE_SMBFS_CONFIG = mock.MagicMock()
_FAKE_SMBFS_CONFIG.smbfs_oversub_ratio = 2
_FAKE_SMBFS_CONFIG.smbfs_used_ratio = 0.5
_FAKE_SMBFS_CONFIG.smbfs_shares_config = '/fake/config/path'
_FAKE_SMBFS_CONFIG.smbfs_default_volume_format = 'raw'
_FAKE_SMBFS_CONFIG.smbfs_sparsed_volumes = False
def setUp(self):
super(SmbFsTestCase, self).setUp()
self._FAKE_SMBFS_CONFIG = mock.MagicMock(
smbfs_oversub_ratio = 2,
smbfs_used_ratio = 0.5,
smbfs_shares_config = '/fake/config/path',
smbfs_default_volume_format = 'raw',
smbfs_sparsed_volumes = False)
self._smbfs_driver = smbfs.SmbfsDriver(configuration=mock.Mock())
self._smbfs_driver._remotefsclient = mock.Mock()
self._smbfs_driver._local_volume_dir = mock.Mock(
@ -734,3 +740,52 @@ class SmbFsTestCase(test.TestCase):
fake_block_size * fake_avail_blocks,
self._FAKE_TOTAL_ALLOCATED)
self.assertEqual(expected, ret_val)
@ddt.data([True, False, False],
[False, False, False],
[True, True, True],
[False, True, True],
[False, False, True],
[True, False, True])
@ddt.unpack
def test_get_volume_format_spec(self, volume_versioned_object,
volume_meta_contains_fmt,
volume_type_contains_fmt):
fake_vol_meta_fmt = 'vhd'
fake_vol_type_fmt = 'vhdx'
volume_metadata = {}
volume_type_extra_specs = {}
fake_vol_dict = fake_volume.fake_db_volume()
del fake_vol_dict['name']
if volume_meta_contains_fmt:
volume_metadata['volume_format'] = fake_vol_meta_fmt
elif volume_type_contains_fmt:
volume_type_extra_specs['smbfs:volume_format'] = fake_vol_type_fmt
ctxt = context.get_admin_context()
volume_type = db.volume_type_create(
ctxt, {'extra_specs': volume_type_extra_specs,
'name': 'fake_vol_type'})
fake_vol_dict.update(metadata=volume_metadata,
volume_type_id=volume_type.id)
# We want to get a 'real' SqlA model object, not just a dict.
volume = db.volume_create(ctxt, fake_vol_dict)
volume = db.volume_get(ctxt, volume.id)
if volume_versioned_object:
volume = objects.Volume._from_db_object(ctxt, objects.Volume(),
volume)
resulted_fmt = self._smbfs_driver._get_volume_format_spec(volume)
if volume_meta_contains_fmt:
expected_fmt = fake_vol_meta_fmt
elif volume_type_contains_fmt:
expected_fmt = fake_vol_type_fmt
else:
expected_fmt = None
self.assertEqual(expected_fmt, resulted_fmt)

34
cinder/volume/drivers/smbfs.py
View File

@ -637,19 +637,37 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
return flags.strip(',')
def _get_volume_format_spec(self, volume):
extra_specs = []
# This method needs to be able to parse metadata/volume type
# specs for volume SQLAlchemy objects and versioned objects,
# as the transition to versioned objects is not complete and the
# driver may receive either of them.
#
# TODO(lpetrut): once the transition to oslo.versionedobjects is
# complete, we can skip some of those checks.
volume_metadata_specs = {}
volume_type_specs = {}
metadata_specs = volume.get('volume_metadata') or []
extra_specs += metadata_specs
if volume.get('metadata') and isinstance(volume.metadata, dict):
volume_metadata_specs.update(volume.metadata)
elif volume.get('volume_metadata'):
volume_metadata_specs.update(
{spec.key: spec.value for spec in volume.volume_metadata})
vol_type = volume.get('volume_type')
if vol_type:
volume_type_specs = vol_type.get('extra_specs') or []
extra_specs += volume_type_specs
specs = vol_type.get('extra_specs') or {}
if isinstance(specs, dict):
volume_type_specs.update(specs)
else:
volume_type_specs.update(
{spec.key: spec.value for spec in specs})
for spec in extra_specs:
if 'volume_format' in spec.key:
return spec.value
# In this case, we want the volume metadata specs to take
# precedence over the volume type specs.
for specs in [volume_metadata_specs, volume_type_specs]:
for key, val in specs.items():
if 'volume_format' in key:
return val
return None
def _is_file_size_equal(self, path, size):