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

Re: [libvirt] [PATCH] Account for defined networks when generating bridge names



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;
> +}


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