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@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.)