[libvirt] [PATCH v2 3/8] blockcopy: expose new API in virsh

[Peter Krempa]

On 08/26/14 13:21, Eric Blake wrote:
> Expose the new power of virDomainBlockCopy through virsh.  Continue
> to use the older API where possible, for maximum compatibility.
> 
> The command now requires either --dest (with optional --format),
> to directly describe the file destination, or --xml, to name a
> file that contains an XML description such as:
> 
> <disk type='network'>
>   <driver type='raw'/>
>   <source protocol='gluster' name='vol1/img'>
>     <host name='red'/>
>   </source>
> </disk>
> 
> Non-zero option parameters are converted into virTypedParameters,
> and if anything requires the new API, the command can synthesize
> appropriate XML even if the --dest option was used instead of --xml.
> 
> The existing --raw flag remains for back-compat, but the preferred
> spelling is now --format=raw, since the new API now allows us
> to specify all formats rather than just a boolean raw to suppress
> probing.
> 
> I hope I did justice in describing the effects of granularity and
> buf-size on how they get passed through to qemu.
> 
> * tools/virsh-domain.c (cmdBlockCopy): Add new options --xml,
> --granularity, --buf-size, --format. Make --raw an alias for
> --format=raw. Call new API if new parameters are in use.
> * tools/virsh.pod (blockcopy): Document new options.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  tools/virsh-domain.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  tools/virsh.pod      |  35 +++++++++------
>  2 files changed, 134 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index fb9c009..a42858a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1793,11 +1793,10 @@ static const vshCmdOptDef opts_block_copy[] = {
>      {.name = "path",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> -     .help = N_("fully-qualified path of disk")
> +     .help = N_("fully-qualified path of source disk")
>      },
>      {.name = "dest",
>       .type = VSH_OT_DATA,
> -     .flags = VSH_OFLAG_REQ,
>       .help = N_("path of the copy to create")
>      },
>      {.name = "bandwidth",
> @@ -1813,8 +1812,8 @@ static const vshCmdOptDef opts_block_copy[] = {
>       .help = N_("reuse existing destination")
>      },
>      {.name = "raw",
> -     .type = VSH_OT_BOOL,
> -     .help = N_("use raw destination file")
> +     .type = VSH_OT_ALIAS,
> +     .help = "format=raw"
>      },
>      {.name = "wait",
>       .type = VSH_OT_BOOL,
> @@ -1840,6 +1839,22 @@ static const vshCmdOptDef opts_block_copy[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("with --wait, don't wait for cancel to finish")
>      },
> +    {.name = "xml",
> +     .type = VSH_OT_DATA,
> +     .help = N_("filename containing XML description of the copy destination")
> +    },
> +    {.name = "format",
> +     .type = VSH_OT_DATA,
> +     .help = N_("format of the destination file")
> +    },
> +    {.name = "granularity",
> +     .type = VSH_OT_INT,
> +     .help = N_("power-of-two granularity to use during the copy")
> +    },
> +    {.name = "buf-size",
> +     .type = VSH_OT_INT,
> +     .help = N_("maximum amount of in-flight data during the copy")
> +    },
>      {.name = NULL}
>  };
> 
> @@ -1847,9 +1862,12 @@ static bool
>  cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
>  {
>      virDomainPtr dom = NULL;
> -    const char *dest;
> +    const char *dest = NULL;
> +    const char *format = NULL;
>      unsigned long bandwidth = 0;
> -    unsigned int flags = VIR_DOMAIN_BLOCK_REBASE_COPY;
> +    unsigned int granularity = 0;
> +    unsigned int buf_size = 0;
> +    unsigned int flags = 0;
>      bool ret = false;
>      bool blocking = vshCommandOptBool(cmd, "wait");
>      bool verbose = vshCommandOptBool(cmd, "verbose");
> @@ -1864,6 +1882,22 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
>      const char *path = NULL;
>      bool quit = false;
>      int abort_flags = 0;
> +    const char *xml = NULL;
> +    char *xmlstr = NULL;
> +    virTypedParameterPtr params = NULL;
> +    int nparams = 0;
> +
> +    if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
> +        return false;
> +    if (vshCommandOptString(cmd, "dest", &dest) < 0)
> +        goto cleanup;
> +    if (vshCommandOptString(cmd, "xml", &xml) < 0)
> +        return false;
> +    if (vshCommandOptString(cmd, "format", &format) < 0)
> +        return false;
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml);
> +    VSH_EXCLUSIVE_OPTIONS_VAR(format, xml);
> 
>      if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
>          return false;
> @@ -1901,18 +1935,82 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
>          vshError(ctl, "%s", _("bandwidth must be a number"));
>          goto cleanup;
>      }
> +    if (vshCommandOptUIntWrap(cmd, "granularity", &granularity) < 0) {
> +        vshError(ctl, "%s", _("granularity must be a number"));
> +        goto cleanup;
> +    }

I don't think wrapping makes sense here as you've documented that it has
to be a power of 2.

> +    if (vshCommandOptUIntWrap(cmd, "buf-size", &buf_size) < 0) {
> +        vshError(ctl, "%s", _("buf-size must be a number"));
> +        goto cleanup;
> +    }
> 
> +    if (xml) {
> +        if (virFileReadAll(xml, 8192, &xmlstr) < 0) {
> +            vshReportError(ctl);
> +            goto cleanup;
> +        }
> +    } else if (!dest) {
> +        vshError(ctl, "%s", _("need either --dest or --xml"));
> +        goto cleanup;
> +    }

ACK, the wrapped granularity number should be rejected by the daemon.

Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140826/2ab5e3ba/attachment-0001.sig>