Re: [libvirt] [PATCH 3/8] virsh: blockjob: Split out vshBlockJobSetSpeed from blockJobImpl

On 04/30/2015 10:45 AM, Peter Krempa wrote: > --- > tools/virsh-domain.c | 33 ++++++++++++++++++++++++--------- > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 720bc68..0d0e39e 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -1667,7 +1667,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) > > typedef enum { > VSH_CMD_BLOCK_JOB_ABORT, > - VSH_CMD_BLOCK_JOB_SPEED, > VSH_CMD_BLOCK_JOB_PULL, > VSH_CMD_BLOCK_JOB_COMMIT, > } vshCmdBlockJobMode; > @@ -1704,10 +1703,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, > if (virDomainBlockJobAbort(dom, path, flags) < 0) > goto cleanup; > break; > - case VSH_CMD_BLOCK_JOB_SPEED: > - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) > - goto cleanup; > - break; > case VSH_CMD_BLOCK_JOB_PULL: > if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) > goto cleanup; > @@ -2543,6 +2538,26 @@ vshBlockJobInfo(vshControl *ctl, > > > static bool > +vshBlockJobSetSpeed(vshControl *ctl, > + const vshCmd *cmd, > + virDomainPtr dom, > + const char *path) > +{ > + unsigned long bandwidth; > + > + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { > + vshError(ctl, "%s", _("bandwidth must be a number")); > + return false; > + } > + > + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) > + return false; > + > + return true; > +} > + > + > +static bool > cmdBlockJob(vshControl *ctl, const vshCmd *cmd) > { > bool ret = false; > @@ -2574,10 +2589,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) > > if (abort || pivot || async) > return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); > - if (bandwidth) > - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); > > - /* Everything below here is for --info mode */ > if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) > goto cleanup; > > @@ -2585,7 +2597,10 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) > if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) > goto cleanup; > > - ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); > + if (bandwidth) > + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path); Since we're getting bandwidth a few lines above - why not just pass that? Especially considering what you do in patch 5 which only allows using a scaled bandwidth for SetSpeed, but not Pull and Commit which patch 5 implies to me in the virsh.pod can be used "in general". If you pass the already parsed bandwidth here - then an ACK; otherwise, I'd have to get more explanation... John

> + else > + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); > > cleanup: > if (dom) > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list