[libvirt] [PATCH v3 1/5] virsh: improve blockcopy UI

[Eric Blake]

Peter's review of my addition of active block commit pointed out
some issues that I had copied from block copy.  It makes sense to
allow the shorter command-line of 'blockcopy $dom $disk --pivot'
without having to explicitly specify --wait.  And my use of
embedded ?: ternaries bordered on unreadable.

* tools/virsh-domain.c (cmdBlockCopy): Make --pivot and --finish
imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 tools/virsh-domain.c | 25 ++++++++++++++-----------
 tools/virsh.pod      | 17 +++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9162db..294b594 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1786,15 +1786,15 @@ static const vshCmdOptDef opts_block_copy[] = {
     },
     {.name = "timeout",
      .type = VSH_OT_INT,
-     .help = N_("with --wait, abort if copy exceeds timeout (in seconds)")
+     .help = N_("implies --wait, abort if copy exceeds timeout (in seconds)")
     },
     {.name = "pivot",
      .type = VSH_OT_BOOL,
-     .help = N_("with --wait, pivot when mirroring starts")
+     .help = N_("implies --wait, pivot when mirroring starts")
     },
     {.name = "finish",
      .type = VSH_OT_BOOL,
-     .help = N_("with --wait, quit when mirroring starts")
+     .help = N_("implies --wait, quit when mirroring starts")
     },
     {.name = "async",
      .type = VSH_OT_BOOL,
@@ -1808,10 +1808,10 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom = NULL;
     bool ret = false;
-    bool blocking = vshCommandOptBool(cmd, "wait");
     bool verbose = vshCommandOptBool(cmd, "verbose");
     bool pivot = vshCommandOptBool(cmd, "pivot");
     bool finish = vshCommandOptBool(cmd, "finish");
+    bool blocking = vshCommandOptBool(cmd, "wait");
     int timeout = 0;
     struct sigaction sig_action;
     struct sigaction old_sig_action;
@@ -1822,6 +1822,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
     bool quit = false;
     int abort_flags = 0;

+    blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish;
     if (blocking) {
         if (pivot && finish) {
             vshError(ctl, "%s", _("cannot mix --pivot and --finish"));
@@ -1844,8 +1845,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
         sigaction(SIGINT, &sig_action, &old_sig_action);

         GETTIMEOFDAY(&start);
-    } else if (verbose || vshCommandOptBool(cmd, "timeout") ||
-               vshCommandOptBool(cmd, "async") || pivot || finish) {
+    } else if (verbose || vshCommandOptBool(cmd, "async")) {
         vshError(ctl, "%s", _("missing --wait option"));
         return false;
     }
@@ -1911,11 +1911,14 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
         vshError(ctl, _("failed to finish job for disk %s"), path);
         goto cleanup;
     }
-    vshPrint(ctl, "\n%s",
-             quit ? _("Copy aborted") :
-             pivot ? _("Successfully pivoted") :
-             finish ? _("Successfully copied") :
-             _("Now in mirroring phase"));
+    if (quit)
+        vshPrint(ctl, "\n%s", _("Copy aborted"));
+    else if (pivot)
+        vshPrint(ctl, "\n%s", _("Successfully pivoted"));
+    else if (finish)
+        vshPrint(ctl, "\n%s", _("Successfully copied"));
+    else
+        vshPrint(ctl, "\n%s", _("Now in mirroring phase"));

     ret = true;
  cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 84a60a7..77013f5 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -804,8 +804,8 @@ I<bandwidth> specifies copying bandwidth limit in MiB/s, although for
 qemu, it may be non-zero only for an online domain.

 =item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>]
-[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose>]
-[{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>] [I<--async>]]
+[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]]
+[{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>]

 Copy a disk backing image chain to I<dest>. By default, this command
 flattens the entire chain; but if I<--shallow> is specified, the copy
@@ -834,12 +834,13 @@ faithful copy of that point in time.  However, if I<--wait> is specified,
 then this command will block until the mirroring phase begins, or cancel
 the operation if the optional I<timeout> in seconds elapses or SIGINT is
 sent (usually with C<Ctrl-C>).  Using I<--verbose> along with I<--wait>
-will produce periodic status updates.  Using I<--pivot> or I<--finish>
-along with I<--wait> will additionally end the job cleanly rather than
-leaving things in the mirroring phase.  If job cancellation is triggered,
-I<--async> will return control to the user as fast as possible, otherwise
-the command may continue to block a little while longer until the job
-is done cleaning up.
+will produce periodic status updates.  Using I<--pivot> (similar to
+B<blockjob> I<--pivot>) or I<--finish> (similar to B<blockjob> I<--abort>)
+implies I<--wait>, and will additionally end the job cleanly rather than
+leaving things in the mirroring phase.  If job cancellation is triggered
+by timeout or by I<--finish>, I<--async> will return control to the user
+as fast as possible, otherwise the command may continue to block a little
+while longer until the job has actually cancelled.

 I<path> specifies fully-qualified path of the disk.
 I<bandwidth> specifies copying bandwidth limit in MiB/s.
-- 
1.9.3