From edf46d78c32b6cf6dc5a8d359603ac035b889610 Mon Sep 17 00:00:00 2001
Message-Id: <edf46d78c32b6cf6dc5a8d359603ac035b889610@dist-git>
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 21 Jul 2015 16:18:30 +0200
Subject: [PATCH] virsh: Refactor argument handling in cmdBlockCopy
https://bugzilla.redhat.com/show_bug.cgi?id=1227551
https://bugzilla.redhat.com/show_bug.cgi?id=1197592
Put all argument parsing together and refactor the argument checking
code.
(cherry picked from commit 8e85f62826a7df46f16c4c9e9abca5ede27b5603)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
tools/virsh-domain.c | 133 ++++++++++++++++++++++++++++-----------------------
1 file changed, 72 insertions(+), 61 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4560d32..c233eb7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2088,11 +2088,12 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
unsigned long long buf_size = 0;
unsigned int flags = 0;
bool ret = false;
- bool blocking = vshCommandOptBool(cmd, "wait");
bool verbose = vshCommandOptBool(cmd, "verbose");
bool pivot = vshCommandOptBool(cmd, "pivot");
bool finish = vshCommandOptBool(cmd, "finish");
bool blockdev = vshCommandOptBool(cmd, "blockdev");
+ bool blocking = vshCommandOptBool(cmd, "wait") || finish || pivot;
+ bool async = vshCommandOptBool(cmd, "async");
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -2117,78 +2118,88 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
return false;
if (vshCommandOptString(ctl, cmd, "format", &format) < 0)
return false;
-
- VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml);
- VSH_EXCLUSIVE_OPTIONS_VAR(format, xml);
- VSH_EXCLUSIVE_OPTIONS_VAR(blockdev, xml);
-
- blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish;
- if (blocking) {
- if (pivot && finish) {
- vshError(ctl, "%s", _("cannot mix --pivot and --finish"));
- return false;
- }
- if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
- return false;
- if (vshCommandOptBool(cmd, "async"))
- abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
-
- sigemptyset(&sigmask);
- sigaddset(&sigmask, SIGINT);
-
- intCaught = 0;
- sig_action.sa_sigaction = vshCatchInt;
- sig_action.sa_flags = SA_SIGINFO;
- sigemptyset(&sig_action.sa_mask);
- sigaction(SIGINT, &sig_action, &old_sig_action);
-
- GETTIMEOFDAY(&start);
- } else if (verbose || vshCommandOptBool(cmd, "async")) {
- vshError(ctl, "%s", _("missing --wait option"));
- return false;
- }
-
- virConnectDomainEventGenericCallback cb =
- VIR_DOMAIN_EVENT_CALLBACK(vshBlockJobStatusHandler);
-
- if ((cb_id = virConnectDomainEventRegisterAny(ctl->conn,
- dom,
- VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
- cb,
- &status,
- NULL)) < 0)
- vshResetLibvirtError();
-
- if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
-
/* XXX: Parse bandwidth as scaled input, rather than forcing
* MiB/s, and either reject negative input or treat it as 0 rather
* than trying to guess which value will work well across both
* APIs with their different sizes and scales. */
if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0)
- goto cleanup;
+ return false;
if (vshCommandOptUInt(ctl, cmd, "granularity", &granularity) < 0)
- goto cleanup;
+ return false;
if (vshCommandOptULongLong(ctl, cmd, "buf-size", &buf_size) < 0)
- goto cleanup;
-
- if (xml) {
- if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlstr) < 0) {
- vshReportError(ctl);
- goto cleanup;
- }
- } else if (!dest) {
- vshError(ctl, "%s", _("need either --dest or --xml"));
- goto cleanup;
- }
-
+ return false;
/* Exploit that some VIR_DOMAIN_BLOCK_REBASE_* and
* VIR_DOMAIN_BLOCK_COPY_* flags have the same values. */
if (vshCommandOptBool(cmd, "shallow"))
flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW;
if (vshCommandOptBool(cmd, "reuse-external"))
flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
+ if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
+ return false;
+
+ if (timeout)
+ blocking = true;
+
+ if (async)
+ abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml);
+ VSH_EXCLUSIVE_OPTIONS_VAR(format, xml);
+ VSH_EXCLUSIVE_OPTIONS_VAR(blockdev, xml);
+ VSH_EXCLUSIVE_OPTIONS_VAR(pivot, finish);
+
+ if (!dest && !xml) {
+ vshError(ctl, "%s", _("need either --dest or --xml"));
+ return false;
+ }
+
+ if (!blocking) {
+ if (verbose) {
+ vshError(ctl, "%s", _("--verbose requires at least one of --timeout, "
+ "--wait, --pivot, or --finish"));
+ return false;
+ }
+
+ if (async) {
+ vshError(ctl, "%s", _("--async requires at least one of --timeout, "
+ "--wait, --pivot, or --finish"));
+ return false;
+ }
+ }
+
+ if (blocking) {
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGINT);
+
+ intCaught = 0;
+ sig_action.sa_sigaction = vshCatchInt;
+ sig_action.sa_flags = SA_SIGINFO;
+ sigemptyset(&sig_action.sa_mask);
+ sigaction(SIGINT, &sig_action, &old_sig_action);
+
+ GETTIMEOFDAY(&start);
+ }
+
+ virConnectDomainEventGenericCallback cb =
+ VIR_DOMAIN_EVENT_CALLBACK(vshBlockJobStatusHandler);
+
+ if ((cb_id = virConnectDomainEventRegisterAny(ctl->conn,
+ dom,
+ VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
+ cb,
+ &status,
+ NULL)) < 0)
+ vshResetLibvirtError();
+
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ goto cleanup;
+
+ if (xml) {
+ if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlstr) < 0) {
+ vshReportError(ctl);
+ goto cleanup;
+ }
+ }
if (granularity || buf_size || (format && STRNEQ(format, "raw")) || xml) {
/* New API */
@@ -2242,7 +2253,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
} else {
/* Old API */
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY;
- if (vshCommandOptBool(cmd, "blockdev"))
+ if (blockdev)
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;
if (STREQ_NULLABLE(format, "raw"))
flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
--
2.5.0