network: Resolve some issues around vlan copying

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:

Signed-off-by: Laine Stump <laine@redhat.com>

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f1be9543992ddd16ae88783a14e7ee830d456b78..c3cb63da1e91b2ea0264276694c752479698dee1 100644 (file)
--- 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