openstack
/
cinder
Code Issues Proposed changes

Remove volid from attachments_get_by_host|instance 

Attachments_get_by_host|instance should be just that, associating
them with a specified volume-id doesn't really solve the problem.

If that relationship is needed a simple get_by_volume will work with
some inspection.

This patch removes the volume_id arg from those get methods.  A follow
up patch will add get_all with filters for more complex relationships.

Change-Id: Ic5ffdced96fdf780cce2a1227c5f2a599860f0ca
Closes-Bug: #1632433
This commit is contained in:
John Griffith 2016-10-11 12:40:53 -06:00
parent 14899a9a8e
commit 6f174b4126
8 changed files with 16 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cinder/db/api.py
View File

@ -344,14 +344,13 @@ def volume_attachment_get_all_by_volume_id(context, volume_id):
return IMPL.volume_attachment_get_all_by_volume_id(context, volume_id)
def volume_attachment_get_all_by_host(context, volume_id, host):
return IMPL.volume_attachment_get_all_by_host(context, volume_id, host)
def volume_attachment_get_all_by_host(context, host):
return IMPL.volume_attachment_get_all_by_host(context, host)
def volume_attachment_get_all_by_instance_uuid(context,
volume_id,
instance_uuid):
return IMPL.volume_attachment_get_all_by_instance_uuid(context, volume_id,
return IMPL.volume_attachment_get_all_by_instance_uuid(context,
instance_uuid)

5
cinder/db/sqlalchemy/api.py
View File

@ -1721,12 +1721,11 @@ def volume_attachment_get_all_by_volume_id(context, volume_id, session=None):
@require_context
def volume_attachment_get_all_by_host(context, volume_id, host):
def volume_attachment_get_all_by_host(context, host):
session = get_session()
with session.begin():
result = model_query(context, models.VolumeAttachment,
session=session).\
filter_by(volume_id=volume_id).\
filter_by(attached_host=host).\
filter(models.VolumeAttachment.attach_status != 'detached').\
all()
@ -1735,13 +1734,11 @@ def volume_attachment_get_all_by_host(context, volume_id, host):
@require_context
def volume_attachment_get_all_by_instance_uuid(context,
volume_id,
instance_uuid):
session = get_session()
with session.begin():
result = model_query(context, models.VolumeAttachment,
session=session).\
filter_by(volume_id=volume_id).\
filter_by(instance_uuid=instance_uuid).\
filter(models.VolumeAttachment.attach_status != 'detached').\
all()

1
cinder/objects/base.py
View File

@ -118,6 +118,7 @@ OBJ_VERSIONS.add('1.11', {'GroupSnapshot': '1.0', 'GroupSnapshotList': '1.0',
'Group': '1.1'})
OBJ_VERSIONS.add('1.12', {'VolumeType': '1.3'})
OBJ_VERSIONS.add('1.13', {'CleanupRequest': '1.0'})
OBJ_VERSIONS.add('1.14', {'VolumeAttachmentList': '1.1'})
class CinderObjectRegistry(base.VersionedObjectRegistry):

11
cinder/objects/volume_attachment.py
View File

@ -61,7 +61,9 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
@base.CinderObjectRegistry.register
class VolumeAttachmentList(base.ObjectListBase, base.CinderObject):
VERSION = '1.0'
# Versoin 1.0: Iniitial version
# Version 1.1: Remove volume_id in get_by_host|instance
VERSION = '1.1'
fields = {
'objects': fields.ListOfObjectsField('VolumeAttachment'),
@ -77,16 +79,15 @@ class VolumeAttachmentList(base.ObjectListBase, base.CinderObject):
attachments)
@classmethod
def get_all_by_host(cls, context, volume_id, host):
def get_all_by_host(cls, context, host):
attachments = db.volume_attachment_get_all_by_host(context,
volume_id,
host)
return base.obj_make_list(context, cls(context),
objects.VolumeAttachment, attachments)
@classmethod
def get_all_by_instance_uuid(cls, context, volume_id, instance_uuid):
def get_all_by_instance_uuid(cls, context, instance_uuid):
attachments = db.volume_attachment_get_all_by_instance_uuid(
context, volume_id, instance_uuid)
context, instance_uuid)
return base.obj_make_list(context, cls(context),
objects.VolumeAttachment, attachments)

2
cinder/tests/unit/objects/test_objects.py
View File

@ -43,7 +43,7 @@ object_data = {
'Volume': '1.5-19919d8086d6a38ab9d3ab88139e70e0',
'VolumeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'VolumeAttachment': '1.0-b30dacf62b2030dd83d8a1603f1064ff',
'VolumeAttachmentList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'VolumeAttachmentList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'VolumeProperties': '1.1-cadac86b2bdc11eb79d1dcea988ff9e8',
'VolumeType': '1.3-a5d8c3473db9bc3bbcdbab9313acf4d1',
'VolumeTypeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',

4
cinder/tests/unit/objects/test_volume_attachment.py
View File

@ -84,7 +84,7 @@ class TestVolumeAttachmentList(test_objects.BaseObjectsTestCase):
get_by_host.return_value = [db_attachment]
attachments = objects.VolumeAttachmentList.get_all_by_host(
self.context, mock.sentinel.volume_id, mock.sentinel.host)
self.context, mock.sentinel.host)
self.assertEqual(1, len(attachments))
TestVolumeAttachment._compare(self, db_attachment, attachments[0])
@ -94,6 +94,6 @@ class TestVolumeAttachmentList(test_objects.BaseObjectsTestCase):
get_by_instance_uuid.return_value = [db_attachment]
attachments = objects.VolumeAttachmentList.get_all_by_instance_uuid(
self.context, mock.sentinel.volume_id, mock.sentinel.uuid)
self.context, mock.sentinel.uuid)
self.assertEqual(1, len(attachments))
TestVolumeAttachment._compare(self, db_attachment, attachments[0])

2
cinder/tests/unit/test_volume.py
View File

@ -5225,7 +5225,7 @@ class VolumeMigrationTestCase(BaseVolumeTestCase):
old_volume.id,
attachment_id)
attachments = db.volume_attachment_get_all_by_instance_uuid(
self.context, old_volume.id, instance_uuid)
self.context, instance_uuid)
self.assertIsNotNone(attachments)
self.assertEqual(attached_host,
attachments[0]['attached_host'])

3
cinder/volume/manager.py
View File

@ -932,12 +932,11 @@ class VolumeManager(manager.SchedulerDependentManager):
if instance_uuid:
attachments = \
self.db.volume_attachment_get_all_by_instance_uuid(
context, volume_id, instance_uuid)
context, instance_uuid)
else:
attachments = (
self.db.volume_attachment_get_all_by_host(
context,
volume_id,
host_name_sanitized))
if attachments:
self.db.volume_update(context, volume_id,