On Mon, Apr 09, 2012 at 21:52:15 -0600, Eric Blake wrote:
> Rather than further overloading 'blockpull', I decided to create a
> new virsh command to expose the new flags of virDomainBlockRebase.
>
> Someday, I'd also like to make blockpull and blockcopy have a
> synchronous mode, which blocks until the event happens or Ctrl-C
> is pressed, as well as a --verbose flag to print status updates
> before the job finishes - but not today.
>
> * tools/virsh.c (VSH_CMD_BLOCK_JOB_COPY): New mode.
> (blockJobImpl): Support new flags.
> (cmdBlockCopy): New command.
> (cmdBlockJob): Support new job info, new abort flag.
> * tools/virsh.pod (blockcopy, blockjob): Document the new command
> and flags.
> ---
> tools/virsh.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-------
> tools/virsh.pod | 36 +++++++++++++++++++++++--
> 2 files changed, 101 insertions(+), 13 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 084d533..25403f5 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7515,16 +7515,18 @@ typedef enum {
> VSH_CMD_BLOCK_JOB_INFO = 1,
> VSH_CMD_BLOCK_JOB_SPEED = 2,
> VSH_CMD_BLOCK_JOB_PULL = 3,
> -} VSH_CMD_BLOCK_JOB_MODE;
> + VSH_CMD_BLOCK_JOB_COPY = 4,
> +} vshCmdBlockJobMode;
>
> static int
> blockJobImpl(vshControl *ctl, const vshCmd *cmd,
> - virDomainBlockJobInfoPtr info, int mode)
> + virDomainBlockJobInfoPtr info, int mode)
> {
> virDomainPtr dom = NULL;
> const char *name, *path;
> unsigned long bandwidth = 0;
> int ret = -1;
> + const char *base = NULL;
> unsigned int flags = 0;
>
> if (!vshConnectionUsability(ctl, ctl->conn))
> @@ -7541,22 +7543,39 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
> goto cleanup;
> }
>
> - if (mode == VSH_CMD_BLOCK_JOB_ABORT) {
> + switch ((vshCmdBlockJobMode) mode) {
> + case VSH_CMD_BLOCK_JOB_ABORT:
> if (vshCommandOptBool(cmd, "async"))
> flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
> + if (vshCommandOptBool(cmd, "pivot"))
> + flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
> ret = virDomainBlockJobAbort(dom, path, flags);
> - } else if (mode == VSH_CMD_BLOCK_JOB_INFO) {
> + break;
> + case VSH_CMD_BLOCK_JOB_INFO:
> ret = virDomainGetBlockJobInfo(dom, path, info, 0);
> - } else if (mode == VSH_CMD_BLOCK_JOB_SPEED) {
> + break;
> + case VSH_CMD_BLOCK_JOB_SPEED:
> ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0);
> - } else if (mode == VSH_CMD_BLOCK_JOB_PULL) {
> - const char *base = NULL;
> + break;
> + case VSH_CMD_BLOCK_JOB_PULL:
> if (vshCommandOptString(cmd, "base", &base) < 0)
> goto cleanup;
> if (base)
> ret = virDomainBlockRebase(dom, path, base, bandwidth, 0);
> else
> ret = virDomainBlockPull(dom, path, bandwidth, 0);
> + break;
> + case VSH_CMD_BLOCK_JOB_COPY:
> + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY;
> + if (vshCommandOptBool(cmd, "shallow"))
> + flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW;
> + if (vshCommandOptBool(cmd, "reuse-external"))
> + flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
> + if (vshCommandOptBool(cmd, "raw"))
> + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
> + if (vshCommandOptString(cmd, "dest", &base) < 0)
> + goto cleanup;
> + ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
> }
>
> cleanup:
> @@ -7566,6 +7585,34 @@ cleanup:
> }
>
> /*
> + * "blockcopy" command
> + */
> +static const vshCmdInfo info_block_copy[] = {
> + {"help", N_("Start a block copy operation.")},
> + {"desc", N_("Populate a disk from its backing image.")},
> + {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_block_copy[] = {
> + {"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")},
> + {"dest", VSH_OT_DATA, VSH_OFLAG_REQ, N_("path of the copy to create")},
> + {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("Bandwidth limit in MB/s")},
> + {"shallow", VSH_OT_BOOL, 0, N_("make the copy share a backing chain")},
> + {"reuse-external", VSH_OT_BOOL, 0, N_("reuse existing destination")},
> + {"raw", VSH_OT_BOOL, 0, N_("use raw destination file")},
> + {NULL, 0, 0, NULL}
> +};
You are pretty inconsistent in upper/lower-case letter at the beginning of
each description string.
> +
> +static bool
> +cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
> +{
> + if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_COPY) != 0)
> + return false;
> + return true;
> +}
> +
> +/*
> * "blockpull" command
> */
> static const vshCmdInfo info_block_pull[] = {
> @@ -7607,6 +7654,8 @@ static const vshCmdOptDef opts_block_job[] = {
> N_("Abort the active job on the specified disk")},
> {"async", VSH_OT_BOOL, VSH_OFLAG_NONE,
> N_("don't wait for --abort to complete")},
> + {"pivot", VSH_OT_BOOL, VSH_OFLAG_NONE,
> + N_("conclude and pivot a copy job")},
> {"info", VSH_OT_BOOL, VSH_OFLAG_NONE,
> N_("Get active job information for the specified disk")},
> {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE,
This has the same kind of inconsistency but you are not making it any worse
:-)
The rest looks good.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list