From: Roman Podolyaka <rpodolyaka@mirantis.com>
Date: Mon, 23 Sep 2013 14:58:54 +0000 (+0300)
Subject: Fix finish_volume_migration() on SQLAlchemy 0.8.x
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=36fab5ae26579c3cc68ef4ca02bdab3c232ce648;p=openstack-build%2Fcinder-build.git

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
---

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