[PATCH v2 3/5] api: add virNetworkCreateFlags

Kristina Hanicova posted 5 patches 4 years, 4 months ago
[PATCH v2 3/5] api: add virNetworkCreateFlags
Posted by Kristina Hanicova 4 years, 4 months ago
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
 include/libvirt/libvirt-network.h | 4 ++++
 src/libvirt-network.c             | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index e505c3eb7e..398d8fccd4 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -113,6 +113,10 @@ virNetworkPtr           virNetworkLookupByUUID          (virConnectPtr conn,
 virNetworkPtr           virNetworkLookupByUUIDString    (virConnectPtr conn,
                                                          const char *uuid);
 
+typedef enum {
+    VIR_NETWORK_CREATE_VALIDATE = 1 << 0, /* Validate the XML document against schema */
+} virNetworkCreateFlags;
+
 /*
  * Create active transient network
  */
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index ee53b9f2c5..883dd40f6b 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -431,7 +431,7 @@ virNetworkCreateXML(virConnectPtr conn, const char *xmlDesc)
  * virNetworkCreateXMLFlags:
  * @conn: pointer to the hypervisor connection
  * @xmlDesc: an XML description of the network
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virNetworkCreateFlags
  *
  * Create and start a new virtual network, based on an XML description
  * similar to the one returned by virNetworkGetXMLDesc()
-- 
2.31.1

Re: [PATCH v2 3/5] api: add virNetworkCreateFlags
Posted by Michal Prívozník 4 years, 4 months ago
On 9/15/21 1:07 PM, Kristina Hanicova wrote:
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> ---
>  include/libvirt/libvirt-network.h | 4 ++++
>  src/libvirt-network.c             | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
> index e505c3eb7e..398d8fccd4 100644
> --- a/include/libvirt/libvirt-network.h
> +++ b/include/libvirt/libvirt-network.h
> @@ -113,6 +113,10 @@ virNetworkPtr           virNetworkLookupByUUID          (virConnectPtr conn,
>  virNetworkPtr           virNetworkLookupByUUIDString    (virConnectPtr conn,
>                                                           const char *uuid);
>  
> +typedef enum {
> +    VIR_NETWORK_CREATE_VALIDATE = 1 << 0, /* Validate the XML document against schema */

Since in patch 4/5 we rely on the fact that VIR_NETWORK_CREATE_VALIDATE
= VIR_NETWORK_DEFINE_VALIDATE should we reflect this in define?

I mean, this could be

 VIR_NETWORK_CREATE_VALIDATE = VIR_NETWORK_DEFINE_VALIDATE /* Validate
... */

But since VIR_NETWORK_DEFINE_VALIDATE is declared only after these lines
we would also need to move this typedef :(


Alternatively, we may do G_STATIC_ASSERT(VIR_NETWORK_CREATE_VALIDATE ==
VIR_NETWORK_DEFINE_VALIDATE) somewhere in our code (we can't do it in
public header because that's glib-free).

Michal

Re: [PATCH v2 3/5] api: add virNetworkCreateFlags
Posted by Kristina Hanicova 4 years, 4 months ago
On Thu, Sep 16, 2021 at 4:02 PM Michal Prívozník <mprivozn@redhat.com>
wrote:

> On 9/15/21 1:07 PM, Kristina Hanicova wrote:
> > Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> > ---
> >  include/libvirt/libvirt-network.h | 4 ++++
> >  src/libvirt-network.c             | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libvirt/libvirt-network.h
> b/include/libvirt/libvirt-network.h
> > index e505c3eb7e..398d8fccd4 100644
> > --- a/include/libvirt/libvirt-network.h
> > +++ b/include/libvirt/libvirt-network.h
> > @@ -113,6 +113,10 @@ virNetworkPtr           virNetworkLookupByUUID
>     (virConnectPtr conn,
> >  virNetworkPtr           virNetworkLookupByUUIDString    (virConnectPtr
> conn,
> >                                                           const char
> *uuid);
> >
> > +typedef enum {
> > +    VIR_NETWORK_CREATE_VALIDATE = 1 << 0, /* Validate the XML document
> against schema */
>
> Since in patch 4/5 we rely on the fact that VIR_NETWORK_CREATE_VALIDATE
> = VIR_NETWORK_DEFINE_VALIDATE should we reflect this in define?
>
> I mean, this could be
>
>  VIR_NETWORK_CREATE_VALIDATE = VIR_NETWORK_DEFINE_VALIDATE /* Validate
> ... */
>
> But since VIR_NETWORK_DEFINE_VALIDATE is declared only after these lines
> we would also need to move this typedef :(
>
>
> Alternatively, we may do G_STATIC_ASSERT(VIR_NETWORK_CREATE_VALIDATE ==
> VIR_NETWORK_DEFINE_VALIDATE) somewhere in our code (we can't do it in
> public header because that's glib-free).
>
> Michal
>
>
Michal, please squash this into the fourth patch:


diff --git i/src/vbox/vbox_network.c w/src/vbox/vbox_network.c
index 343b8d35ad..9c27ccda56 100644
--- i/src/vbox/vbox_network.c
+++ w/src/vbox/vbox_network.c
@@ -385,9 +385,15 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const
char *xml, bool start,
     IHost *host = NULL;
     virNetworkPtr ret = NULL;
     nsresult rc;
+    bool validate;

-    virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE |
-                  VIR_NETWORK_CREATE_VALIDATE, NULL);
+    if (start) {
+        virCheckFlags(VIR_NETWORK_CREATE_VALIDATE, NULL);
+        validate = flags & VIR_NETWORK_CREATE_VALIDATE;
+    } else {
+        virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE, NULL);
+        validate = flags & VIR_NETWORK_DEFINE_VALIDATE;
+    }

     if (!data->vboxObj)
         return ret;
@@ -398,11 +404,7 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const
char *xml, bool start,

     VBOX_IID_INITIALIZE(&vboxnetiid);

-    /* Here we rely on the fact that VIR_NETWORK_DEFINE_VALIDATE and
-     * VIR_NETWORK_CREATE_VALIDATE always have the same value.
-    */
-    if (!(def = virNetworkDefParseString(xml, NULL,
-                                         !!(flags &
VIR_NETWORK_DEFINE_VALIDATE))) ||
+    if (!(def = virNetworkDefParseString(xml, NULL, validate)) ||
         (def->forward.type != VIR_NETWORK_FORWARD_NONE) ||
         (def->nips == 0 || !def->ips))
         goto cleanup;


Kristina