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

2014-11-12 Thread Matthias Gatto 
On Tue, Nov 11, 2014 at 1:20 PM, John Ferlan  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 
>> ---
>>  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);
>> +}
>> +
>
> 3755if (disk->blkdeviotune.read_iops_sec_max) {
> 3756virBufferAsprintf(&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]
>
> 3760if (disk->blkdeviotune.write_iops_sec_max) {
> 3761virBufferAsprintf(&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]
>
> 3765if (disk->blkdeviotune.write_iops_sec_max) {
> 3766virBufferAsprintf(&opt, ",iops_size=%llu",
> 3767  disk->blkdeviotune.size_iops_sec);
> 3768

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

2014-11-11 Thread 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 
> ---
>  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);
> +}
> +

3755if (disk->blkdeviotune.read_iops_sec_max) {
3756virBufferAsprintf(&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]

3760if (disk->blkdeviotune.write_iops_sec_max) {
3761virBufferAsprintf(&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]

3765if (disk->blkdeviotune.write_iops_sec_max) {
3766virBufferAsprintf(&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;
>  
> 

--
libvi

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

2014-11-07 Thread Michal Privoznik 

On 29.10.2014 13:16, 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 
---
  src/qemu/qemu_command.c | 57 -
  1 file changed, 56 insertions(+), 1 deletion(-)

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;


This is rather a long line. Can you split it better, so that it has max 
~80 characters?



+}
+



Michal

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

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

2014-10-29 Thread Matthias Gatto 
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 
---
 src/qemu/qemu_command.c | 57 -
 1 file changed, 56 insertions(+), 1 deletion(-)

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);
+}
+
 if (virBufferCheckError(&opt) < 0)
 goto error;
 
-- 
1.8.3.1

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