[libvirt] [PATCH v6 6/7] qemu: Add bps_max and friends to qemu command generation

[Matthias Gatto]

On Tue, Nov 11, 2014 at 1:20 PM, John Ferlan <jferlan at redhat.com> wrote:
>
>
> On 10/29/2014 08:16 AM, Matthias Gatto wrote:
>> Check the arability of the options with the current qemu binary,
>> add them in the varable opt if yes, print a message if not.
>>
>> Signed-off-by: Matthias Gatto <matthias.gatto at outscale.com>
>> ---
>>  src/qemu/qemu_command.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 56 insertions(+), 1 deletion(-)
>>
>
> Coverity was a bit unhappy about this change...
>
>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 2e5af4f..b3dc919 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3669,12 +3669,32 @@ qemuBuildDriveStr(virConnectPtr conn,
>>          goto error;
>>      }
>>
>> +    /* block I/O throttling 1.7 */
>> +    if ((disk->blkdeviotune.total_bytes_sec_max ||
>> +         disk->blkdeviotune.read_bytes_sec_max ||
>> +         disk->blkdeviotune.write_bytes_sec_max ||
>> +         disk->blkdeviotune.total_iops_sec_max ||
>> +         disk->blkdeviotune.read_iops_sec_max ||
>> +         disk->blkdeviotune.write_iops_sec_max) &&
>> +        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                       _("there is some block I/O throttling paramater that are not supported with this "
>> +                         "QEMU binary(need QEMU 1.7 or superior)"));
>> +        goto error;
>> +    }
>> +
>>      if (disk->blkdeviotune.total_bytes_sec > LLONG_MAX ||
>>          disk->blkdeviotune.read_bytes_sec > LLONG_MAX ||
>>          disk->blkdeviotune.write_bytes_sec > LLONG_MAX ||
>>          disk->blkdeviotune.total_iops_sec > LLONG_MAX ||
>>          disk->blkdeviotune.read_iops_sec > LLONG_MAX ||
>> -        disk->blkdeviotune.write_iops_sec > LLONG_MAX) {
>> +        disk->blkdeviotune.write_iops_sec > LLONG_MAX ||
>> +        disk->blkdeviotune.total_bytes_sec_max > LLONG_MAX ||
>> +        disk->blkdeviotune.read_bytes_sec_max > LLONG_MAX ||
>> +        disk->blkdeviotune.write_bytes_sec_max > LLONG_MAX ||
>> +        disk->blkdeviotune.total_iops_sec_max > LLONG_MAX ||
>> +        disk->blkdeviotune.read_iops_sec_max > LLONG_MAX ||
>> +        disk->blkdeviotune.write_iops_sec_max > LLONG_MAX) {
>>          virReportError(VIR_ERR_OVERFLOW,
>>                        _("block I/O throttle limit must "
>>                          "be less than %llu using QEMU"), LLONG_MAX);
>> @@ -3711,6 +3731,41 @@ qemuBuildDriveStr(virConnectPtr conn,
>>                            disk->blkdeviotune.write_iops_sec);
>>      }
>>
>> +    if (disk->blkdeviotune.total_bytes_sec_max) {
>> +        virBufferAsprintf(&opt, ",bps_max=%llu",
>> +                          disk->blkdeviotune.total_bytes_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.read_bytes_sec_max) {
>> +        virBufferAsprintf(&opt, ",bps_rd_max=%llu",
>> +                          disk->blkdeviotune.read_bytes_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.write_bytes_sec_max) {
>> +        virBufferAsprintf(&opt, ",bps_wr_max=%llu",
>> +                          disk->blkdeviotune.write_bytes_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.total_iops_sec_max) {
>> +        virBufferAsprintf(&opt, ",iops_max=%llu",
>> +                          disk->blkdeviotune.total_iops_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.read_iops_sec_max) {
>> +        virBufferAsprintf(&opt, ",iops_rd_max=%llu",
>> +                          disk->blkdeviotune.read_iops_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.write_iops_sec_max) {
>> +        virBufferAsprintf(&opt, ",iops_wr_max=%llu",
>> +                          disk->blkdeviotune.write_iops_sec_max);
>> +    }
>> +
>> +    if (disk->blkdeviotune.write_iops_sec_max) {
>> +        virBufferAsprintf(&opt, ",iops_size=%llu",
>> +                          disk->blkdeviotune.size_iops_sec);
>> +    }
>> +
>
> 3755        if (disk->blkdeviotune.read_iops_sec_max) {
> 3756            virBufferAsprintf(&opt, ",iops_rd_max=%llu",
> 3757                              disk->blkdeviotune.read_iops_sec_max);
> 3758        }
> 3759
>
> (1) Event original:     "disk->blkdeviotune.write_iops_sec_max" looks like the original copy.
> Also see events:        [copy_paste_error][remediation]
>
> 3760        if (disk->blkdeviotune.write_iops_sec_max) {
> 3761            virBufferAsprintf(&opt, ",iops_wr_max=%llu",
> 3762                              disk->blkdeviotune.write_iops_sec_max);
> 3763        }
> 3764
>
> (2) Event copy_paste_error:     "write_iops_sec_max" in "disk->blkdeviotune.write_iops_sec_max" looks like a copy-paste error.
> (3) Event remediation:  Should it say "size_iops_sec" instead?
> Also see events:        [original]
>
> 3765        if (disk->blkdeviotune.write_iops_sec_max) {
> 3766            virBufferAsprintf(&opt, ",iops_size=%llu",
> 3767                              disk->blkdeviotune.size_iops_sec);
> 3768        }
>
>
> I "assume" the (2) if should be "if (disk->blkdeviotune.size_iops_sec) {", correct?
>
>
> John
>>      if (virBufferCheckError(&opt) < 0)
>>          goto error;
>>
>>

Yes your right, I've made a mistake here.
I send the correction now.