Re: [PATCH v2 11/12] virsh: Create a macro to add IOTUNE values

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

 




On 10/25/2016 01:44 PM, Erik Skultety wrote:
> On Thu, Oct 06, 2016 at 06:38:59PM -0400, John Ferlan wrote:
>> Rework the repetitive lines to add iotune values into easier to read macro
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>>  tools/virsh-domain.c | 141 +++++++++------------------------------------------
>>  1 file changed, 25 insertions(+), 116 deletions(-)
>>
> 
> [...]
> 
>> +#define VSH_ADD_IOTUNE(PARAM, CONST)                                           \
>> +    if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value,                 \
>> +                                     1, ULLONG_MAX)) < 0) {                    \
> 
> Again a nitpick, I guess the -length setting should not get scaled when one

Upon closer inspect, good catch and it's not just -length... The iops
values aren't scaled either!

There's now two macros... I renamed the current one to add a _SCALED and
created a new one that uses vshCommandOptULongLong (I don't want to mess
with another SCALE argument)


> tries e.g. 'kib' to pass along with the setting. In terms of a cleanup, yes,
> it's very welcome, so maybe add a 'SCALE' argument - your call.

and the -length values will all be part of the non scaled version..

Thanks for the review - the series is now pushed.

John
> 
> ACK
> 
> Erik
> 
>> +        goto interror;                                                         \
>> +    } else if (rv > 0) {                                                       \
>> +        if (virTypedParamsAddULLong(&params, &nparams, &maxparams,             \
>> +                                    VIR_DOMAIN_BLOCK_IOTUNE_##CONST,           \
>> +                                    value) < 0)                                \
>> +            goto save_error;                                                   \
>> +    }                                                                          \
>> +
>> +    VSH_ADD_IOTUNE(total-bytes-sec, TOTAL_BYTES_SEC);
>> +    VSH_ADD_IOTUNE(read-bytes-sec, READ_BYTES_SEC);
>> +    VSH_ADD_IOTUNE(write-bytes-sec, WRITE_BYTES_SEC);
>> +    VSH_ADD_IOTUNE(total-iops-sec, TOTAL_IOPS_SEC);
>> +    VSH_ADD_IOTUNE(read-iops-sec, READ_IOPS_SEC);
>> +    VSH_ADD_IOTUNE(write-iops-sec, WRITE_IOPS_SEC);
>> +
>> +    VSH_ADD_IOTUNE(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
>> +    VSH_ADD_IOTUNE(read-bytes-sec-max, READ_BYTES_SEC_MAX);
>> +    VSH_ADD_IOTUNE(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
>> +    VSH_ADD_IOTUNE(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
>> +    VSH_ADD_IOTUNE(read-iops-sec-max, READ_IOPS_SEC_MAX);
>> +    VSH_ADD_IOTUNE(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
>> +    VSH_ADD_IOTUNE(size-iops-sec, SIZE_IOPS_SEC);
>>  
>>      if (nparams == 0) {
>>          if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) {
>> -- 
>> 2.7.4
>>
>> --
>> 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]