Re: [libvirt PATCH 04/10] virDomainControllerDefParseXML: Use virXMLProp*

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux