[libvirt] [PATCHv2] conf: Report errors on cputune parameter parsing

This patch adds proper error reporting if parsing of cputune parameters fails due to incorrect values provided by the user. Previously no errors were reported in such a case and the failure was silently ignored. --- v2: report errors instead of ignoring them src/conf/domain_conf.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ca645c7..1422b47 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9471,29 +9471,43 @@ virDomainDefParseXML(virCapsPtr caps, /* Extract cpu tunables. */ if (virXPathULong("string(./cputune/shares[1])", ctxt, - &def->cputune.shares) < 0) - def->cputune.shares = 0; + &def->cputune.shares) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune shares value")); + goto error; + } if (virXPathULongLong("string(./cputune/period[1])", ctxt, - &def->cputune.period) < 0) - def->cputune.period = 0; + &def->cputune.period) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune period value")); + goto error; + } if (virXPathLongLong("string(./cputune/quota[1])", ctxt, - &def->cputune.quota) < 0) - def->cputune.quota = 0; + &def->cputune.quota) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune quota value")); + goto error; + } if (virXPathULongLong("string(./cputune/emulator_period[1])", ctxt, - &def->cputune.emulator_period) < 0) - def->cputune.emulator_period = 0; + &def->cputune.emulator_period) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune emulator period value")); + goto error; + } if (virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt, - &def->cputune.emulator_quota) < 0) - def->cputune.emulator_quota = 0; - - if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { + &def->cputune.emulator_quota) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune emualtor quota value")); goto error; } + if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) + goto error; + if (n && VIR_ALLOC_N(def->cputune.vcpupin, n) < 0) goto no_memory; -- 1.8.1.1

On 03/04/2013 07:08 AM, Peter Krempa wrote:
This patch adds proper error reporting if parsing of cputune parameters fails due to incorrect values provided by the user. Previously no errors were reported in such a case and the failure was silently ignored. --- v2: report errors instead of ignoring them
- - if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { + &def->cputune.emulator_quota) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune emualtor quota value"));
s/emualtor/emulator/ ACK with that fixed, and seems safe enough for 1.0.3 if you see this in time. I had to go researching why you only printed errors for < -1; but I confirm that in util/virxml.c, the function is documented as returning -1 for not present, -2 for present but parse error; and as these fields are optional, not present does not mean an error. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/04/13 22:41, Eric Blake wrote:
On 03/04/2013 07:08 AM, Peter Krempa wrote:
This patch adds proper error reporting if parsing of cputune parameters fails due to incorrect values provided by the user. Previously no errors were reported in such a case and the failure was silently ignored. --- v2: report errors instead of ignoring them
- - if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { + &def->cputune.emulator_quota) < -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't parse cputune emualtor quota value"));
s/emualtor/emulator/
ACK with that fixed, and seems safe enough for 1.0.3 if you see this in time. I had to go researching why you only printed errors for < -1; but I confirm that in util/virxml.c, the function is documented as returning -1 for not present, -2 for present but parse error; and as these fields are optional, not present does not mean an error.
Thanks; pushed; unfortunately after the release. Peter
participants (2)
-
Eric Blake
-
Peter Krempa