[PATCH] src: network_conf: propagate only bool to virNetworkDefParseString()

Kristina Hanicova posted 1 patch 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/f6245fee60d84a262038d7529b5c76b95804cfa2.1631285067.git.khanicov@redhat.com
src/conf/domain_conf.c       |  2 +-
src/conf/network_conf.c      | 12 ++++++------
src/conf/network_conf.h      |  2 +-
src/esx/esx_network_driver.c |  2 +-
src/network/bridge_driver.c  |  5 +++--
src/qemu/qemu_process.c      |  2 +-
src/test/test_driver.c       |  5 +++--
src/vbox/vbox_network.c      |  3 ++-
8 files changed, 18 insertions(+), 15 deletions(-)

[PATCH] src: network_conf: propagate only bool to virNetworkDefParseString()

Posted by Kristina Hanicova 2 weeks ago
We don't need to propagate all public flags, only the information
about the presence of the validation one, which can differ from
function to function. This patch makes it easier and more
readable in case of a future additions of validation flags.
This change was suggested by Daniel.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
 src/conf/domain_conf.c       |  2 +-
 src/conf/network_conf.c      | 12 ++++++------
 src/conf/network_conf.h      |  2 +-
 src/esx/esx_network_driver.c |  2 +-
 src/network/bridge_driver.c  |  5 +++--
 src/qemu/qemu_process.c      |  2 +-
 src/test/test_driver.c       |  5 +++--
 src/vbox/vbox_network.c      |  3 ++-
 8 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb9e7218ff..858f6f923a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30523,7 +30523,7 @@ virDomainNetResolveActualType(virDomainNetDef *iface)
     if (!(xml = virNetworkGetXMLDesc(net, 0)))
         goto cleanup;
 
-    if (!(def = virNetworkDefParseString(xml, NULL, 0)))
+    if (!(def = virNetworkDefParseString(xml, NULL, false)))
         goto cleanup;
 
     switch ((virNetworkForwardType) def->forward.type) {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f23599abac..7a0f6f02c3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -318,7 +318,7 @@ virNetworkDefCopy(virNetworkDef *def,
     if (!(xml = virNetworkDefFormat(def, xmlopt, flags)))
        return NULL;
 
-    return virNetworkDefParseString(xml, xmlopt, 0);
+    return virNetworkDefParseString(xml, xmlopt, false);
 }
 
 
@@ -2086,14 +2086,14 @@ static virNetworkDef *
 virNetworkDefParse(const char *xmlStr,
                    const char *filename,
                    virNetworkXMLOption *xmlopt,
-                   unsigned int flags)
+                   bool validate_flag)
 {
     g_autoptr(xmlDoc) xml = NULL;
     virNetworkDef *def = NULL;
     int keepBlanksDefault = xmlKeepBlanksDefault(0);
 
     if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)"),
-                           "network.rng", flags & VIR_NETWORK_DEFINE_VALIDATE)))
+                           "network.rng", validate_flag)))
         def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt);
 
     xmlKeepBlanksDefault(keepBlanksDefault);
@@ -2104,9 +2104,9 @@ virNetworkDefParse(const char *xmlStr,
 virNetworkDef *
 virNetworkDefParseString(const char *xmlStr,
                          virNetworkXMLOption *xmlopt,
-                         unsigned int flags)
+                         bool validate_flag)
 {
-    return virNetworkDefParse(xmlStr, NULL, xmlopt, flags);
+    return virNetworkDefParse(xmlStr, NULL, xmlopt, validate_flag);
 }
 
 
@@ -2114,7 +2114,7 @@ virNetworkDef *
 virNetworkDefParseFile(const char *filename,
                        virNetworkXMLOption *xmlopt)
 {
-    return virNetworkDefParse(NULL, filename, xmlopt, 0);
+    return virNetworkDefParse(NULL, filename, xmlopt, false);
 }
 
 
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 30958eff64..1dda45b25c 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -329,7 +329,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
 virNetworkDef *
 virNetworkDefParseString(const char *xmlStr,
                          virNetworkXMLOption *xmlopt,
-                         unsigned int flags);
+                         bool validate_flag);
 
 virNetworkDef *
 virNetworkDefParseFile(const char *filename,
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 673ed5421f..8acf7591cf 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -298,7 +298,7 @@ esxNetworkDefineXMLFlags(virConnectPtr conn, const char *xml,
         return NULL;
 
     /* Parse network XML */
-    def = virNetworkDefParseString(xml, NULL, flags);
+    def = virNetworkDefParseString(xml, NULL, flags & VIR_NETWORK_DEFINE_VALIDATE);
 
     if (!def)
         return NULL;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 06822fb3a0..4b6daf19a8 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3474,7 +3474,7 @@ networkCreateXML(virConnectPtr conn,
     virNetworkPtr net = NULL;
     virObjectEvent *event = NULL;
 
-    if (!(newDef = virNetworkDefParseString(xml, network_driver->xmlopt, 0)))
+    if (!(newDef = virNetworkDefParseString(xml, network_driver->xmlopt, false)))
         goto cleanup;
 
     if (virNetworkCreateXMLEnsureACL(conn, newDef) < 0)
@@ -3529,7 +3529,8 @@ networkDefineXMLFlags(virConnectPtr conn,
 
     virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE, NULL);
 
-    if (!(def = virNetworkDefParseString(xml, network_driver->xmlopt, flags)))
+    if (!(def = virNetworkDefParseString(xml, network_driver->xmlopt,
+                                         flags & VIR_NETWORK_DEFINE_VALIDATE)))
         goto cleanup;
 
     if (virNetworkDefineXMLFlagsEnsureACL(conn, def) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 207129a556..51d02e9fb6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4778,7 +4778,7 @@ qemuProcessGetNetworkAddress(const char *netname,
     if (!xml)
         goto cleanup;
 
-    netdef = virNetworkDefParseString(xml, NULL, 0);
+    netdef = virNetworkDefParseString(xml, NULL, false);
     if (!netdef)
         goto cleanup;
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2f19b7c520..33f611081c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5581,7 +5581,7 @@ testNetworkCreateXML(virConnectPtr conn, const char *xml)
     virNetworkPtr net = NULL;
     virObjectEvent *event = NULL;
 
-    if ((newDef = virNetworkDefParseString(xml, NULL, 0)) == NULL)
+    if ((newDef = virNetworkDefParseString(xml, NULL, false)) == NULL)
         goto cleanup;
 
     if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef,
@@ -5620,7 +5620,8 @@ testNetworkDefineXMLFlags(virConnectPtr conn,
 
     virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE, NULL);
 
-    if ((newDef = virNetworkDefParseString(xml, NULL, flags)) == NULL)
+    if (!(newDef = virNetworkDefParseString(xml, NULL,
+                                            flags & VIR_NETWORK_DEFINE_VALIDATE)))
         goto cleanup;
 
     if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef, 0)))
diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c
index c554b052c9..c0694d42d5 100644
--- a/src/vbox/vbox_network.c
+++ b/src/vbox/vbox_network.c
@@ -397,7 +397,8 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start,
 
     VBOX_IID_INITIALIZE(&vboxnetiid);
 
-    if (!(def = virNetworkDefParseString(xml, NULL, flags)) ||
+    if (!(def = virNetworkDefParseString(xml, NULL,
+                                         flags & VIR_NETWORK_DEFINE_VALIDATE)) ||
         (def->forward.type != VIR_NETWORK_FORWARD_NONE) ||
         (def->nips == 0 || !def->ips))
         goto cleanup;
-- 
2.31.1

Re: [PATCH] src: network_conf: propagate only bool to virNetworkDefParseString()

Posted by Michal Prívozník 2 weeks ago
On 9/10/21 4:48 PM, Kristina Hanicova wrote:
> We don't need to propagate all public flags, only the information
> about the presence of the validation one, which can differ from
> function to function. This patch makes it easier and more
> readable in case of a future additions of validation flags.
> This change was suggested by Daniel.
> 
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> ---
>  src/conf/domain_conf.c       |  2 +-
>  src/conf/network_conf.c      | 12 ++++++------
>  src/conf/network_conf.h      |  2 +-
>  src/esx/esx_network_driver.c |  2 +-
>  src/network/bridge_driver.c  |  5 +++--
>  src/qemu/qemu_process.c      |  2 +-
>  src/test/test_driver.c       |  5 +++--
>  src/vbox/vbox_network.c      |  3 ++-
>  8 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cb9e7218ff..858f6f923a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30523,7 +30523,7 @@ virDomainNetResolveActualType(virDomainNetDef *iface)
>      if (!(xml = virNetworkGetXMLDesc(net, 0)))
>          goto cleanup;
>  
> -    if (!(def = virNetworkDefParseString(xml, NULL, 0)))
> +    if (!(def = virNetworkDefParseString(xml, NULL, false)))
>          goto cleanup;
>  
>      switch ((virNetworkForwardType) def->forward.type) {
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index f23599abac..7a0f6f02c3 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -318,7 +318,7 @@ virNetworkDefCopy(virNetworkDef *def,
>      if (!(xml = virNetworkDefFormat(def, xmlopt, flags)))
>         return NULL;
>  
> -    return virNetworkDefParseString(xml, xmlopt, 0);
> +    return virNetworkDefParseString(xml, xmlopt, false);
>  }
>  
>  
> @@ -2086,14 +2086,14 @@ static virNetworkDef *
>  virNetworkDefParse(const char *xmlStr,
>                     const char *filename,
>                     virNetworkXMLOption *xmlopt,
> -                   unsigned int flags)
> +                   bool validate_flag)

I think we can call it just 'validate'.

And there are some other places where we are passing flags &
VIR_NETWORK_DEFINE_VALIDATE. Those should be fixed to !!(flags &
VIR_NETWORK_DEFINE_VALIDATE).

I'm fixing this patch per my suggestions and pushing.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal