Re: [libvirt] [PATCH v3 11/18] blockcopy: split out virsh implementation

On 08/31/14 06:02, Eric Blake wrote: > I'm about to extend the capabilities of blockcopy. Hiding a few > common lines of implementation gets in the way of the new required > logic, and putting the new logic in the common implementation won't > benefit any of the other blockjob operations. Therefore, it is > simpler to just do the work inline. There should be no semantic > change in this patch. > > * tools/virsh-domain.c (blockJobImpl): Drop unused variable. Move > block copy guts... > (cmdBlockCopy): ...into their lone caller. > > Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > --- > tools/virsh-domain.c | 45 ++++++++++++++++++++++++++------------------- > 1 file changed, 26 insertions(+), 19 deletions(-) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 813d8e0..b92b2eb 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -1907,6 +1894,9 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) > bool quit = false; > int abort_flags = 0; > > + if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) > + return false; > + > blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish; > if (blocking) { > if (pivot && finish) { ... > @@ -1935,7 +1923,26 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) > return false; > } > > - if (!blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_COPY, &dom)) > + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) > + goto cleanup; > + > + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { > + vshError(ctl, "%s", _("bandwidth must be a number")); > + goto cleanup; > + } > + > + 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 (vshCommandOptBool(cmd, "blockdev")) > + flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV; > + if (vshCommandOptStringReq(ctl, cmd, "dest", &dest) < 0) > + goto cleanup; > + You could extract the flags and string at the top where you extract @path to be consistent, but that's more bikeshedding than useful.

> + if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0) > goto cleanup; > > if (!blocking) { > ACK as-is. Peter -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list