[libvirt] [PATCH] qemu: Yet another check for blkdeviotune values

[John Ferlan]

On 06/07/2016 05:26 PM, Martin Kletzander wrote:
> If you want to set block device I/O tuning values that end with '_max'
> and there is nothing else set, libvirt emits an error.  In particular:
> 
>   error: internal error: Unexpected error
> 
> That's an unknown error.  That is because *_max values depend on their
> respective non-_max values.  QEMU even says that in the error message
> sent as a response to the monitor command:
> 
>   "error": {"class": "GenericError", "desc": "bps_max/iops_max require
>   corresponding bps/iops values"}
> 
> the problem was that we didn't know that and there was no check for it.
> Adding such check makes sure that there will be less confused users.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a7202ed3e3a1..d73238a66c66 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17391,6 +17391,25 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>          }
>          if (!set_size_iops)
>              info.size_iops_sec = oldinfo->size_iops_sec;
> +
> +#define CHECK_MAX(val)                                                  \
> +        do {                                                            \
> +            if (info.val##_max && !info.val) {                          \
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,              \
> +                               _("Value '%s' cannot be set if "         \
> +                                 "'%s' is not set"),                    \
> +                               #val "_max", #val);                      \
> +                goto endjob;                                            \
> +            }                                                           \
> +        } while (false);
> +
> +        CHECK_MAX(total_bytes_sec);
> +        CHECK_MAX(read_bytes_sec);
> +        CHECK_MAX(write_bytes_sec);
> +        CHECK_MAX(total_iops_sec);
> +        CHECK_MAX(read_iops_sec);
> +        CHECK_MAX(write_iops_sec);
> +

s/Value/value/  (NIT: and only since  you'll be in there)

Couldn't decide if CONFIG_UNSUPPORTED was better than perhaps
OPERATION_UNSUPPORTED which indicates that qemu refuses to accept just
_max without non-_max

Need to add:

#undef CHECK_MAX

ACK w/ that added.

John

>          qemuDomainObjEnterMonitor(driver, vm);
>          ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
>                                              &info, supportMaxOptions);
>