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@xxxxxxxxxx> --- 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.
JohnqemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, supportMaxOptions);
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list