[libvirt PATCH 06/10] virDomainActualNetDefParseXML: Use virXMLProp*

Tim Wiederhake posted 10 patches 4 years, 9 months ago
There is a newer version of this series
[libvirt PATCH 06/10] virDomainActualNetDefParseXML: Use virXMLProp*
Posted by Tim Wiederhake 4 years, 9 months ago
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 src/conf/domain_conf.c | 57 ++++++++++++++----------------------------
 1 file changed, 19 insertions(+), 38 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 554efe0aea..3051a5d56d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10066,49 +10066,30 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
     xmlNodePtr bandwidth_node = NULL;
     xmlNodePtr vlanNode;
     xmlNodePtr virtPortNode;
-    g_autofree char *type = NULL;
-    g_autofree char *mode = NULL;
     g_autofree char *addrtype = NULL;
-    g_autofree char *trustGuestRxFilters = NULL;
     g_autofree char *macTableManager = NULL;
-    int type_value;
 
     actual = g_new0(virDomainActualNetDef, 1);
 
     ctxt->node = node;
 
-    type = virXMLPropString(node, "type");
-    if (!type) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("missing type attribute in interface's <actual> element"));
+    if (virXMLPropEnum(node, "type", virDomainNetTypeFromString,
+                       VIR_XML_PROP_REQUIRED, &actual->type) < 0)
         goto error;
-    }
-    if ((type_value = virDomainNetTypeFromString(type)) < 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unknown type '%s' in interface's <actual> element"), type);
-        goto error;
-    }
-    actual->type = type_value;
+
     if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
         actual->type != VIR_DOMAIN_NET_TYPE_DIRECT &&
         actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
         actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unsupported type '%s' in interface's <actual> element"),
-                       type);
+                       virDomainNetTypeToString(actual->type));
         goto error;
     }
 
-    if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) {
-        int value;
-        if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("unknown trustGuestRxFilters value '%s'"),
-                           trustGuestRxFilters);
-            goto error;
-        }
-        actual->trustGuestRxFilters = value;
-    }
+    if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE,
+                               &actual->trustGuestRxFilters) < 0)
+        goto error;
 
     virtPortNode = virXPathNode("./virtualport", ctxt);
     if (virtPortNode) {
@@ -10127,7 +10108,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
         } else {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("<virtualport> element unsupported for type='%s'"
-                             " in interface's <actual> element"), type);
+                             " in interface's <actual> element"),
+                             virDomainNetTypeToString(actual->type));
             goto error;
         }
     }
@@ -10136,19 +10118,18 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
         xmlNodePtr sourceNode = virXPathNode("./source[1]", ctxt);
 
         if (sourceNode) {
+            int rc;
+            virNetDevMacVLanMode mode;
+
             actual->data.direct.linkdev = virXMLPropString(sourceNode, "dev");
 
-            mode = virXMLPropString(sourceNode, "mode");
-            if (mode) {
-                int m;
-                if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("Unknown mode '%s' in interface <actual> element"),
-                                   mode);
-                    goto error;
-                }
-                actual->data.direct.mode = m;
-            }
+            if ((rc = virXMLPropEnum(sourceNode, "mode",
+                               virNetDevMacVLanModeTypeFromString,
+                               VIR_XML_PROP_NONE, &mode)) < 0)
+                goto error;
+
+            if (rc)
+                actual->data.direct.mode = mode;
         }
     } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
         virDomainHostdevDef *hostdev = &actual->data.hostdev.def;
-- 
2.26.3

