[PATCH] network: Resolve some issues around vlan copying

[Laine Stump <laine@xxxxxxxxx>]

From: John Ferlan <jferlan@xxxxxxxxxx>

Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
Prior code would already check if netdef was NULL.

Coverity complained about a path where the 'vlan' was potentially valid,
but a prior checks may not have allocated 'iface->data.network.actual',
so like other paths it needs to be allocated on the fly.

Move the copying of vlan up earlier in networkAllocateActualDevice, so
that actual.type gets properly set.

Since the first assignment to vlan is redundant except in the case of
jumping immediately to validate from the start of the function,
eliminate its initial setting at the top of the function in favor of
calling the helper function virDomainNetGetActualVlan() (which doesn't
depend on the local vlan pointer being initialized) down at validate:

---

Difference's between John's V1 and this V2 are described in paragraphs
2 and 3 above. I moved the lines that John changed, but left his
changes intact.

 src/network/bridge_driver.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f1be954..c3cb63d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2,7 +2,7 @@
 /*
  * bridge_driver.c: core driver methods for managing network
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -3715,10 +3715,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
     int ii;
     int ret = -1;
 
-    /* it's handy to have this initialized if we skip directly to validate */
-    if (iface->vlan.nTags > 0)
-        vlan = &iface->vlan;
-
     if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
         goto validate;
 
@@ -3764,6 +3760,24 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
             goto error;
     }
 
+    /* copy appropriate vlan info to actualNet */
+    if (iface->vlan.nTags > 0)
+        vlan = &iface->vlan;
+    else if (portgroup && portgroup->vlan.nTags > 0)
+        vlan = &portgroup->vlan;
+    else if (netdef->vlan.nTags > 0)
+        vlan = &netdef->vlan;
+
+    if (vlan) {
+        if (!iface->data.network.actual
+            && (VIR_ALLOC(iface->data.network.actual) < 0)) {
+            virReportOOMError();
+            goto error;
+        }
+        if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0)
+           goto error;
+    }
+
     if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) ||
         (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) ||
         (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE)) {
@@ -4000,24 +4014,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
     if (virNetDevVPortProfileCheckComplete(virtport, true) < 0)
         goto error;
 
-    /* copy appropriate vlan info to actualNet */
-    if (iface->vlan.nTags > 0)
-        vlan = &iface->vlan;
-    else if (portgroup && portgroup->vlan.nTags > 0)
-        vlan = &portgroup->vlan;
-    else if (netdef && netdef->vlan.nTags > 0)
-        vlan = &netdef->vlan;
-
-    if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0)
-        goto error;
-
 validate:
     /* make sure that everything now specified for the device is
      * actually supported on this type of network. NB: network,
      * netdev, and iface->data.network.actual may all be NULL.
      */
 
-    if (vlan) {
+    if (virDomainNetGetActualVlan(iface)) {
         /* vlan configuration via libvirt is only supported for
          * PCI Passthrough SR-IOV devices and openvswitch bridges.
          * otherwise log an error and fail
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list