On 08/21/2014 02:28 PM, Erik Skultety wrote: > 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, The indentation is off here - c and " should be on the same column. > - &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; > } I'd rewrite these conditions as if (ret == -2) { goto error; } else if (ret < 0) { } Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list