On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx> --- src/conf/domain_conf.c | 281 +++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 178 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e79b20e6..152b4b8813 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9499,39 +9499,19 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
[...]
if (!(def = virDomainControllerDefNew(type))) return NULL; - model = virXMLPropString(node, "model"); - if (model) { + if ((model = virXMLPropString(node, "model"))) { if ((def->model = virDomainControllerModelTypeFromString(def, model)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown model type '%s'"), model); @@ -9542,8 +9522,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, idx = virXMLPropString(node, "index"); if (idx) { unsigned int idxVal; - if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || - idxVal > INT_MAX) { + if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 || idxVal > INT_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse controller index %s"), idx); return NULL;
The two hunks above only reformat code. Please don't mix functional and cosmetic changes in one commit.
@@ -9579,12 +9581,39 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, "controller definition not allowed")); return NULL; } - chassisNr = virXMLPropString(cur, "chassisNr"); - chassis = virXMLPropString(cur, "chassis"); - port = virXMLPropString(cur, "port"); - busNr = virXMLPropString(cur, "busNr"); - hotplug = virXMLPropString(cur, "hotplug"); - targetIndex = virXMLPropString(cur, "index"); + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (virXMLPropInt(cur, "chassisNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassisNr) < 0) + return NULL; + + if (virXMLPropInt(cur, "chassis", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.chassis) < 0) + return NULL; + + if (virXMLPropInt(cur, "port", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.port) < 0) + return NULL; + + if (virXMLPropInt(cur, "busNr", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.busNr) < 0) + return NULL; + + if (virXMLPropTristateSwitch(cur, "hotplug", + VIR_XML_PROP_NONE, + &def->opts.pciopts.hotplug) < 0) + return NULL; + + if ((rc = virXMLPropInt(cur, "index", 0, VIR_XML_PROP_NONE, + &def->opts.pciopts.targetIndex)) < 0) + return NULL; + + if (rc && def->opts.pciopts.targetIndex == -1) {
if (rc == 1 && ...) per our coding style: https://libvirt.org/coding-style.html#conditional-expressions
+ virReportError(VIR_ERR_XML_ERROR, + _("Invalid target index '%i' in PCI controller"), + def->opts.pciopts.targetIndex); + } + } + processedTarget = true; } } @@ -9645,30 +9638,28 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - portsStr = virXMLPropString(node, "ports"); - if (portsStr) { - int r = virStrToLong_i(portsStr, NULL, 10, &ports); - if (r != 0 || ports < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid ports: %s"), portsStr); - return NULL; - } + if ((rc = virXMLPropInt(node, "ports", 10, VIR_XML_PROP_NONE, &ports)) < 0) + return NULL; + if (rc && ports < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid ports: %i"), ports); + return NULL; }
Same here.
switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: { - g_autofree char *vectors = virXMLPropString(node, "vectors"); + if ((rc = virXMLPropInt(node, "vectors", 10, VIR_XML_PROP_NONE, + &def->opts.vioserial.vectors)) < 0) + return NULL; - def->opts.vioserial.ports = ports; - if (vectors) { - int r = virStrToLong_i(vectors, NULL, 10, - &def->opts.vioserial.vectors); - if (r != 0 || def->opts.vioserial.vectors < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vectors: %s"), vectors); - return NULL; - } + if (rc && def->opts.vioserial.vectors < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vectors: %i"), + def->opts.vioserial.vectors); + return NULL;
And here.
} + + def->opts.vioserial.ports = ports; break; } case VIR_DOMAIN_CONTROLLER_TYPE_USB: {
[...]
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: { - g_autofree char *gntframes = virXMLPropString(node, "maxGrantFrames"); - g_autofree char *eventchannels = virXMLPropString(node, "maxEventChannels"); + if ((rc = virXMLPropInt(node, "maxGrantFrames", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxGrantFrames)) < 0)
Indentation is off.
+ return NULL; - if (gntframes) { - int r = virStrToLong_i(gntframes, NULL, 10, - &def->opts.xenbusopts.maxGrantFrames); - if (r != 0 || def->opts.xenbusopts.maxGrantFrames < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxGrantFrames: %s"), gntframes); - return NULL; - } + if (rc && def->opts.xenbusopts.maxGrantFrames < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxGrantFrames: %i"), + def->opts.xenbusopts.maxGrantFrames); + return NULL;
Here too.
} - if (eventchannels) { - int r = virStrToLong_i(eventchannels, NULL, 10, - &def->opts.xenbusopts.maxEventChannels); - if (r != 0 || def->opts.xenbusopts.maxEventChannels < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid maxEventChannels: %s"), eventchannels); - return NULL; - } + + if ((rc = virXMLPropInt(node, "maxEventChannels", 10, VIR_XML_PROP_NONE, + &def->opts.xenbusopts.maxEventChannels)) < 0) + return NULL; + + if (rc && def->opts.xenbusopts.maxEventChannels < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid maxEventChannels: %i"), + def->opts.xenbusopts.maxEventChannels);
And here.
+ return NULL; } break; }
Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: PGP signature