[libvirt] [PATCHv4 06/18] blockjob: add 'blockcopy' to virsh

[Jiri Denemark]

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