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

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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