[libvirt] [PATCH] Account for defined networks when generating bridge names
Jim Meyering
jim at meyering.net
Mon Mar 2 14:41:42 UTC 2009
Cole Robinson wrote:
...
> Okay, I rolled these changes and the BridgeInUse changes into one patch
> (along with Jim's suggestions).
>
> I added a helper function SetBridgeName which basically does:
>
> if user passed bridge name:
> verify it doesn't collide
> else:
> generate bridge name
>
> We call this in Define and CreateXML. When loading configs from a driver
> restart, we only attempt to generate a bridge: if checking for
> collisions failed, the network wouldn't even be defined, which would
> only cause more pain for users.
...
Hi Cole,
Here's a quick review:
One nit below, but what I'd really like is a stand-alone virsh-oriented
way to exercise a few of these new code paths (tests), just to make sure
we get coverage when running "make check". If you can outline something
like that, I'll massage it into a new test.
> Generate network bridge names if none passed at define/create time.
...
> diff --git a/src/network_conf.c b/src/network_conf.c
> index 6ad0d01..5de164e 100644
> --- a/src/network_conf.c
> +++ b/src/network_conf.c
> @@ -43,6 +43,7 @@
> #include "buf.h"
> #include "c-ctype.h"
>
> +#define MAX_BRIDGE_ID 256
...
> +char *virNetworkAllocateBridge(virConnectPtr conn,
> + const virNetworkObjListPtr nets)
> +{
> +
> + int id = 0;
> + char *newname;
> +
> + do {
> + char try[50];
> +
> + snprintf(try, sizeof(try), "virbr%d", id);
> +
> + if (!virNetworkBridgeInUse(nets, try, NULL)) {
> + if (!(newname = strdup(try))) {
> + virReportOOMError(conn);
> + return NULL;
> + }
> + return newname;
> + }
> +
> + id++;
> + } while (id < MAX_BRIDGE_ID);
This should probably be <= MAX_BRIDGE_ID,
or change MAX_BRIDGE_ID to 255, ..
so that the diagnostic below makes sense.
> + virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Bridge generation exceeded max id %d"),
> + MAX_BRIDGE_ID);
> + return NULL;
> +}
More information about the libvir-list
mailing list