From 9fe4477f9b1f4e825941cb1ce42ced1a1a862b33 Mon Sep 17 00:00:00 2001
From: jking-6 <jking@internap.com>
Date: Wed, 24 Oct 2012 13:15:03 -0400
Subject: [PATCH] Allow the user to update a volume's metadata

The fix mainly involves updating the VolumeController's update method
to allow the 'metadata' key to be a valid key in a request body.

The _translate_volume_summary_view method is updated to check for a
'metadata' key in the vol parameter after checking first for
'volume_metadata'.  This is to avoid making a third call to the
database after updating the volume model.  This solution introduces
the least amount of new code.

Change-Id: I41256d03f9c4c7a0866ff0f2d35fce8e4fd26b11
Fixes: bug #1046382
---
 cinder/api/openstack/volume/volumes.py        |  4 +++
 cinder/db/sqlalchemy/api.py                   | 12 ++++----
 .../api/openstack/volume/test_volumes.py      | 28 +++++++++++++++++++
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/cinder/api/openstack/volume/volumes.py b/cinder/api/openstack/volume/volumes.py
index 69d55c6bd..b53a249f0 100644
--- a/cinder/api/openstack/volume/volumes.py
+++ b/cinder/api/openstack/volume/volumes.py
@@ -107,6 +107,9 @@ def _translate_volume_summary_view(context, vol, image_id=None):
     if vol.get('volume_metadata'):
         metadata = vol.get('volume_metadata')
         d['metadata'] = dict((item['key'], item['value']) for item in metadata)
+    # avoid circular ref when vol is a Volume instance
+    elif vol.get('metadata') and isinstance(vol.get('metadata'), dict):
+        d['metadata'] = vol['metadata']
     else:
         d['metadata'] = {}
 
@@ -355,6 +358,7 @@ class VolumeController(wsgi.Controller):
         valid_update_keys = (
             'display_name',
             'display_description',
+            'metadata',
         )
 
         for key in valid_update_keys:
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 8a560419a..63f1f1f4b 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1065,9 +1065,9 @@ def volume_update(context, volume_id, values):
     metadata = values.get('metadata')
     if metadata is not None:
         volume_metadata_update(context,
-                                volume_id,
-                                values.pop('metadata'),
-                                delete=True)
+                               volume_id,
+                               values.pop('metadata'),
+                               delete=True)
     with session.begin():
         volume_ref = volume_get(context, volume_id, session=session)
         volume_ref.update(values)
@@ -1134,15 +1134,15 @@ def volume_metadata_update(context, volume_id, metadata, delete):
     meta_ref = None
 
     # Now update all existing items with new values, or create new meta objects
-    for meta_key, meta_value in metadata.iteritems():
+    for meta_key, meta_value in metadata.items():
 
         # update the value whether it exists or not
         item = {"value": meta_value}
 
         try:
             meta_ref = volume_metadata_get_item(context, volume_id,
-                                                  meta_key, session)
-        except exception.VolumeMetadataNotFound, e:
+                                                meta_key, session)
+        except exception.VolumeMetadataNotFound as e:
             meta_ref = models.VolumeMetadata()
             item.update({"key": meta_key, "volume_id": volume_id})
 
diff --git a/cinder/tests/api/openstack/volume/test_volumes.py b/cinder/tests/api/openstack/volume/test_volumes.py
index f5141f631..ec57e5a46 100644
--- a/cinder/tests/api/openstack/volume/test_volumes.py
+++ b/cinder/tests/api/openstack/volume/test_volumes.py
@@ -207,6 +207,34 @@ class VolumeApiTest(test.TestCase):
         }}
         self.assertEquals(res_dict, expected)
 
+    def test_volume_update_metadata(self):
+        self.stubs.Set(volume_api.API, "update", fakes.stub_volume_update)
+        updates = {
+            "metadata": {"qos_max_iops": 2000}
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v1/volumes/1')
+        res_dict = self.controller.update(req, '1', body)
+        expected = {'volume': {
+            'status': 'fakestatus',
+            'display_description': 'displaydesc',
+            'availability_zone': 'fakeaz',
+            'display_name': 'displayname',
+            'attachments': [{
+                'id': '1',
+                'volume_id': '1',
+                'server_id': 'fakeuuid',
+                'device': '/',
+            }],
+            'volume_type': 'vol_type_name',
+            'snapshot_id': None,
+            'metadata': {"qos_max_iops": 2000},
+            'id': '1',
+            'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+            'size': 1,
+        }}
+        self.assertEquals(res_dict, expected)
+
     def test_update_empty_body(self):
         body = {}
         req = fakes.HTTPRequest.blank('/v1/volumes/1')
-- 
2.45.2