On Thu, Jun 09, 2016 at 10:56:01AM -0400, John Ferlan wrote:
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(a)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)
More of a good point then a nit, actually.
Couldn't decide if CONFIG_UNSUPPORTED was better than perhaps
OPERATION_UNSUPPORTED which indicates that qemu refuses to accept just
_max without non-_max
This was a copy from few lines above when we report CONFIG_UNSUPPORTED
for some values or older QEMU.
Need to add:
#undef CHECK_MAX
oh, forgot that completely
ACK w/ that added.
Thanks, pushed.
John
> qemuDomainObjEnterMonitor(driver, vm);
> ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
> &info, supportMaxOptions);
>