Re: [libvirt] [PATCH] network: Resolve some issues around vlan copying

2013-01-17 Thread John Ferlan
On 01/16/2013 12:55 PM, Laine Stump wrote:
 From: John Ferlan jfer...@redhat.com
 
 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(-)
 

ACK

The change removes the Coverity warning.

John

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


Re: [libvirt] [PATCH] network: Resolve some issues around vlan copying

2013-01-17 Thread Laine Stump
On 01/17/2013 11:05 AM, John Ferlan wrote:
 On 01/16/2013 12:55 PM, Laine Stump wrote:
 From: John Ferlan jfer...@redhat.com

 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(-)

 ACK

 The change removes the Coverity warning.

Pushed. Thanks!

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


[libvirt] [PATCH] network: Resolve some issues around vlan copying

2013-01-16 Thread Laine Stump
From: John Ferlan jfer...@redhat.com

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list