From 6a3eabcd01981c6ccead47e2b610bd82b5d6be80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady@redhat.com>
Date: Fri, 16 Mar 2012 03:43:49 +0000
Subject: [PATCH] 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(-)
diff --git a/nova/utils.py b/nova/utils.py
index 819929a..7188d98 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
@@ -1013,8 +1014,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
@@ -1033,8 +1034,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
@@ -1344,6 +1345,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 1e0ae0a..626f3ff 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 31e6511..dc16d05 100644
--- a/nova/virt/libvirt/connection.py
+++ b/nova/virt/libvirt/connection.py
@@ -1105,7 +1105,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)
@@ -1121,8 +1122,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)
@@ -1132,7 +1134,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',
@@ -1305,8 +1308,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 FLAGS.libvirt_inject_key and instance['key_data']:
key = str(instance['key_data'])