Re: [libvirt] [PATCH v2 10/23] domain-conf: add network def private data

6 Sep 2019

On 8/8/19 4:55 PM, marcandre.lureau@redhat.com wrote:
...
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
  src/conf/domain_conf.c | 21 ++++++++++++++++++++-
  src/conf/domain_conf.h |  6 ++++++
  2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0456369d55..fb0904177f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2454,6 +2454,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
      if (!def)
          return;
      virDomainNetDefClear(def);
+    virObjectUnref(def->privateData);
      VIR_FREE(def);
  }
@@ -11441,7 +11442,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
      VIR_AUTOFREE(char *) trustGuestRxFilters = NULL;
      VIR_AUTOFREE(char *) vhost_path = NULL;
-    if (VIR_ALLOC(def) < 0)
+    if (!(def = virDomainNetDefNew(xmlopt)))
          return NULL;
ctxt->node = node;
@@ -14337,6 +14338,24 @@ virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt)
  }
+virDomainNetDefPtr
+virDomainNetDefNew(virDomainXMLOptionPtr xmlopt)
+{
+    virDomainNetDefPtr def = NULL;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    if (xmlopt && xmlopt->privateData.networkNew &&
+        !(def->privateData = xmlopt->privateData.networkNew())) {
+        VIR_FREE(def);
+        def = NULL;
This call to 'def = NULL' is not needed. VIR_FREE() does that for us. However, I actually prefer using virDomainNetDefFree() as that is more fool proof if somebody ever allocs something else in @def after VIR_ALLOC() and before these lines.
...
+    }
+
+    return def;
+}
+
+
  /* Parse the XML definition for a graphics device */
  static virDomainGraphicsDefPtr
  virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 57ca2a8ad1..9bd196b53c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1018,6 +1018,7 @@ struct _virDomainNetDef {
      unsigned int mtu;
      virNetDevCoalescePtr coalesce;
      virDomainVirtioOptionsPtr virtio;
+    virObjectPtr privateData;
  };
typedef enum {
@@ -2711,6 +2712,7 @@ struct _virDomainXMLPrivateDataCallbacks {
      virDomainXMLPrivateDataNewFunc    chrSourceNew;
      virDomainXMLPrivateDataNewFunc    vsockNew;
      virDomainXMLPrivateDataNewFunc    graphicsNew;
+    virDomainXMLPrivateDataNewFunc    networkNew;
      virDomainXMLPrivateDataFormatFunc format;
      virDomainXMLPrivateDataParseFunc  parse;
      /* following function shall return a pointer which will be used as the
@@ -2894,6 +2896,10 @@ virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
virDomainGraphicsDefPtr
  virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt);
+
+virDomainNetDefPtr
+virDomainNetDefNew(virDomainXMLOptionPtr xmlopt);
This function must be exposed in libvirt_private.syms too.

I've identified other places where we VIR_ALLOC() a virDomainNetDef structure:

bhyveParsePCINet(), xenParseVif(), lxcCreateNetDef() and vboxDumpNetwork()

In some of them we already have xmlopt available, in others we might pass NULL safely:

diff --git c/src/bhyve/bhyve_parse_command.c w/src/bhyve/bhyve_parse_command.c
index 490381688c..7d460e9824 100644
--- c/src/bhyve/bhyve_parse_command.c
+++ w/src/bhyve/bhyve_parse_command.c
@@ -501,7 +501,7 @@ bhyveParsePCINet(virDomainDefPtr def,
     const char *separator = NULL;
     const char *mac = NULL;
 
-    if (VIR_ALLOC(net) < 0)
+    if (!(net = virDomainNetDefNew(xmlopt)))
         goto cleanup;
 
     /* As we only support interface type='bridge' and cannot
diff --git c/src/libvirt_private.syms w/src/libvirt_private.syms
index a34d92f5ef..f1fe7259f9 100644
--- c/src/libvirt_private.syms
+++ w/src/libvirt_private.syms
@@ -478,6 +478,7 @@ virDomainNetDefActualToNetworkPort;
 virDomainNetDefClear;
 virDomainNetDefFormat;
 virDomainNetDefFree;
+virDomainNetDefNew;
 virDomainNetDefToNetworkPort;
 virDomainNetFind;
 virDomainNetFindByName;
diff --git c/src/libxl/xen_common.c w/src/libxl/xen_common.c
index 7eb52c8c84..d327f03d73 100644
--- c/src/libxl/xen_common.c
+++ w/src/libxl/xen_common.c
@@ -1234,7 +1234,7 @@ xenParseVif(char *entry, const char *vif_typename)
         key = nextkey;
     }
 
-    if (VIR_ALLOC(net) < 0)
+    if (!(net = virDomainNetDefNew(NULL)))
         goto cleanup;
 
     if (mac[0]) {
diff --git c/src/lxc/lxc_native.c w/src/lxc/lxc_native.c
index b4c6e790d8..018eec6977 100644
--- c/src/lxc/lxc_native.c
+++ w/src/lxc/lxc_native.c
@@ -359,7 +359,7 @@ lxcCreateNetDef(const char *type,
     virDomainNetDefPtr net = NULL;
     virMacAddr macAddr;
 
-    if (VIR_ALLOC(net) < 0)
+    if (!(net = virDomainNetDefNew(NULL)))
         goto error;
 
     if (STREQ_NULLABLE(flag, "up"))
diff --git c/src/vbox/vbox_common.c w/src/vbox/vbox_common.c
index 49e657cdb7..ddabcb80ca 100644
--- c/src/vbox/vbox_common.c
+++ w/src/vbox/vbox_common.c
@@ -3692,7 +3692,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter)
     char *utf8 = NULL;
     virDomainNetDefPtr net = NULL;
 
-    if (VIR_ALLOC(net) < 0)
+    if (!(net = virDomainNetDefNew(data->xmlopt)))
         return NULL;
 
     gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType);


With this squashed in:

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

Michal

    