From 17840379910757c0bf04d204b58a3e3a016787d0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 11 Jun 2014 09:36:49 -0600 Subject: [PATCH] virsh: improve blockcopy UI Peter's review of an early version of my addition of active block commit pointed out some issues that I was copying from the block copy code; fix them up now before perpetuating them. For virsh commands that manage a single API call, it's nice to have a 1:1 mapping of options to flags, so that we can test that lower-layer software handles flag combinations correctly. But where virsh is introducing syntactic sugar to combine multiple API calls into a single user interface, we might as well make that interface compact. That is, we should allow the shorter command-line of 'blockcopy $dom $disk --pivot' without having to explicitly specify --wait, because this isn't directly a flag passed to a single underlying API call. Also, my use of embedded ?: ternaries bordered on unreadable. * tools/virsh-domain.c (cmdBlockCopy): Make --pivot, --finish, and --timeout imply --wait. Drop excess ?: operators. * tools/virsh.pod (blockcopy): Update documentation. Signed-off-by: Eric Blake --- tools/virsh-domain.c | 23 +++++++++++++---------- tools/virsh.pod | 17 +++++++++-------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 04fed79a54..6b3dd7001a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1775,15 +1775,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, @@ -1811,6 +1811,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")); @@ -1833,8 +1834,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; } @@ -1900,11 +1900,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 b2fd53bb06..35cf878617 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -802,8 +802,8 @@ I specifies copying bandwidth limit in MiB/s, although for qemu, it may be non-zero only for an online domain. =item B I I I [I] [I<--shallow>] -[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose>] -[{I<--pivot> | I<--finish>}] [I<--timeout> B] [I<--async>]] +[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]] +[{I<--pivot> | I<--finish>}] [I<--timeout> B] Copy a disk backing image chain to I. By default, this command flattens the entire chain; but if I<--shallow> is specified, the copy @@ -832,12 +832,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 in seconds elapses or SIGINT is sent (usually with 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 I<--pivot>) or I<--finish> (similar to B 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 specifies fully-qualified path of the disk. I specifies copying bandwidth limit in MiB/s. -- GitLab