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

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(a)laine.org&gt; 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 >