openstack/cinder
Code Issues Proposed changes

Fix finish_volume_migration() on SQLAlchemy 0.8.x

Browse Source 
In SQAlchemy 0.8.x "Unconsumed column names" warning became
an exception, so refering to a non-existent column in insert()
or update() call raises an error.

finish_volume_migration() calls Session.update() method passing
values of two non-existent columns as arguments (volume_metadata,
volume_admin_metadata, volume_type). These two are not table columns
at all, but rather SQLAlchemy models relationships.

As SQLAlchemy ORM implements Unity of Work pattern, we should not
really track changes to a model instance manually at all, because
Session class already does it for us. finish_volume_migration()
is refactored to take benefit of this fact.

Fixes bug 1206561

Change-Id: I4513e3155a7dc6dcbd1c95aa9c14d1e1e5d02ab4
This commit is contained in:
Roman Podolyaka 2013-09-23 17:58:54 +03:00
parent 180513d5e4
commit 36fab5ae26
1 changed files with 16 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1100,25 +1100,27 @@ def volume_data_get_for_project(context, project_id, volume_type_id=None):
@require_admin_context @require_admin_context
def finish_volume_migration(context, src_vol_id, dest_vol_id): def finish_volume_migration(context, src_vol_id, dest_vol_id):
"""Copy almost all columns from dest to source, then delete dest.""" """Copy almost all columns from dest to source."""
session = get_session() session = get_session()
with session.begin(): with session.begin():
src_volume_ref = _volume_get(context, src_vol_id, session=session)
dest_volume_ref = _volume_get(context, dest_vol_id, session=session) dest_volume_ref = _volume_get(context, dest_vol_id, session=session)
updates = {}
if dest_volume_ref['_name_id']: # NOTE(rpodolyaka): we should copy only column values, while model
updates['_name_id'] = dest_volume_ref['_name_id'] # instances also have relationships attributes, which
else: # should be ignored
updates['_name_id'] = dest_volume_ref['id'] def is_column(inst, attr):
return attr in inst.__class__.__table__.columns
for key, value in dest_volume_ref.iteritems(): for key, value in dest_volume_ref.iteritems():
if key in ['id', '_name_id']: if key == 'id' or not is_column(dest_volume_ref, key):
continue continue
if key == 'migration_status': elif key == 'migration_status':
updates[key] = None value = None
continue elif key == '_name_id':
updates[key] = value value = dest_volume_ref['_name_id'] or dest_volume_ref['id']
session.query(models.Volume).\
filter_by(id=src_vol_id).\ setattr(src_volume_ref, key, value)
update(updates)
@require_admin_context @require_admin_context