From 36fab5ae26579c3cc68ef4ca02bdab3c232ce648 Mon Sep 17 00:00:00 2001 From: Roman Podolyaka Date: Mon, 23 Sep 2013 17:58:54 +0300 Subject: [PATCH] Fix finish_volume_migration() on SQLAlchemy 0.8.x 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 --- cinder/db/sqlalchemy/api.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 79aa0b4b7..36f9e8643 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1100,25 +1100,27 @@ def volume_data_get_for_project(context, project_id, volume_type_id=None): @require_admin_context 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() 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) - updates = {} - if dest_volume_ref['_name_id']: - updates['_name_id'] = dest_volume_ref['_name_id'] - else: - updates['_name_id'] = dest_volume_ref['id'] + + # NOTE(rpodolyaka): we should copy only column values, while model + # instances also have relationships attributes, which + # should be ignored + def is_column(inst, attr): + return attr in inst.__class__.__table__.columns + 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 - if key == 'migration_status': - updates[key] = None - continue - updates[key] = value - session.query(models.Volume).\ - filter_by(id=src_vol_id).\ - update(updates) + elif key == 'migration_status': + value = None + elif key == '_name_id': + value = dest_volume_ref['_name_id'] or dest_volume_ref['id'] + + setattr(src_volume_ref, key, value) @require_admin_context -- 2.45.2