From e1b254dad1cbab7ff693446a2bf595e8932b3e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Fri, 15 Jul 2016 17:00:57 +0200 Subject: [PATCH] Fix mistakes introduced with QoSSpecs object If15ea8b628a6f88211a5d5cc7aadff44f7840138 introduced some mistakes that I was able to identify when rebasing RequestSpec object patch: * Missing loading volume_types relationship in QualityOfServiceSpecs _from_db_object. * Wrong source of qos_specs relationship in VolumeType _from_db_object. * Inefficient loading od QoSSpecs associations in db layer. * Use of self in classmethod. This commit fixes the issues. Change-Id: I3b78127791a03e1a35a30d7f84f66e97d89c7cf9 Closes-Bug: 1603472 --- cinder/db/sqlalchemy/api.py | 8 +++++--- cinder/objects/qos_specs.py | 10 +++++++++- cinder/objects/volume_type.py | 8 +++----- cinder/tests/unit/test_qos_specs.py | 2 ++ cinder/volume/qos_specs.py | 6 +++--- tools/lintstack.py | 2 ++ 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 83bf1c244ba..502b79d82f2 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -3089,9 +3089,11 @@ def volume_type_qos_associations_get(context, qos_specs_id, inactive=False): models.QualityOfServiceSpecs, qos_specs_id): raise exception.QoSSpecsNotFound(specs_id=qos_specs_id) - return model_query(context, models.VolumeTypes, - read_deleted=read_deleted). \ - filter_by(qos_specs_id=qos_specs_id).all() + vts = (model_query(context, models.VolumeTypes, read_deleted=read_deleted). + options(joinedload('extra_specs')). + options(joinedload('projects')). + filter_by(qos_specs_id=qos_specs_id).all()) + return vts @require_admin_context diff --git a/cinder/objects/qos_specs.py b/cinder/objects/qos_specs.py index 467d373069a..d9b63cd348a 100644 --- a/cinder/objects/qos_specs.py +++ b/cinder/objects/qos_specs.py @@ -113,7 +113,10 @@ class QualityOfServiceSpecs(base.CinderPersistentObject, self._context, self.id) @staticmethod - def _from_db_object(context, qos_spec, db_qos_spec): + def _from_db_object(context, qos_spec, db_qos_spec, expected_attrs=None): + if expected_attrs is None: + expected_attrs = [] + for name, field in qos_spec.fields.items(): if name not in QualityOfServiceSpecs.OPTIONAL_FIELDS: value = db_qos_spec.get(name) @@ -123,6 +126,11 @@ class QualityOfServiceSpecs(base.CinderPersistentObject, value = {} setattr(qos_spec, name, value) + if 'volume_types' in expected_attrs: + volume_types = objects.VolumeTypeList.get_all_types_for_qos( + context, db_qos_spec['id']) + qos_spec.volume_types = volume_types + qos_spec._context = context qos_spec.obj_reset_changes() return qos_spec diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index b36305db97f..5f8debb8d08 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -88,9 +88,7 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject, type.projects = db_type.get('projects', []) if 'qos_specs' in expected_attrs: qos_specs = objects.QualityOfServiceSpecs(context) - qos_specs._from_db_object(context, - qos_specs, - type['qos_specs']) + qos_specs._from_db_object(context, qos_specs, db_type['qos_specs']) type.qos_specs = qos_specs type._context = context type.obj_reset_changes() @@ -143,7 +141,7 @@ class VolumeTypeList(base.ObjectListBase, base.CinderObject): expected_attrs=expected_attrs) @classmethod - def get_all_types_for_qos(self, context, qos_id): + def get_all_types_for_qos(cls, context, qos_id): types = db.qos_specs_associations_get(context, qos_id) - return base.obj_make_list(context, self(context), objects.VolumeType, + return base.obj_make_list(context, cls(context), objects.VolumeType, types) diff --git a/cinder/tests/unit/test_qos_specs.py b/cinder/tests/unit/test_qos_specs.py index 68a747d5f71..31aa644279e 100644 --- a/cinder/tests/unit/test_qos_specs.py +++ b/cinder/tests/unit/test_qos_specs.py @@ -231,6 +231,8 @@ class QoSSpecsTestCase(test.TestCase): } self.assertIn(expected_type, res) + e = exception.QoSSpecsNotFound(specs_id='Trouble') + mock_qos_specs_associations_get.side_effect = e self.assertRaises(exception.CinderException, qos_specs.get_associations, self.ctxt, 'Trouble') diff --git a/cinder/volume/qos_specs.py b/cinder/volume/qos_specs.py index 85fea67c7e4..6f0d439d2ac 100644 --- a/cinder/volume/qos_specs.py +++ b/cinder/volume/qos_specs.py @@ -135,8 +135,8 @@ def delete_keys(context, qos_specs_id, keys): def get_associations(context, qos_specs_id): """Get all associations of given qos specs.""" try: - qos_spec = objects.QualityOfServiceSpecs.get_by_id( - context, qos_specs_id) + types = objects.VolumeTypeList.get_all_types_for_qos(context, + qos_specs_id) except db_exc.DBError: LOG.exception(_LE('DB error:')) msg = _('Failed to get all associations of ' @@ -145,7 +145,7 @@ def get_associations(context, qos_specs_id): raise exception.CinderException(message=msg) result = [] - for vol_type in qos_spec.volume_types: + for vol_type in types: result.append({ 'association_type': 'volume_type', 'name': vol_type.name, diff --git a/tools/lintstack.py b/tools/lintstack.py index 6def20f1498..385cf46db07 100755 --- a/tools/lintstack.py +++ b/tools/lintstack.py @@ -95,6 +95,8 @@ objects_ignore_messages = [ "Module 'cinder.objects' has no 'SnapshotList' member", "Module 'cinder.objects' has no 'Volume' member", "Module 'cinder.objects' has no 'VolumeList' member", + "Module 'cinder.objects' has no 'VolumeType' member", + "Module 'cinder.objects' has no 'VolumeTypeList' member", ] objects_ignore_modules = ["cinder/objects/"]