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

[John Ferlan]

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;
>  
>