On 10/20/2012 02:45 AM, Laine Stump wrote:
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(-)
ACK.
@@ -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."),
How does one change which group is the default? Via back-to-back change
commands, the first which removes the default flag from the old name,
and the second adding the new name with the new default flag? I think
that works, so it doesn't stop this patch.
However, I wonder if you want a followup patch to add a flag to the
overall API that allows one to forcefully change the default to the new
element by also removing the default flag from the old group, all in one
API call. It would be needed if there is ever a reason where having a
window with no default is unacceptable, so atomically moving the default
in one API call becomes important to close such a window, but I'm having
a hard time convincing myself whether we even care about such a race.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org