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

[Pavel Hrdina]

On Mon, Jan 25, 2016 at 09:00:27PM -0500, Laine Stump wrote:
> >> +    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.
> 
> Well, the man page says -1 is returned by the *printf functions "if an 
> output error is encountered", which would include an I/O error (moot for 
> snprintf() since it doesn't do any I/O) or a bad format string (but 
> we're calling it with a literal format string that only includes a 
> single %d, so it is known to be good). Checking the returned length is 
> pointless too, since by definition the longest possible result of 
> macvtap%d where the %d is guaranteed between 0 and 8192 is 
> "macvtap8192", which is 12 characters - safely below the limit of 
> IFNAMSIZ (16).

Sure, I've looked at the man page and also into gnulib code and just wanted to
point it out, that we may want to somehow handle -1, but if it happens probably
the whole libvirt is going to die with OOM.  Let's leave it as it is.