[libvirt] [PATCHv2] util: keep/use a bitmap of in-use macvtap devices

[Pavel Hrdina]

On Fri, Jan 22, 2016 at 12:52:27PM -0500, Laine Stump wrote:
> This patch creates two bitmaps, one for macvlan devicenames and one

s/devicenames/device names/

[...]

> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c

[...]

> +/**
> + * virNetDevMacVLanReserveName:
> + *
> + *  @name: already-known name of device
> + *  @quietFail: don't log an error if this name is already in-use
> + *
> + *  Extract the device type and id from a macvtap/macvlan device name
> + *  and mark the appropriate position as in-use in the appropriate
> + *  bitmap.
> + *
> + *  returns 0 on success, -1 on failure, -2 if the name doesn't fit the auto-pattern

Long line, would be nice to wrap it.  And the return 0 isn't true, because
virNetDevMacVLanReserveID() returns ID on success.

> + */
> +int
> +virNetDevMacVLanReserveName(const char *name, bool quietFail)
> +{
> +    unsigned int id;
> +    unsigned int flags = 0;
> +    const char *idstr = NULL;
> +
> +    if (virNetDevMacVLanInitialize() < 0)
> +       return -1;
> +
> +    if (STRPREFIX(name, MACVTAP_NAME_PREFIX)) {
> +        idstr = name + strlen(MACVTAP_NAME_PREFIX);
> +        flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
> +    } else if (STRPREFIX(name, MACVLAN_NAME_PREFIX)) {
> +        idstr = name + strlen(MACVLAN_NAME_PREFIX);
> +    } else {
> +        return -2;
> +    }
> +
> +    if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("couldn't get id value from macvtap device name %s"),
> +                       name);
> +        return -1;
> +    }
> +    return virNetDevMacVLanReserveID(id, flags, quietFail, false);
> +}

[...]

>      const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>          "macvtap" : "macvlan";
> -    const char *prefix = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
> -        MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;

Maybe use the MACV(TAP|LAN)_NAME_PREFIX instead of the strings for *type.

>      const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>          MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
> -    int c, rc;
> +    int rc, reservedID = -1;
>      char ifname[IFNAMSIZ];
>      int retries, do_retry = 0;
>      uint32_t macvtapMode;
> -    const char *cr_ifname = NULL;
> +    const char *ifnameCreated = NULL;
>      int ret;
>      int vf = -1;
>      bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR;

[...]

> +    retries = MACVLAN_MAX_ID + 1;
> +    while (!ifnameCreated && retries) {
>          virMutexLock(&virNetDevMacVLanCreateMutex);
> -        for (c = 0; c < 8192; c++) {
> -            snprintf(ifname, sizeof(ifname), pattern, c);
> -            if ((ret = virNetDevExists(ifname)) < 0) {
> -                virMutexUnlock(&virNetDevMacVLanCreateMutex);
> +        reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
> +        if (reservedID < 0) {
> +            virMutexUnlock(&virNetDevMacVLanCreateMutex);
> +            return -1;
> +        }
> +        snprintf(ifname, sizeof(ifname), pattern, reservedID);

Since you're changing this, snprintf returns -1 in case of error.

ACK with the issues fixed.