Allow proper cleanup of partially created disks
authorMichele Tartara <mtartara@google.com>
Tue, 16 Apr 2013 12:56:39 +0000 (12:56 +0000)
committerMichele Tartara <mtartara@google.com>
Mon, 22 Apr 2013 07:20:41 +0000 (09:20 +0200)
During the creation of an instance, if the creation of disks fails, some
partially created disks might remain lying around. There is an already
implemented cleanup procedure, but it requires the disks to be listed in the
disks_created variable, which is updated after the creation of the disk itself
to prevent it from removing disks already existing previously.

This patch introduces a better tracking of partially created disks, so that
they can be removed without risking removing already existing ones.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

lib/cmdlib.py
lib/errors.py

index 5ad1338..228abd5 100644 (file)
@@ -9340,20 +9340,34 @@ def _CreateBlockDevInner(lu, node, instance, device, force_create,
   @type excl_stor: boolean
   @param excl_stor: Whether exclusive_storage is active for the node
 
+  @return: list of created devices
   """
-  if device.CreateOnSecondary():
-    force_create = True
+  created_devices = []
+  try:
+    if device.CreateOnSecondary():
+      force_create = True
 
-  if device.children:
-    for child in device.children:
-      _CreateBlockDevInner(lu, node, instance, child, force_create,
-                           info, force_open, excl_stor)
+    if device.children:
+      for child in device.children:
+        devs = _CreateBlockDevInner(lu, node, instance, child, force_create,
+                                    info, force_open, excl_stor)
+        created_devices.extend(devs)
 
-  if not force_create:
-    return
+    if not force_create:
+      return created_devices
+
+    _CreateSingleBlockDev(lu, node, instance, device, info, force_open,
+                          excl_stor)
+    # The device has been completely created, so there is no point in keeping
+    # its subdevices in the list. We just add the device itself instead.
+    created_devices = [(node, device)]
+    return created_devices
 
-  _CreateSingleBlockDev(lu, node, instance, device, info, force_open,
-                        excl_stor)
+  except errors.DeviceCreationError, e:
+    e.created_devices.extend(created_devices)
+    raise e
+  except errors.OpExecError, e:
+    raise errors.DeviceCreationError(str(e), created_devices)
 
 
 def _CreateSingleBlockDev(lu, node, instance, device, info, force_open,
@@ -9716,13 +9730,17 @@ def _CreateDisks(lu, instance, to_skip=None, target_node=None):
       except errors.OpExecError:
         logging.warning("Creating disk %s for instance '%s' failed",
                         idx, instance.name)
+      except errors.DeviceCreationError, e:
+        logging.warning("Creating disk %s for instance '%s' failed",
+                        idx, instance.name)
+        disks_created.extend(e.created_devices)
         for (node, disk) in disks_created:
           lu.cfg.SetDiskID(disk, node)
           result = lu.rpc.call_blockdev_remove(node, disk)
           if result.fail_msg:
             logging.warning("Failed to remove newly-created disk %s on node %s:"
                             " %s", device, node, result.fail_msg)
-        raise
+        raise errors.OpExecError(e.message)
 
 
 def _RemoveDisks(lu, instance, target_node=None, ignore_failures=False):
index 3a264c3..22b7502 100644 (file)
@@ -209,6 +209,22 @@ class OpResultError(GenericError):
   """
 
 
+class DeviceCreationError(GenericError):
+  """Error during the creation of a device.
+
+  This exception should contain the list of the devices actually created
+  up to now, in the form of pairs (node, device)
+
+  """
+  def __init__(self, message, created_devices):
+    GenericError.__init__(self)
+    self.message = message
+    self.created_devices = created_devices
+
+  def __str__(self):
+    return self.message
+
+
 class OpCodeUnknown(GenericError):
   """Unknown opcode submitted.