openstack/nova
Code Issues Proposed changes

optimize libvirt raw image handling. Bug 924970

Browse Source 
Tests were seen to time-out on libvirt when raw images were
used, which was due to large disk images being copied around
inefficiently.  A system with standard disks was seen to take
an extra 60s/10G which was a problem with large root and
ephemeral disks.  The changes below attempt to minimize the
I/O in dealing with cached raw images.  These changes should
also help to minimize the disk space used for such images,
by avoiding the naïve copy which undoes the sparseness.

* nova/virt/libvirt/connection.py (_cache_image): Do the
resize here, rather than in _fetch_image(), so that we
can control when the resizing is done, to minimize the
amount of data that needs to be copied.  Also if we're
generating rather than fetching the image, then just
generate in the instance dir too, as this should be faster.
* nova/tests/fake_libvirt_utils.py: Remove the resize
functionality since it's no longer used.
* nova/tests/test_libvirt.py: Likewise.
* nova/virt/libvirt/utils.py (fetch_image): Likewise.
(copy_image): Shell out to cp since it deals better
with sparse files.  Note the above changes avoid sparse
copies, so this is just an ancillary improvement in the area.

Change-Id: I678d125c61aab56c62c668559eb2220d56702952
This commit is contained in:
Pádraig Brady
2012-01-31 23:46:42 +00:00
parent b2cd9068af
commit d20b48bd96
4 changed files with 51 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
nova/tests/fake_libvirt_utils.py
View File

@@ -99,6 +99,5 @@ def get_fs_info(path):
'free': 84 * (1024 ** 3)}
def fetch_image(context, target, image_id, user_id, project_id,
size=None):
def fetch_image(context, target, image_id, user_id, project_id):
pass

24
nova/tests/test_libvirt.py
View File

@@ -210,12 +210,17 @@ class CacheConcurrencyTestCase(test.TestCase):
def fake_execute(*args, **kwargs):
pass
def fake_extend(image, size):
pass
self.stubs.Set(os.path, 'exists', fake_exists)
self.stubs.Set(utils, 'execute', fake_execute)
connection.libvirt_utils = fake_libvirt_utils
connection.disk.extend = fake_extend
def tearDown(self):
connection.libvirt_utils = libvirt_utils
connection.disk.extend = disk.extend
super(CacheConcurrencyTestCase, self).tearDown()
def test_same_fname_concurrency(self):
@@ -224,11 +229,11 @@ class CacheConcurrencyTestCase(test.TestCase):
wait1 = eventlet.event.Event()
done1 = eventlet.event.Event()
eventlet.spawn(conn._cache_image, _concurrency,
'target', 'fname', False, wait1, done1)
'target', 'fname', False, None, wait1, done1)
wait2 = eventlet.event.Event()
done2 = eventlet.event.Event()
eventlet.spawn(conn._cache_image, _concurrency,
'target', 'fname', False, wait2, done2)
'target', 'fname', False, None, wait2, done2)
wait2.send()
eventlet.sleep(0)
try:
@@ -245,11 +250,11 @@ class CacheConcurrencyTestCase(test.TestCase):
wait1 = eventlet.event.Event()
done1 = eventlet.event.Event()
eventlet.spawn(conn._cache_image, _concurrency,
'target', 'fname2', False, wait1, done1)
'target', 'fname2', False, None, wait1, done1)
wait2 = eventlet.event.Event()
done2 = eventlet.event.Event()
eventlet.spawn(conn._cache_image, _concurrency,
'target', 'fname1', False, wait2, done2)
'target', 'fname1', False, None, wait2, done2)
wait2.send()
eventlet.sleep(0)
try:
@@ -292,8 +297,14 @@ class LibvirtConnTestCase(test.TestCase):
self.call_libvirt_dependant_setup = False
connection.libvirt_utils = fake_libvirt_utils
def fake_extend(image, size):
pass
connection.disk.extend = fake_extend
def tearDown(self):
connection.libvirt_utils = libvirt_utils
connection.disk.extend = disk.extend
super(LibvirtConnTestCase, self).tearDown()
test_instance = {'memory_kb': '1024000',
@@ -1915,7 +1926,6 @@ disk size: 4.4M''', ''))
def test_fetch_image(self):
self.mox.StubOutWithMock(images, 'fetch')
self.mox.StubOutWithMock(disk, 'extend')
context = 'opaque context'
target = '/tmp/targetfile'
@@ -1923,11 +1933,7 @@ disk size: 4.4M''', ''))
user_id = 'fake'
project_id = 'fake'
images.fetch(context, image_id, target, user_id, project_id)
images.fetch(context, image_id, target, user_id, project_id)
disk.extend(target, '10G')
self.mox.ReplayAll()
libvirt_utils.fetch_image(context, target, image_id,
user_id, project_id)
libvirt_utils.fetch_image(context, target, image_id,
user_id, project_id, size='10G')

