On a Friday in 2021, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
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(a)redhat.com>
Jano