[PATCH v2 0/2] virnetdevtap.c: Disallow pre-existing TAP devices

[Laine Stump]

On 12/8/22 11:59 AM, Michal Privoznik wrote:
> v2 of:
> 
> https://listman.redhat.com/archives/libvir-list/2022-December/236197.html
> 
> diff to v1:
> - Check for existing device iff virNetDevGenerateName() returned early
>    (per Laine's suggestion).
> 
> Michal Prívozník (2):
>    virnetdev: Make virNetDevGenerateName() return 1 if no name was
>      generated
>    virnetdevtap.c: Disallow pre-existing TAP devices
> 
>   src/qemu/qemu_interface.c |  2 ++
>   src/util/virnetdev.c      |  6 ++++--
>   src/util/virnetdevtap.c   | 23 +++++++++++++++++++++--
>   src/util/virnetdevtap.h   |  2 ++
>   4 files changed, 29 insertions(+), 4 deletions(-)
> 


Reviewed-by: Laine Stump <laine at redhat.com>


(As I was reviewing, I realized I only got half of the story about the 
FreeeBSD virNetDevTapCreate(), and there is actually a 2nd pre-existing 
bug - since FreeBSD creates the tap device by creating a device and then 
renaming it to the desired name, not only is it not necessary to 
separately check for a pre-existing tap in order to avoid the 
"unreported error" that your patch fixes for tap devices on Linux, but 
*also* it means that on FreeBSD there is no way that we can use a 
pre-existing tap device when we actually want to (i.e. when managed='no' 
is set in <target>).

So instead of just telling you that the call to virNetDevExists() isn't 
needed in order to prevent silently opening an existing device when we 
don't want to allow it, I should have also said that there should be a 
check for virNetDevExists(), but it should instead be used to alter the 
logic in the case that your newly added ALLOW_EXISTING flag is set. 
Since it's fixing a totally separate problem, it should be a separate 
patch anyway though. I can send an RFC patch for that, but haven't had a 
working FreeBSD system since 1998 or so (although I did spin up a VM 
maybe 5 years ago) so I would have nothing to test it on other than 
verifying it could compile with libvirt CI.)