36
nova/virt/libvirt/connection.py
View File

@@ -799,7 +799,7 @@ class LibvirtConnection(driver.ComputeDriver):
return {'host': host, 'port': port, 'internal_access_path': None}
@staticmethod
def _cache_image(fn, target, fname, cow=False, *args, **kwargs):
def _cache_image(fn, target, fname, cow=False, size=None, *args, **kwargs):
"""Wrapper for a method that creates an image that caches the image.
This wrapper will save the image into a common store and create a
@@ -812,8 +812,12 @@ class LibvirtConnection(driver.ComputeDriver):
to be unique to a given image.
If cow is True, it will make a CoW image instead of a copy.
If size is specified, we attempt to resize up to that size.
"""
generating = 'image_id' not in kwargs
if not os.path.exists(target):
base_dir = os.path.join(FLAGS.instances_path, '_base')
if not os.path.exists(base_dir):
@@ -825,20 +829,36 @@ class LibvirtConnection(driver.ComputeDriver):
if not os.path.exists(base):
fn(target=base, *args, **kwargs)
call_if_not_exists(base, fn, *args, **kwargs)
if cow or not generating:
call_if_not_exists(base, fn, *args, **kwargs)
elif generating:
# For raw it's quicker to generate both
# FIXME(p-draigbrady) the first call here is probably
# redundant, as it's of no benefit to cache in base?
call_if_not_exists(base, fn, *args, **kwargs)
if os.path.exists(target):
os.unlink(target)
fn(target=target, *args, **kwargs)
if cow:
if size:
disk.extend(base, size)
libvirt_utils.create_cow_image(base, target)
else:
elif not generating:
libvirt_utils.copy_image(base, target)
# Resize after the copy, as it's usually much faster
# to make sparse updates, rather than potentially
# naively copying the whole image file.
if size:
# FIXME(p-draigbrady) the first call here is probably
# redundant, as it's of no benefit have full size in base?
disk.extend(base, size)
disk.extend(target, size)
@staticmethod
def _fetch_image(context, target, image_id, user_id, project_id,
size=None):
"""Grab image and optionally attempt to resize it"""
def _fetch_image(context, target, image_id, user_id, project_id):
"""Grab image to raw format"""
images.fetch_to_raw(context, image_id, target, user_id, project_id)
if size:
disk.extend(target, size)
@staticmethod
def _create_local(target, local_size, unit='G', fs_format=None):

13
nova/virt/libvirt/utils.py
View File

@@ -103,7 +103,11 @@ def copy_image(src, dest):
:param src: Source image
:param dest: Destination path
"""
shutil.copyfile(src, dest)
# We shell out to cp because that will intelligently copy
# sparse files. I.E. holes will not be written to DEST,
# rather recreated efficiently. In addition, since
# coreutils 8.11, holes can be read efficiently too.
execute('cp', src, dest)
def mkfs(fs, path):
@@ -254,9 +258,6 @@ def get_fs_info(path):
'used': used}
def fetch_image(context, target, image_id, user_id, project_id,
size=None):
"""Grab image and optionally attempt to resize it"""
def fetch_image(context, target, image_id, user_id, project_id):
"""Grab image"""
images.fetch(context, image_id, target, user_id, project_id)
if size:
disk.extend(target, size)