On 10/20/2012 12:49 PM, Eric Blake wrote: > 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.
Good point, and a good proposal for how to close it. I'm also not sure if that will ever be important (to date I've only found two people who actually even *use* portgroups so far (one of them just yesterday), but we should probably do that just to avoid future surprises. I'm pushing this patch as-is, though, and adding your suggestion to my to-do list. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
