[libvirt] [PATCH 2/3] blockjob: add virsh blockcommit
Peter Krempa
pkrempa at redhat.com
Mon Sep 17 23:29:55 UTC 2012
On 09/18/12 00:08, Eric Blake wrote:
> The wait loop logic borrows heavily from blockcommit.
Maybe a little too sparse on the commit message, but the man page
addition makes it up.
>
> * tools/virsh-domain.c (cmdBlockCommit): New function.
> * tools/virsh.pod (blockcommit): Document it.
> ---
> tools/virsh-domain.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> tools/virsh.pod | 30 ++++++++++
> 2 files changed, 182 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index b6de9a8..aa23011 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1260,6 +1272,144 @@ static void vshCatchInt(int sig ATTRIBUTE_UNUSED,
> }
>
> /*
> + * "blockcommit" command
> + */
> +static const vshCmdInfo info_block_commit[] = {
> + {"help", N_("Start a block commit operation.")},
> + {"desc", N_("Commit changes from a snapshot down to its backing image.")},
> + {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_block_commit[] = {
> + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> + {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("fully-qualified path of disk")},
The API says you can use "vda" and similar, is that considered
fully-qualified? / this question was answered below /
> + {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("bandwidth limit in MiB/s")},
Megabits look like a reasonable granularity to limit this. Well maybe
apart from testing purposes where somebody would like to do it really
slow to be able to poke around.
> + {"base", VSH_OT_DATA, VSH_OFLAG_NONE,
> + N_("path of base file to commit into (default bottom of chain)")},
> + {"shallow", VSH_OT_BOOL, 0, N_("use backing file of top as base")},
> + {"top", VSH_OT_DATA, VSH_OFLAG_NONE,
> + N_("path of top file to commit from (default top of chain)")},
> + {"delete", VSH_OT_BOOL, 0,
> + N_("delete files that were successfully committed")},
> + {"wait", VSH_OT_BOOL, 0, N_("wait for job to complete")},
> + {"verbose", VSH_OT_BOOL, 0, N_("with --wait, display the progress")},
> + {"timeout", VSH_OT_INT, VSH_OFLAG_NONE,
> + N_("with --wait, abort if copy exceeds timeout (in seconds)")},
> + {NULL, 0, 0, NULL}
> +};
> +
> +static bool
> +cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
> +{
> + virDomainPtr dom = NULL;
> + bool ret = false;
> + bool blocking = vshCommandOptBool(cmd, "wait");
> + bool verbose = vshCommandOptBool(cmd, "verbose");
> + int timeout = 0;
> + struct sigaction sig_action;
> + struct sigaction old_sig_action;
> + sigset_t sigmask;
> + struct timeval start;
> + struct timeval curr;
> + const char *path = NULL;
> + bool quit = false;
> + int abort_flags = 0;
> +
> + if (blocking) {
> + if (vshCommandOptInt(cmd, "timeout", &timeout) > 0) {
> + if (timeout < 1) {
> + vshError(ctl, "%s", _("invalid timeout"));
> + return false;
> + }
> +
> + /* Ensure that we can multiply by 1000 without overflowing. */
> + if (timeout > INT_MAX / 1000) {
> + vshError(ctl, "%s", _("timeout is too big"));
> + return false;
> + }
> + }
> + if (vshCommandOptString(cmd, "path", &path) < 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, "timeout") ||
> + vshCommandOptBool(cmd, "async")) {
> + vshError(ctl, "%s", _("blocking control options require --wait"));
This error message isn't 100% clear on the first read. What about:
"--verbose and --timeout require --wait" ?
> + return false;
> + }
> +
> + if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_COMMIT, &dom) < 0)
> + goto cleanup;
> +
> + if (!blocking) {
> + vshPrint(ctl, "%s", _("Block Commit started"));
> + ret = true;
> + goto cleanup;
> + }
> +
> + while (blocking) {
> + virDomainBlockJobInfo info;
> + int result = virDomainGetBlockJobInfo(dom, path, &info, 0);
> +
> + if (result < 0) {
> + vshError(ctl, _("failed to query job for disk %s"), path);
> + goto cleanup;
> + }
> + if (result == 0)
> + break;
> +
> + if (verbose)
> + print_job_progress(_("Block Commit"),
> + info.end - info.cur, info.end);
> +
> + GETTIMEOFDAY(&curr);
> + if (intCaught || (timeout &&
> + (((int)(curr.tv_sec - start.tv_sec) * 1000 +
> + (int)(curr.tv_usec - start.tv_usec) / 1000) >
> + timeout * 1000))) {
Wouldn't it be better to multiply timeout by 1000 at the beginning and
not bother re-doing it on every iteration?
> + vshDebug(ctl, VSH_ERR_DEBUG,
> + intCaught ? "interrupted" : "timeout");
> + intCaught = 0;
> + timeout = 0;
> + quit = true;
> + if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
> + vshError(ctl, _("failed to abort job for disk %s"), path);
> + goto cleanup;
> + }
> + if (abort_flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)
> + break;
> + } else {
> + usleep(500 * 1000);
> + }
> + }
> +
> + if (verbose && !quit) {
> + /* printf [100 %] */
> + print_job_progress(_("Block Commit"), 0, 1);
> + }
> + vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete"));
Meh a ternary ... but saves lines.
> +
> + ret = true;
> +cleanup:
> + if (dom)
> + virDomainFree(dom);
> + if (blocking)
> + sigaction(SIGINT, &old_sig_action, NULL);
> + return ret;
> +}
> +
> +/*
> * "blockcopy" command
> */
> static const vshCmdInfo info_block_copy[] = {
> @@ -8112,6 +8262,7 @@ const vshCmdDef domManagementCmds[] = {
> {"autostart", cmdAutostart, opts_autostart, info_autostart, 0},
> {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune, 0},
> {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0},
> + {"blockcommit", cmdBlockCommit, opts_block_commit, info_block_commit, 0},
> {"blockcopy", cmdBlockCopy, opts_block_copy, info_block_copy, 0},
> {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
> {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index bb135da..4a79e12 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -694,6 +694,36 @@ currently in use by a running domain. Other contexts that require a MAC
> address of virtual interface (such as I<detach-interface> or
> I<domif-setlink>) will accept the MAC address printed by this command.
>
> +=item B<blockcommit> I<domain> I<path> [I<bandwidth>]
> +{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>]
> +[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>]]
> +
> +Reduce the length of a backing image chain, by committing changes at the
> +top of the chain (snapshot or delta files) into backing images. By
> +default, this command attempts to flatten the entire chain. If I<base>
> +and/or I<top> are specified as files within the backing chain, then the
> +operation is constrained to committing just that portion of the chain;
> +I<--shallow> can be used instead of I<base> to specify the immediate
> +backing file of the resulting top image to be committed. The files
> +being committed are rendered invalid, possibly as soon as the operation
> +starts; using the I<--delete> flag will remove these files at the successful
> +completion of the commit operation.
> +
> +By default, this command returns as soon as possible, and data for
> +the entire disk is committed in the background; the progress of the
> +operation can be checked with B<blockjob>. However, if I<--wait> is
> +specified, then this command will block until the operation completes,
> +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.
> +
> +I<path> specifies fully-qualified path of the disk; it corresponds
> +to a unique target name (<target dev='name'/>) or source file (<source
> +file='name'/>) for one of the disk devices attached to I<domain> (see
Okay, this answers my question from the beginning.
> +also B<domblklist> for listing these names).
> +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>]]
>
Your docs are always great. ACK
Peter
More information about the libvir-list
mailing list