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

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.