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

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(a)redhat.com&gt; --- 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..8a3c7eb 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; + } + 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; + } + + /* Exploit that VIR_DOMAIN_BLOCK_REBASE_* and + * VIR_DOMAIN_BLOCK_COPY_* flags have the same values. */ 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 (vshCommandOptStringReq(ctl, cmd, "dest", &dest) < 0) - goto cleanup; - if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0) - goto cleanup; + if (granularity || buf_size || (format && STRNEQ(format, "raw")) || xml) { + /* New API */ + if (bandwidth || granularity || buf_size) { + params = vshCalloc(ctl, 3, sizeof(*params)); + /* bandwidth is ulong, but the typed parameter is ullong; + * use addition to ensure correct vararg typing */ + if (bandwidth && + virTypedParameterAssign(&params[nparams++], + VIR_DOMAIN_BLOCK_COPY_BANDWIDTH, + VIR_TYPED_PARAM_ULLONG, + bandwidth + 0ULL) < 0) + goto cleanup; + if (granularity && + virTypedParameterAssign(&params[nparams++], + VIR_DOMAIN_BLOCK_COPY_GRANULARITY, + VIR_TYPED_PARAM_UINT, + granularity) < 0) + goto cleanup; + if (buf_size && + virTypedParameterAssign(&params[nparams++], + VIR_DOMAIN_BLOCK_COPY_BUF_SIZE, + VIR_TYPED_PARAM_UINT, + buf_size) < 0) + goto cleanup; + } + + if (!xmlstr) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + virBufferAddLit(&buf, "<disk type='file'>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<source file='%s'/>\n", dest); + virBufferEscapeString(&buf, "<driver type='%s'/>\n", format); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</disk>\n"); + if (virBufferCheckError(&buf) < 0) + goto cleanup; + xmlstr = virBufferContentAndReset(&buf); + } + + if (virDomainBlockCopy(dom, path, xmlstr, params, nparams, flags) < 0) + goto cleanup; + } else { + /* Old API */ + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY; + if (STREQ_NULLABLE(format, "raw")) + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW; + + if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0) + goto cleanup; + } if (!blocking) { vshPrint(ctl, "%s", _("Block Copy started")); @@ -1983,6 +2081,8 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + VIR_FREE(xmlstr); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); if (blocking) diff --git a/tools/virsh.pod b/tools/virsh.pod index e0dfd13..32c5ff9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -873,27 +873,29 @@ value is interpreted as an unsigned long long value or essentially unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed. -=item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>] -[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]] +=item B<blockcopy> I<domain> I<path> { I<dest> [I<format>] | I<xml> } +[I<--shallow>] [I<--reuse-external>] [I<--wait> [I<--async>] [I<--verbose>]] [{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>] +[I<bandwidth>] [I<granularity>] [I<buf-size>] -Copy a disk backing image chain to I<dest>. By default, this command -flattens the entire chain; but if I<--shallow> is specified, the copy -shares the backing chain. +Copy a disk backing image chain to a destination. Either I<dest> as +the destination file name, or I<xml> as the name of an XML file containing +a top-level <disk> element describing the destination, must be present. +Additionally, if I<dest> is given, I<format> should be specified to declare +the format of the destination (if I<format> is omitted, then libvirt +will reuse the format of the source, or with I<--reuse-external> will +be forced to probe the destination format, which could be a potential +security hole). The command supports I<--raw> as a boolean flag synonym for +I<--format=raw>. By default, this command flattens the entire chain; but +if I<--shallow> is specified, the copy shares the backing chain. -If I<--reuse-external> is specified, then I<dest> must exist and have +If I<--reuse-external> is specified, then the destination must exist and have sufficient space to hold the copy. If I<--shallow> is used in conjunction with I<--reuse-external> then the pre-created image must have guest visible contents identical to guest visible contents of the backing file of the original image. This may be used to modify the backing file names on the destination. -The format of the destination is determined by the first match in the -following list: if I<--raw> is specified, it will be raw; if -I<--reuse-external> is specified, the existing destination is probed -for a format; and in all other cases, the destination format will -match the source format. - By default, the copy job runs in the background, and consists of two phases. Initially, the job must copy all data from the source, and during this phase, the job can only be canceled to revert back to the @@ -917,7 +919,14 @@ I<path> specifies fully-qualified path of the disk. I<bandwidth> specifies copying bandwidth limit in MiB/s. Specifying a negative value is interpreted as an unsigned long long value or essentially unlimited. The hypervisor can choose whether to reject the value or -convert it to the maximum value allowed. +convert it to the maximum value allowed. Specifying I<granularity> +allows fine-tuning of the granularity that will be copied when a dirty +page is detected; larger values trigger less I/O overhead but may end +up copying more data overall (the default value is usually correct); +this value must be a power of two. Specifying I<buf-size> will control +how much data can be simultaneously in-flight during the copy; larger +values use more memory but may allow faster completion (the default +value is usually correct). =item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<base>] [I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] -- 1.9.3