4:27 a.m.
On Tue, Nov 11, 2014 at 1:20 PM, John Ferlan <jferlan(a)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(a)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.