openstack/cinder
Code Issues Proposed changes

Fix volume creation from image with allowed_direct_url_schemes

Browse Source 
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
This commit is contained in:
Tomoki Sekiyama 2015-03-27 15:35:05 -04:00
parent fac178fd58
commit 181be53a2b
2 changed files with 39 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
cinder/image/glance.py
View File

@ -242,8 +242,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
@ -266,16 +267,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)

25
cinder/tests/unit/image/test_glance.py
View File

@ -518,6 +518,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']