[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/7] conf: clarify what is returned for actual bandwidth and vlan

On 21.02.2014 14:58, Laine Stump wrote:
In practice, if a virDomainNetDef has a virDomainActualNetDef
allocated, the ActualNetDef will *always* contain the bandwidth and
vlan data from the NetDef (unless there was also a portgroup involved
- see networkAllocateActualDevice()).

However, virDomainNetGetActual(Bandwidth|Vlan)() were coded to make it
appear as if it might be possible to have a valid bandwidth/vlan in
the NetDef, but a NULL in the ActualNetDef. Believing this un-truth
could lead to writing unnecessarily defensive code when dealing with
the virDomainGetActual*() functions, so this patch makes it more

    If there is an ActualNetDef, it will always have a copy of the
    various appropriate bits from its parent NetDef, and the
    virDomainGetActual* function will *always* return the data from the
    ActualNetDef, not from the NetDef.

The reason for this effective-NOP patch is that a subsequent patch to
change virDomainNetDefFormat will rely on the above rule.
  src/conf/domain_conf.c | 20 ++++++++++++++------
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 064a40e..755066c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18638,8 +18638,11 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface)
  virDomainNetGetActualBandwidth(virDomainNetDefPtr iface)
+    /* if there is an ActualNetDef, *always* return
+     * its bandwidth rather than the NetDef's bandwidth.
+     */
      if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
-        iface->data.network.actual && iface->data.network.actual->bandwidth) {
+        iface->data.network.actual) {
          return iface->data.network.actual->bandwidth;
      return iface->bandwidth;
@@ -18648,12 +18651,17 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface)
  virDomainNetGetActualVlan(virDomainNetDefPtr iface)
+    virNetDevVlanPtr vlan = &iface->vlan;
+    /* if there is an ActualNetDef, *always* return
+     * its vlan rather than the NetDef's vlan.
+     */
      if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
-        iface->data.network.actual &&
-        iface->data.network.actual->vlan.nTags > 0)
-        return &iface->data.network.actual->vlan;
-    if (iface->vlan.nTags > 0)
-        return &iface->vlan;
+        iface->data.network.actual)
+        vlan = &iface->data.network.actual->vlan;
+    if (vlan->nTags > 0)
+        return vlan;
      return 0;

When you're at this, can you s/0/NULL/? We are returning a pointer after all.




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]