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

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&gt; > --- > 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); >