[libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults

[John Ferlan]

On 10/25/2016 09:05 AM, Erik Skultety wrote:
> On Thu, Oct 06, 2016 at 06:38:52PM -0400, John Ferlan wrote:
>> Create a helper to set the bytes/iops iotune default values based on
>> the current qemu setting
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++--------------------
>>  1 file changed, 40 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 78e917e..fcce3f0 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -17286,6 +17286,43 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
>>      return ret;
>>  }
>>  
>> +
>> +/* If the user didn't specify bytes limits, inherit previous values;
>> + * likewise if the user didn't specify iops limits.  */
>> +static void
>> +qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
>> +                                    virDomainBlockIoTuneInfoPtr oldinfo,
>> +                                    bool set_bytes,
>> +                                    bool set_iops,
>> +                                    bool set_bytes_max,
>> +                                    bool set_iops_max,
>> +                                    bool set_size_iops)
>> +{
> 
> Could this be a macro instead? 5 booleans, it just seems a tiny bit odd.
> 

Probably - I can try...  Obvious impact later when _length is added.

>> +    if (!set_bytes) {
>> +        newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
>> +        newinfo->read_bytes_sec = oldinfo->read_bytes_sec;
>> +        newinfo->write_bytes_sec = oldinfo->write_bytes_sec;
>> +    }
> 
> And then you'll end up just with snippet like ^^this. Or you could use
> bitwise OR'd flags instead of the booleans, as it scales better (in terms of
> number of arguments), though I'm not very fond of that even though it works.
> 
> ACK with the adjustment, but see 5/12 as well before pushing.

There is no official bug report - it was one of those visual inspection
and oh sh*t moments during testing when I didn't release my domain
wasn't started and my max settings were zero'd out.

I can merge the two together as well - not a problem.

John