[libvirt PATCH 0/3] Eliminate old tap/macvtap teardown stomping on new tap setup

Laine Stump posted 3 patches 3 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200824042316.821783-1-laine@redhat.com
There is a newer version of this series
src/libvirt_private.syms    |   1 +
src/qemu/qemu_process.c     |  22 +++++++-
src/util/virnetdevmacvlan.c | 109 +++++++++++++++++++++++++++---------
src/util/virnetdevtap.c     |  79 +++++++++++++++++++++++++-
src/util/virnetdevtap.h     |   4 ++
5 files changed, 186 insertions(+), 29 deletions(-)
[libvirt PATCH 0/3] Eliminate old tap/macvtap teardown stomping on new tap setup
Posted by Laine Stump 3 years, 8 months ago
The problem and 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.

These patches eliminate that problem by changing the strategy to do
our best to *not* reuse tap / macvtap device names, but instead use a
monotonically incrementing counter to name the devices.

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 that the same
thing happened with PIDs a few years ago, and while it looked strange
at first, everyone is now accustomed to it.

Laine Stump (3):
  util: make locking versions of virNetDevMacVLan(Reserve|Release)Name()
  util: assign macvtap names using a monotonically increasing integer
  util: assign tap device names using a monotonically increasing integer

 src/libvirt_private.syms    |   1 +
 src/qemu/qemu_process.c     |  22 +++++++-
 src/util/virnetdevmacvlan.c | 109 +++++++++++++++++++++++++++---------
 src/util/virnetdevtap.c     |  79 +++++++++++++++++++++++++-
 src/util/virnetdevtap.h     |   4 ++
 5 files changed, 186 insertions(+), 29 deletions(-)

-- 
2.26.2

Re: [libvirt PATCH 0/3] Eliminate old tap/macvtap teardown stomping on new tap setup
Posted by Michal Privoznik 3 years, 8 months ago
On 8/24/20 6:23 AM, Laine Stump wrote:
> The problem and 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.
> 
> These patches eliminate that problem by changing the strategy to do
> our best to *not* reuse tap / macvtap device names, but instead use a
> monotonically incrementing counter to name the devices.
> 
> 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 that the same
> thing happened with PIDs a few years ago, and while it looked strange
> at first, everyone is now accustomed to it.
> 
> Laine Stump (3):
>    util: make locking versions of virNetDevMacVLan(Reserve|Release)Name()
>    util: assign macvtap names using a monotonically increasing integer
>    util: assign tap device names using a monotonically increasing integer
> 
>   src/libvirt_private.syms    |   1 +
>   src/qemu/qemu_process.c     |  22 +++++++-
>   src/util/virnetdevmacvlan.c | 109 +++++++++++++++++++++++++++---------
>   src/util/virnetdevtap.c     |  79 +++++++++++++++++++++++++-
>   src/util/virnetdevtap.h     |   4 ++
>   5 files changed, 186 insertions(+), 29 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal