From 6de9e979d11345a88ac0d5d061772aa6d55a40cc Mon Sep 17 00:00:00 2001
From: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Date: Fri, 27 Mar 2015 15:35:05 -0400
Subject: [PATCH] Fix volume creation from image with
 allowed_direct_url_schemes

When CONF.allowed_direct_url_schemes is set to ['file'] and
Glance image has direct_url or locations metadata with 'file:///'
scheme, cinder tries to directly read the image data from the
local file. However, currently the code is broken because return
value from get_location() has been changed to a tuple.
This fixes the code so that it can handle the metadata correctly.

Change-Id: I39a12a31fbfbd3a9824c67391096f74406d8a749
Closes-Bug: #1437477
(cherry picked from commit 181be53a2b0a54f7b4cd33dacbaa560e124e9421)
---
 cinder/image/glance.py            | 23 ++++++++++++++---------
 cinder/tests/image/test_glance.py | 25 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/cinder/image/glance.py b/cinder/image/glance.py
index 16340d5..80e97d2 100644
--- a/cinder/image/glance.py
+++ b/cinder/image/glance.py
@@ -239,8 +239,9 @@ class GlanceImageService(object):
         return base_image_meta
 
     def get_location(self, context, image_id):
-        """Returns the direct url representing the backend storage location,
-        or None if this attribute is not shown by Glance.
+        """Returns a tuple of the direct url and locations representing the
+        backend storage location, or (None, None) if these attributes are not
+        shown by Glance.
         """
         if CONF.glance_api_version == 1:
             # image location not available in v1
@@ -263,16 +264,20 @@ class GlanceImageService(object):
 
     def download(self, context, image_id, data=None):
         """Calls out to Glance for data and writes data."""
-        if 'file' in CONF.allowed_direct_url_schemes:
-            location = self.get_location(context, image_id)
-            o = urlparse.urlparse(location)
-            if o.scheme == "file":
-                with open(o.path, "r") as f:
+        if data and 'file' in CONF.allowed_direct_url_schemes:
+            direct_url, locations = self.get_location(context, image_id)
+            urls = [direct_url] + [loc.get('url') for loc in locations or []]
+            for url in urls:
+                if url is None:
+                    continue
+                parsed_url = urlparse.urlparse(url)
+                if parsed_url.scheme == "file":
                     # a system call to cp could have significant performance
                     # advantages, however we do not have the path to files at
                     # this point in the abstraction.
-                    shutil.copyfileobj(f, data)
-                return
+                    with open(parsed_url.path, "r") as f:
+                        shutil.copyfileobj(f, data)
+                    return
 
         try:
             image_chunks = self._client.call(context, 'data', image_id)
diff --git a/cinder/tests/image/test_glance.py b/cinder/tests/image/test_glance.py
index b252ac9..f7436a4 100644
--- a/cinder/tests/image/test_glance.py
+++ b/cinder/tests/image/test_glance.py
@@ -519,6 +519,31 @@ class TestGlanceImageService(test.TestCase):
         self.assertRaises(exception.ImageNotFound, service.download,
                           self.context, image_id, writer)
 
+    @mock.patch('__builtin__.open')
+    @mock.patch('shutil.copyfileobj')
+    def test_download_from_direct_file(self, mock_copyfileobj, mock_open):
+        fixture = self._make_fixture(name='test image',
+                                     locations=[{'url': 'file:///tmp/test'}])
+        image_id = self.service.create(self.context, fixture)['id']
+        writer = NullWriter()
+        self.flags(allowed_direct_url_schemes=['file'])
+        self.flags(glance_api_version=2)
+        self.service.download(self.context, image_id, writer)
+        mock_copyfileobj.assert_called_once_with(mock.ANY, writer)
+
+    @mock.patch('__builtin__.open')
+    @mock.patch('shutil.copyfileobj')
+    def test_download_from_direct_file_non_file(self,
+                                                mock_copyfileobj, mock_open):
+        fixture = self._make_fixture(name='test image',
+                                     direct_url='swift+http://test/image')
+        image_id = self.service.create(self.context, fixture)['id']
+        writer = NullWriter()
+        self.flags(allowed_direct_url_schemes=['file'])
+        self.flags(glance_api_version=2)
+        self.service.download(self.context, image_id, writer)
+        self.assertEqual(None, mock_copyfileobj.call_args)
+
     def test_glance_client_image_id(self):
         fixture = self._make_fixture(name='test image')
         image_id = self.service.create(self.context, fixture)['id']
-- 
2.5.1

