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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 05/08/2015 07:25 AM, John Ferlan wrote:
> 
> 
> 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

Ug - didn't read and consider 7/8 and 8/8 closely enough <sigh>, so let
me retract the if you pass and just ACK

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]