From 7c265343159bb76e0322f150f8d194ecd342b637 Mon Sep 17 00:00:00 2001 From: Pádraig Brady Date: Fri, 16 Mar 2012 03:43:49 +0000 Subject: ensure atomic manipulation of libvirt disk images This pattern could probably be used elsewhere, but only libvirt disk images are considered for now. This change ensures there are no stale files left anywhere in the path from glance, through the libvirt image cache. These could cause subsequent operational errors either directly or indirectly through disk wastage. * nova/utils.py: Add a new remove_path_on_error() context manager that is used to remove the passed PATH on a raised exception. * nova/virt/images.py: Ensure temporary downloaded and converted images are protected. * nova/virt/libvirt/connection.py: Ensure all the images in the image cache and instance dirs are protected. Change-Id: I81a5407665a6998128c0dee41387ef00ebddeb4d --- nova/utils.py | 21 ++++++++++--- nova/virt/images.py | 69 ++++++++++++++++++----------------------- nova/virt/libvirt/connection.py | 16 ++++++---- 3 files changed, 57 insertions(+), 49 deletions(-) (limited to 'nova') diff --git a/nova/utils.py b/nova/utils.py index 29df28cea..8119861c6 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -21,6 +21,7 @@ import contextlib import datetime +import errno import functools import hashlib import inspect @@ -1014,8 +1015,8 @@ def cleanup_file_locks(): continue try: stat_info = os.stat(os.path.join(FLAGS.lock_path, filename)) - except OSError as (errno, strerror): - if errno == 2: # doesn't exist + except OSError as e: + if e.errno == errno.ENOENT: continue else: raise @@ -1034,8 +1035,8 @@ def delete_if_exists(pathname): try: os.unlink(pathname) - except OSError as (errno, strerror): - if errno == 2: # doesn't exist + except OSError as e: + if e.errno == errno.ENOENT: return else: raise @@ -1345,6 +1346,18 @@ def logging_error(message): LOG.exception(message) +@contextlib.contextmanager +def remove_path_on_error(path): + """Protect code that wants to operate on PATH atomically. + Any exception will cause PATH to be removed. + """ + try: + yield + except Exception: + with save_and_reraise_exception(): + delete_if_exists(path) + + def make_dev_path(dev, partition=None, base='/dev'): """Return a path to a particular device. diff --git a/nova/virt/images.py b/nova/virt/images.py index 1e0ae0a22..626f3ff76 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -51,18 +51,10 @@ def fetch(context, image_href, path, _user_id, _project_id): # checked before we got here. (image_service, image_id) = nova.image.get_image_service(context, image_href) - try: + with utils.remove_path_on_error(path): with open(path, "wb") as image_file: metadata = image_service.get(context, image_id, image_file) - except Exception: - with utils.save_and_reraise_exception(): - try: - os.unlink(path) - except OSError, e: - if e.errno != errno.ENOENT: - LOG.warn("unable to remove stale image '%s': %s" % - (path, e.strerror)) - return metadata + return metadata def fetch_to_raw(context, image_href, path, user_id, project_id): @@ -85,37 +77,36 @@ def fetch_to_raw(context, image_href, path, user_id, project_id): return(data) - data = _qemu_img_info(path_tmp) - - fmt = data.get("file format") - if fmt is None: - os.unlink(path_tmp) - raise exception.ImageUnacceptable( - reason=_("'qemu-img info' parsing failed."), image_id=image_href) - - if "backing file" in data: - backing_file = data['backing file'] - os.unlink(path_tmp) - raise exception.ImageUnacceptable(image_id=image_href, - reason=_("fmt=%(fmt)s backed by: %(backing_file)s") % locals()) - - if fmt != "raw" and FLAGS.force_raw_images: - staged = "%s.converted" % path - LOG.debug("%s was %s, converting to raw" % (image_href, fmt)) - out, err = utils.execute('qemu-img', 'convert', '-O', 'raw', - path_tmp, staged) - os.unlink(path_tmp) - - data = _qemu_img_info(staged) - if data.get('file format', None) != "raw": - os.unlink(staged) + with utils.remove_path_on_error(path_tmp): + data = _qemu_img_info(path_tmp) + + fmt = data.get("file format") + if fmt is None: + raise exception.ImageUnacceptable( + reason=_("'qemu-img info' parsing failed."), + image_id=image_href) + + if "backing file" in data: + backing_file = data['backing file'] raise exception.ImageUnacceptable(image_id=image_href, - reason=_("Converted to raw, but format is now %s") % - data.get('file format', None)) + reason=_("fmt=%(fmt)s backed by: %(backing_file)s") % locals()) + + if fmt != "raw" and FLAGS.force_raw_images: + staged = "%s.converted" % path + LOG.debug("%s was %s, converting to raw" % (image_href, fmt)) + with utils.remove_path_on_error(staged): + out, err = utils.execute('qemu-img', 'convert', '-O', 'raw', + path_tmp, staged) + + data = _qemu_img_info(staged) + if data.get('file format', None) != "raw": + raise exception.ImageUnacceptable(image_id=image_href, + reason=_("Converted to raw, but format is now %s") % + data.get('file format', None)) - os.rename(staged, path) + os.rename(staged, path) - else: - os.rename(path_tmp, path) + else: + os.rename(path_tmp, path) return metadata diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index d41f4551c..97deae048 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -1047,7 +1047,8 @@ class LibvirtConnection(driver.ComputeDriver): @utils.synchronized(fname) def call_if_not_exists(base, fn, *args, **kwargs): if not os.path.exists(base): - fn(target=base, *args, **kwargs) + with utils.remove_path_on_error(base): + fn(target=base, *args, **kwargs) if cow or not generating: call_if_not_exists(base, fn, *args, **kwargs) @@ -1063,8 +1064,9 @@ class LibvirtConnection(driver.ComputeDriver): size_gb = size / (1024 * 1024 * 1024) cow_base += "_%d" % size_gb if not os.path.exists(cow_base): - libvirt_utils.copy_image(base, cow_base) - disk.extend(cow_base, size) + with utils.remove_path_on_error(cow_base): + libvirt_utils.copy_image(base, cow_base) + disk.extend(cow_base, size) libvirt_utils.create_cow_image(cow_base, target) elif not generating: libvirt_utils.copy_image(base, target) @@ -1074,7 +1076,8 @@ class LibvirtConnection(driver.ComputeDriver): if size: disk.extend(target, size) - copy_and_extend(cow, generating, base, target, size) + with utils.remove_path_on_error(target): + copy_and_extend(cow, generating, base, target, size) @staticmethod def _create_local(target, local_size, unit='G', @@ -1234,8 +1237,9 @@ class LibvirtConnection(driver.ComputeDriver): project_id=instance['project_id'],) elif config_drive: label = 'config' - self._create_local(basepath('disk.config'), 64, unit='M', - fs_format='msdos', label=label) # 64MB + with utils.remove_path_on_error(basepath('disk.config')): + self._create_local(basepath('disk.config'), 64, unit='M', + fs_format='msdos', label=label) # 64MB if instance['key_data']: key = str(instance['key_data']) -- cgit