Currently, the way to signal export failures is by the return value.
This means that if a client doesn't check the values (e.g. burnin), any
failure is being ignore. And this is what we've been doing forever in
burning (not actually testing that the export is successful).
This patch changes the behaviour of ExportInstance: it will abort with
an exception for any error, and removes the custom handling from
gnt-backup. This makes the behaviour consistent for any client (e.g.
RAPI), and it prevents false positives. If, for a given instance, a
subset of disks should not be backed up, the OS scripts should handle
that case.
---
lib/cmdlib.py | 18 ++++++++++++++----
scripts/gnt-backup | 22 ++--------------------
2 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index ae419c3..b32b85e 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -9506,12 +9506,22 @@ class LUExportInstance(LogicalUnit):
feedback_fn("Deactivating disks for %s" % instance.name)
_ShutdownInstanceDisks(self, instance)
+ if not (compat.all(dresults) and fin_resu):
+ failures = []
+ if not fin_resu:
+ failures.append("export finalization")
+ if not compat.all(dresults):
+ fdsk = utils.CommaJoin(idx for (idx, dsk) in enumerate(dresults)
+ if not dsk)
+ failures.append("disk export: disk(s) %s" % fdsk)
+
+ raise errors.OpExecError("Export failed, errors in %s" %
+ utils.CommaJoin(failures))
+
+ # At this point, the export was successful, we can cleanup/finish
+
# Remove instance if requested
if self.op.remove_instance:
- if not (compat.all(dresults) and fin_resu):
- feedback_fn("Not removing instance %s as parts of the export failed" %
- instance.name)
- else:
feedback_fn("Removing instance %s" % instance.name)
_RemoveInstance(self, feedback_fn, instance,
self.op.ignore_remove_failures)
diff --git a/scripts/gnt-backup b/scripts/gnt-backup
index b7e49d1..62ed706 100755
--- a/scripts/gnt-backup
+++ b/scripts/gnt-backup
@@ -80,26 +80,8 @@ def ExportInstance(opts, args):
remove_instance=opts.remove_instance,
ignore_remove_failures=ignore_remove_failures)
- fin_resu, dlist = SubmitOpCode(op, opts=opts)
- if not isinstance(dlist, list):
- ToStderr("Cannot parse execution results")
- return 1
- # TODO: handle diskless instances
- if dlist.count(False) == 0:
- # all OK
- rcode = 0
- elif dlist.count(True) == 0:
- ToStderr("Error: No disks were backed up successfully."
- " The export doesn't have any valid data,"
- " it is recommended to retry the operation.")
- rcode = 1
- else:
- ToStderr("Partial export failure: %d disks backed up, %d disks failed.",
- dlist.count(True), dlist.count(False))
- rcode = 2
- if not fin_resu:
- rcode = 1
- return rcode
+ SubmitOpCode(op, opts=opts)
+ return 0
def ImportInstance(opts, args):
--
1.7.1
