[libvirt] [PATCH] network: don't allow multiple default portgroups

Laine Stump laine at laine.org
Sat Oct 20 08:45:36 UTC 2012


This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868483

virNetworkUpdate, virNetworkDefine, and virNetworkCreate all three
allow network definitions to contain multiple <portgroup> elements
with default='yes'. Only a single default portgroup should be allowed
for each network.

This patch updates networkValidate() (called by both
virNetworkCreate() and virNetworkDefine()) and
virNetworkDefUpdatePortGroup (called by virNetworkUpdate() to not
allow multiple default portgroups.
---
 src/conf/network_conf.c     | 35 ++++++++++++++++++++++++++---------
 src/network/bridge_driver.c | 12 ++++++++++++
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 2f9ad2e..8976f2a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2752,7 +2752,8 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def,
                              /* virNetworkUpdateFlags */
                              unsigned int fflags ATTRIBUTE_UNUSED)
 {
-    int ii, ret = -1;
+    int ii, foundName = -1, foundDefault = -1;
+    int ret = -1;
     virPortGroupDef portgroup;
 
     memset(&portgroup, 0, sizeof(portgroup));
@@ -2766,9 +2767,11 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def,
     /* check if a portgroup with same name already exists */
     for (ii = 0; ii < def->nPortGroups; ii++) {
         if (STREQ(portgroup.name, def->portGroups[ii].name))
-            break;
+            foundName = ii;
+        if (def->portGroups[ii].isDefault)
+            foundDefault = ii;
     }
-    if (ii == def->nPortGroups &&
+    if (foundName == -1 &&
         ((command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) ||
          (command == VIR_NETWORK_UPDATE_COMMAND_DELETE))) {
         virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2776,7 +2779,7 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def,
                          "in network '%s' matching <portgroup name='%s'>"),
                        def->name, portgroup.name);
         goto cleanup;
-    } else if (ii < def->nPortGroups &&
+    } else if (foundName >= 0 &&
                ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
                 (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST))) {
         virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2787,11 +2790,25 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def,
         goto cleanup;
     }
 
+    /* if there is already a different default, we can't make this
+     * one the default.
+     */
+    if (command != VIR_NETWORK_UPDATE_COMMAND_DELETE &&
+        portgroup.isDefault &&
+        foundDefault >= 0 && foundDefault != foundName) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("a different portgroup entry in "
+                         "network '%s' is already set as the default. "
+                         "Only one default is allowed."),
+                       def->name);
+        goto cleanup;
+    }
+
     if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
 
         /* replace existing entry */
-        virPortGroupDefClear(&def->portGroups[ii]);
-        def->portGroups[ii] = portgroup;
+        virPortGroupDefClear(&def->portGroups[foundName]);
+        def->portGroups[foundName] = portgroup;
         memset(&portgroup, 0, sizeof(portgroup));
 
     } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
@@ -2816,9 +2833,9 @@ virNetworkDefUpdatePortGroup(virNetworkDefPtr def,
     } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
 
         /* remove it */
-        virPortGroupDefClear(&def->portGroups[ii]);
-        memmove(def->portGroups + ii, def->portGroups + ii + 1,
-                sizeof(*def->portGroups) * (def->nPortGroups - ii - 1));
+        virPortGroupDefClear(&def->portGroups[foundName]);
+        memmove(def->portGroups + foundName, def->portGroups + foundName + 1,
+                sizeof(*def->portGroups) * (def->nPortGroups - foundName - 1));
         def->nPortGroups--;
         ignore_value(VIR_REALLOC_N(def->portGroups, def->nPortGroups));
     } else {
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1c97f29..8837843 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2618,6 +2618,7 @@ networkValidate(virNetworkDefPtr def)
 {
     int ii;
     bool vlanUsed, vlanAllowed;
+    const char *defaultPortGroup = NULL;
 
     /* The only type of networks that currently support transparent
      * vlan configuration are those using hostdev sr-iov devices from
@@ -2638,6 +2639,17 @@ networkValidate(virNetworkDefPtr def)
              == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)) {
             vlanAllowed = true;
         }
+        if (def->portGroups[ii].isDefault) {
+            if (defaultPortGroup) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("network '%s' has multiple default "
+                                 "<portgroup> elements (%s and %s), "
+                                 "but only one default is allowed"),
+                               def->name, defaultPortGroup,
+                               def->portGroups[ii].name);
+            }
+            defaultPortGroup = def->portGroups[ii].name;
+        }
     }
     if (vlanUsed && !vlanAllowed) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
1.7.11.7




More information about the libvir-list mailing list