[libvirt] [PATCH 2/2] network: check for bridge name conflict with existing devices

[Shivaprasad bhat]

Thanks for the patches Laine. I agree pretty much with both the patches.
also had a chance to try these out.

Only scenario I see a trouble is,

net-create without bridge name in the xml, followed by net-define
using the same xml file.
The live and --inactive xmls both show different bridge names, though
the active bridge
can be reused by the network for "this scenario alone". I think this
is a very rare case and not worth
to fix it, as net-destroy followed by net-create using the same xml
file would anyway reuse the
same old bridge name if available.

Outside of this patch,
the net->newDef->bridge's are not considered in virNetworkBridgeInUseHelper().
Do we need to?

Thanks,
Shiva

On Fri, Apr 24, 2015 at 12:33 AM, Laine Stump <laine at laine.org> wrote:
> Since some people use the same naming convention as libvirt for bridge
> devices they create outside the context of libvirt, it is much nicer
> if we check for those devices when looking for a bridge device name to
> auto-assign to a new network.
> ---
>  src/network/bridge_driver.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index abacae3..3b879cd 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2037,11 +2037,13 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
>
>      /* Create and configure the bridge device */
>      if (!network->def->bridge) {
> -        /* this can only happen if the config files were edited
> -         * directly. Otherwise networkValidate() (called after parsing
> -         * the XML from networkCreateXML() and networkDefine())
> -         * guarantees we will have a valid bridge name before this
> -         * point.
> +        /* bridge name can only be empty if the config files were
> +         * edited directly. Otherwise networkValidate() (called after
> +         * parsing the XML from networkCreateXML() and
> +         * networkDefine()) guarantees we will have a valid bridge
> +         * name before this point. Since hand editing of the config
> +         * files is explicitly prohibited we can, with clear
> +         * conscience, log an error and fail at this point.
>           */
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("network '%s' has no bridge name defined"),
> @@ -2762,8 +2764,9 @@ static int networkIsPersistent(virNetworkPtr net)
>
>  /*
>   * networkFindUnusedBridgeName() - try to find a bridge name that is
> - * unused by the currently configured libvirt networks, and set this
> - * network's name to that new name.
> + * unused by the currently configured libvirt networks, as well as by
> + * the host system itself (possibly created by someone/something other
> + * than libvirt). Set this network's name to that new name.
>   */
>  static int
>  networkFindUnusedBridgeName(virNetworkObjListPtr nets,
> @@ -2777,7 +2780,13 @@ networkFindUnusedBridgeName(virNetworkObjListPtr nets,
>      do {
>          if (virAsprintf(&newname, templ, id) < 0)
>              goto cleanup;
> -        if (!virNetworkBridgeInUse(nets, newname, def->name)) {
> +        /* check if this name is used in another libvirt network or
> +         * there is an existing device with that name. ignore errors
> +         * from virNetDevExists(), just in case it isn't implemented
> +         * on this platform (probably impossible).
> +         */
> +        if (!(virNetworkBridgeInUse(nets, newname, def->name) ||
> +              virNetDevExists(newname) == 1)) {
>              VIR_FREE(def->bridge); /*could contain template */
>              def->bridge = newname;
>              ret = 0;
> --
> 2.1.0
>