Re: [libvirt PATCH v2 0/2] Eliminate old tap/macvtap teardown stomping on new tap setup

V1 is here: https://www.redhat.com/archives/libvir-list/2020-August/msg00756.html The problem and this solution are very well described in patches 2 and 3, but in short - because we (libvirt for macvtap, the kernel for tap) always try to assign the lowest numbered names possible to macvtap and tap devices, we sometimes create a new tap for a new guest using the same name as an old tap for an old guest that is shutting down simultaneous to setting up the new guest/tap. This can lead to the old guest teardown stomping on the new guest setup. This is the problem that the authors were attempting to solve in these two patches sent earlier in the summer: https://www.redhat.com/archives/libvir-list/2020-June/msg00481.html https://www.redhat.com/archives/libvir-list/2020-June/msg00525.html and also in this V2 patch, which Bingsong Si sent in response to my poorly-thought-out advice in my response to his original patch: https://www.redhat.com/archives/libvir-list/2020-June/msg00755.html Somewhere during that discussion, danpb suggested that in order to *really* solve the problem, we should use our own counter for auto-generated tap device names (instead of relying on the kernel) and just never re-use a name until the counter rolls over. That's essentially what these two patches do. One possibly undesirable side effect of this (and the other) patch is that the longer a host is running without reboot, the higher the numbers tap device names will get. While users are accustomed to always seeing vnet0 and vnet1, they may be a bit surprised to now see vnet39283 or macvtap735. It has been pointed out to me (again by danpb) that the same thing happened with PIDs a few years ago, and while it looked strange at first, everyone is now accustomed to it. Changes from V1: Patch 1 from V1 was removed - everything it changed is now removed/replaced in the new Patch 1 (which was Patch 2 in V1). And so of course, what was Patch 3 in V1 is now Patch 2 in V2. I eliminated the old bitmap reservation system in the macvtap patch (Patch 1) rather than adding on top of it as I had in V1 - it really was beyond redundant and unnecessary, and just clouded up the whole situation. This also allowed me to get rid of the 8192 limit (which was there only to limit the size of the virBitmap, which we no longer need), and allow the device names to count up until they overflow either the ifname[IFNAMSIZ] buffer, or reach INT_MAX. Likewise, I modified the standard tap patch to remove the artificial maximum of 99999, and just let it count up until it overflows. (Jano suggested that I should have a test case to test the entire range, but I don't think anyone would be happy with that. If I was masochistic and wanted to mock a bunch of virNetDev functions I could artificially test it by bumping up the counters with calls to the virNetDevTapReserveName() function, but it's 1AM. I did test the rollover of both cases (macvtap, where it overflows the buffer size first, and standard tap where it overflow the 32 bit int first) with a one-off build that started the counter just a few below the overflow point, and it does work correctly.) Laine Stump (2): util: replace macvtap name reservation bitmap with a simple counter util: assign tap device names using a monotonically increasing integer src/libvirt_private.syms | 2 +- src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_process.c | 22 +- src/util/virnetdevmacvlan.c | 402 +++++++++++++----------------------- src/util/virnetdevmacvlan.h | 6 +- src/util/virnetdevtap.c | 108 +++++++++- src/util/virnetdevtap.h | 4 + 8 files changed, 275 insertions(+), 273 deletions(-)