When trying to set an invalid value into iotune element, standard behavior was to not report any error, rather to reset all affected subelements of the iotune element back to 0 which results in ignoring those particular subelements by XML generator. Patch further examines the return code of the virXPathULongLong function and in case of an invalid non-integer value raises an error. Fixed to preserve consistency with invalid value checking of other elements. Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811 --- src/conf/domain_conf.c | 55 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9557020..592aa94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5416,6 +5416,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *mirrorType = NULL; int expected_secret_usage = -1; int auth_secret_usage = -1; + int ret = 0; if (!(def = virDomainDiskDefNew())) return NULL; @@ -5644,40 +5645,70 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) { - if (virXPathULongLong("string(./iotune/total_bytes_sec)", + ret = virXPathULongLong("string(./iotune/total_bytes_sec)", ctxt, - &def->blkdeviotune.total_bytes_sec) < 0) { + &def->blkdeviotune.total_bytes_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.total_bytes_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("total throughput limit must be an integer")); + goto error; } - if (virXPathULongLong("string(./iotune/read_bytes_sec)", + ret = virXPathULongLong("string(./iotune/read_bytes_sec)", ctxt, - &def->blkdeviotune.read_bytes_sec) < 0) { + &def->blkdeviotune.read_bytes_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.read_bytes_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("read throughput limit must be an integer")); + goto error; } - if (virXPathULongLong("string(./iotune/write_bytes_sec)", + ret = virXPathULongLong("string(./iotune/write_bytes_sec)", ctxt, - &def->blkdeviotune.write_bytes_sec) < 0) { + &def->blkdeviotune.write_bytes_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.write_bytes_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("write throughput limit must be an integer")); + goto error; } - if (virXPathULongLong("string(./iotune/total_iops_sec)", + ret = virXPathULongLong("string(./iotune/total_iops_sec)", ctxt, - &def->blkdeviotune.total_iops_sec) < 0) { + &def->blkdeviotune.total_iops_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.total_iops_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("total I/O operations limit must be an integer")); + goto error; } - if (virXPathULongLong("string(./iotune/read_iops_sec)", + ret = virXPathULongLong("string(./iotune/read_iops_sec)", ctxt, - &def->blkdeviotune.read_iops_sec) < 0) { + &def->blkdeviotune.read_iops_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.read_iops_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("read I/O operations limit must be an integer")); + goto error; } - if (virXPathULongLong("string(./iotune/write_iops_sec)", + ret = virXPathULongLong("string(./iotune/write_iops_sec)", ctxt, - &def->blkdeviotune.write_iops_sec) < 0) { + &def->blkdeviotune.write_iops_sec); + if (ret < 0 && ret != -2) { def->blkdeviotune.write_iops_sec = 0; + } else if (ret == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("write I/O operations limit must be an integer")); + goto error; } if ((def->blkdeviotune.total_bytes_sec && -- 1.9.3