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

Re: [libvirt] [PATCH] network: truncate bridges' dummy tap device names to IFNAMSIZ (15) chars



On 04/13/2011 03:24 PM, Eric Blake wrote:
On 04/13/2011 11:55 AM, Laine Stump wrote:
The solution is to shorten the part of the original name used to
generate the tap device name. However, simply truncating it is
insufficient, because the last few characters of an interface name are
often a number used to indicate one of a list of several similar
devices (for example, "verylongname123", "verylongname124", etc) and
simple truncation would lead to duplicate names. So instead we take
the first 8 characters of $bridgename ("verylong" in the example), add
on the final 3 bytes ("123"), then add "-nic" (so "verylong123-nic").
Not pretty, but it is much more likely to generate a unique name, and
is reproducible (unlike, say, a random number).
Should we also minimize the truncation by adding just "-n" instead of
"-nic", so that there are fewer user strings being butchered?  Or would
that cause problems for existing users that already have bridge-nic and
would now also have bridge-n?

Well, for someone who upgraded with a network that was still up, then restarted the network without rebooting, it would leave around an extra bridge-nic tap interface with the same MAC address as the new bridge-n interface. But that interface would be down (which also means it wouldn't show up in the ifconfig output unless you added "-a"), not connected to anything, and would disappear completely the next time the system was rebooted. (Also, I just checked to verify that an extra tap device with matching MAC address doesn't mess with proper networking of the guests, and it doesn't.)

So, is this minor artifact worth getting the extra two characters? I'll let the patch sit until midday, and if someone comes out in support of changing it, I'll reduce the suffix to "-n" and push, otherwise I'll push as-is.

-    virAsprintf(&nicname, "%s-nic", brname);
+    if (strlen(brname)>  (IFNAMSIZ - 5)) {
+        /* because the length of an ifname is limited to IFNAMSIZ-1
+         * (usually 15), and we're adding 4 more characters, we must
+         * truncate the original name to 11 to fit. In order to catch
+         * a possible numeric ending (eg virbr0, virbr1, etc), we grab
+         * the first 8 and last 3 characters of the string.
+         */
+         virAsprintf(&nicname, "%.*s%s-nic",
+                     IFNAMSIZ - 8, /* space for last 3 chars + "-nic" + NULL */
+                     brname, brname + strlen(brname) - 3);
+    } else {
+        virAsprintf(&nicname, "%s-nic", brname);
+    }
ACK.



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