Re: [libvirt] [PATCH] blkdeviotune: check for overflow when parsing XML

On Tue, Aug 26, 2014 at 10:17:22AM +0200, Erik Skultety wrote: > According to docs/schemas/domaincommon.rng and _virDomainBlockIoTuneInfo > all the iotune values are interpreted as unsigned long long, however > according to qemu_monitor_json.c, qemu silently truncates numbers > larger than LLONG_MAX. There's really not much of a usage for such > large numbers anyway yet. This patch provides the same overflow > check during domain XML parsing phase as it does during setting > a blkdeviotune element in qemu_driver.c and thus reports an error when > a larger number than LLONG_MAX is detected. > --- > src/conf/domain_conf.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 22a7f7e..4cc900f 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -5711,6 +5711,18 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr > xmlopt, > def->blkdeviotune.write_iops_sec = 0; > } > > + if (def->blkdeviotune.total_bytes_sec > LLONG_MAX || > + def->blkdeviotune.read_bytes_sec > LLONG_MAX || > + def->blkdeviotune.write_bytes_sec > LLONG_MAX || > + def->blkdeviotune.total_iops_sec > LLONG_MAX || > + def->blkdeviotune.read_iops_sec > LLONG_MAX || > + def->blkdeviotune.write_iops_sec > LLONG_MAX) { > + virReportError(VIR_ERR_OVERFLOW, > + _("block I/O throttle limit must " > + "be less than %llu"), LLONG_MAX); > + goto error; > + } > + > if ((def->blkdeviotune.total_bytes_sec && > def->blkdeviotune.read_bytes_sec) || > (def->blkdeviotune.total_bytes_sec && > -- > 1.9.3 > Oh, c**p, so if someone has that in the XML, then such domain will disappear with the upgrade of libvirt daemon, right. We should instead put this in the starting phase (and any API that touches that data, hotplug, blkdeviotune, etc.), so such domains don't disappear. Fortunately that makes sense, too, because only QEMU truncates the value, but in case any other hypervisor may accept the values properly (in the future for example). Test case in tests/qemuxml2argvtest.c could be nice way to show that this is fixed ;) Martin -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list