Re: [libvirt PATCH 06/10] virDomainActualNetDefParseXML: Use virXMLProp*
Posted by Ján Tomko 4 years, 9 months ago
On a Friday in 2021, Tim Wiederhake wrote:
>Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
>---
> src/conf/domain_conf.c | 57 ++++++++++++++----------------------------
> 1 file changed, 19 insertions(+), 38 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 554efe0aea..3051a5d56d 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -10066,49 +10066,30 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
>     xmlNodePtr bandwidth_node = NULL;
>     xmlNodePtr vlanNode;
>     xmlNodePtr virtPortNode;
>-    g_autofree char *type = NULL;
>-    g_autofree char *mode = NULL;
>     g_autofree char *addrtype = NULL;
>-    g_autofree char *trustGuestRxFilters = NULL;
>     g_autofree char *macTableManager = NULL;
>-    int type_value;
>
>     actual = g_new0(virDomainActualNetDef, 1);
>
>     ctxt->node = node;
>
>-    type = virXMLPropString(node, "type");
>-    if (!type) {
>-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                       _("missing type attribute in interface's <actual> element"));
>+    if (virXMLPropEnum(node, "type", virDomainNetTypeFromString,
>+                       VIR_XML_PROP_REQUIRED, &actual->type) < 0)
>         goto error;
>-    }
>-    if ((type_value = virDomainNetTypeFromString(type)) < 0) {
>-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                       _("unknown type '%s' in interface's <actual> element"), type);
>-        goto error;
>-    }
>-    actual->type = type_value;
>+
>     if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
>         actual->type != VIR_DOMAIN_NET_TYPE_DIRECT &&
>         actual->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>         actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        _("unsupported type '%s' in interface's <actual> element"),
>-                       type);
>+                       virDomainNetTypeToString(actual->type));
>         goto error;
>     }
>
>-    if ((trustGuestRxFilters = virXMLPropString(node, "trustGuestRxFilters"))) {
>-        int value;
>-        if ((value = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) {
>-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                           _("unknown trustGuestRxFilters value '%s'"),
>-                           trustGuestRxFilters);
>-            goto error;
>-        }
>-        actual->trustGuestRxFilters = value;
>-    }
>+    if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE,
>+                               &actual->trustGuestRxFilters) < 0)
>+        goto error;
>
>     virtPortNode = virXPathNode("./virtualport", ctxt);
>     if (virtPortNode) {
>@@ -10127,7 +10108,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
>         } else {
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>                            _("<virtualport> element unsupported for type='%s'"
>-                             " in interface's <actual> element"), type);
>+                             " in interface's <actual> element"),
>+                             virDomainNetTypeToString(actual->type));
>             goto error;
>         }
>     }
>@@ -10136,19 +10118,18 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
>         xmlNodePtr sourceNode = virXPathNode("./source[1]", ctxt);
>
>         if (sourceNode) {
>+            int rc;
>+            virNetDevMacVLanMode mode;
>+
>             actual->data.direct.linkdev = virXMLPropString(sourceNode, "dev");
>
>-            mode = virXMLPropString(sourceNode, "mode");
>-            if (mode) {
>-                int m;
>-                if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) {
>-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                                   _("Unknown mode '%s' in interface <actual> element"),
>-                                   mode);
>-                    goto error;
>-                }
>-                actual->data.direct.mode = m;
>-            }
>+            if ((rc = virXMLPropEnum(sourceNode, "mode",
>+                               virNetDevMacVLanModeTypeFromString,
>+                               VIR_XML_PROP_NONE, &mode)) < 0)

Indentation is off.

>+                goto error;
>+
>+            if (rc)
>+                actual->data.direct.mode = mode;

You can just pass actual->data.direct.mode as an argument to the
virXMLPropEnum call, eliminating the need for this if and both of the
local variables, unless I'm missing something.

>         }
>     } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>         virDomainHostdevDef *hostdev = &actual->data.hostdev.def;

With that addressed:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH 06/10] virDomainActualNetDefParseXML: Use virXMLProp*
Posted by Tim Wiederhake 4 years, 9 months ago
On Fri, 2021-04-23 at 15:35 +0200, Ján Tomko wrote:
> On a Friday in 2021, Tim Wiederhake wrote:
> > 
> > (...)
> >
> > +                goto error;
> > +
> > +            if (rc)
> > +                actual->data.direct.mode = mode;
> 
> You can just pass actual->data.direct.mode as an argument to the
> virXMLPropEnum call, eliminating the need for this if and both of the
> local variables, unless I'm missing something.

acutal->data.direct.mode is an int, so we have to do the extra step.

> >         }
> >     } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> >         virDomainHostdevDef *hostdev = &actual->data.hostdev.def;
> 
> With that addressed:
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> Jano