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@xxxxxxxxx> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list