[PATCH v2 0/2] virnetdevmacvlan: Wait for udev to settle after creating macvtap
This is a v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/YRT32... diff to v1: - Dropped all virWaitForDevices() but the one in virNetDevMacVLanCreate() which is also made conditional Michal Prívozník (2): virnetdevmacvlan: Wait for udev to settle after creating macvtap virnetdevmacvlan: Drop udev busy loop from virNetDevMacVLanTapOpen() src/util/virnetdevmacvlan.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) -- 2.52.0
From: Michal Privoznik <mprivozn@redhat.com> When a macvtap interface is created (e.g. during domain startup or on device hotplug) libvirt then open corresponding /dev/tapNN in order to pass FDs to the hypervisor. These FDs are labelled before passing, but if creating the interface and open() happen in quick succession, i.e. when udev did not had chance to run, then the /dev/tapNN node might have default SELinux label (device_t) instead of correct one (tun_tap_device_t). This then leads to AVC messages, like the following: type=AVC msg=audit(1774535384.365:1238): avc: denied { open } for pid=6765 comm="rpc-virtqemud" path="/dev/tap33" dev="devtmpfs" ino=805 scontext=system_u:system_r:virtqemud_t:s0 tcontext=system_u:object_r:device_t:s0 tclass=chr_file permissive=1 Therefore, allow udev to settle down after macvtap is created (by calling virWaitForDevices()). Resolves: https://gitlab.com/libvirt/libvirt/-/work_items/866 Tested-by: Johannes Segitz <jsegitz@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index cde9d70eef..436f8479a9 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -119,6 +119,11 @@ virNetDevMacVLanCreate(const char *ifname, return -1; } + if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { + /* Allow udev to process newly created macvtap. */ + virWaitForDevices(); + } + VIR_INFO("created device: '%s'", ifname); return 0; } -- 2.52.0
On 4/10/26 9:39 AM, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
When a macvtap interface is created (e.g. during domain startup or on device hotplug) libvirt then open corresponding /dev/tapNN in order to pass FDs to the hypervisor. These FDs are labelled before passing, but if creating the interface and open() happen in quick succession, i.e. when udev did not had chance to run, then the /dev/tapNN node might have default SELinux label (device_t) instead of correct one (tun_tap_device_t). This then leads to AVC messages, like the following:
type=AVC msg=audit(1774535384.365:1238): avc: denied { open } for pid=6765 comm="rpc-virtqemud" path="/dev/tap33" dev="devtmpfs" ino=805 scontext=system_u:system_r:virtqemud_t:s0 tcontext=system_u:object_r:device_t:s0 tclass=chr_file permissive=1
Therefore, allow udev to settle down after macvtap is created (by calling virWaitForDevices()).
Resolves: https://gitlab.com/libvirt/libvirt/-/work_items/866 Tested-by: Johannes Segitz <jsegitz@suse.de> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com> (sorry for the delay, I was distracted by "other events" and hadn't paid attention to mail :-/)
--- src/util/virnetdevmacvlan.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index cde9d70eef..436f8479a9 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -119,6 +119,11 @@ virNetDevMacVLanCreate(const char *ifname, return -1; }
+ if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { + /* Allow udev to process newly created macvtap. */ + virWaitForDevices(); + } + VIR_INFO("created device: '%s'", ifname); return 0; }
From: Michal Privoznik <mprivozn@redhat.com> Now that after previous commit the wait for udev to settle down is done right after device creation, there's no need to have additional wait in virNetDevMacVLanTapOpen(). It's effectively a dead code. Remove it. Tested-by: Johannes Segitz <jsegitz@suse.de> Reviewed-by: Laine Stump <laine@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnetdevmacvlan.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 436f8479a9..e6f4e1ab0e 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -159,7 +159,6 @@ virNetDevMacVLanTapOpen(const char *ifname, int *tapfd, size_t tapfdSize) { - int retries = 10; int ret = -1; int ifindex; size_t i = 0; @@ -173,20 +172,13 @@ virNetDevMacVLanTapOpen(const char *ifname, for (i = 0; i < tapfdSize; i++) { int fd = -1; - while (fd < 0) { - if ((fd = open(tapname, O_RDWR)) >= 0) { - tapfd[i] = fd; - } else if (retries-- > 0) { - /* may need to wait for udev to be done */ - g_usleep(20000); - } else { - /* However, if haven't succeeded, quit. */ - virReportSystemError(errno, - _("cannot open macvtap tap device %1$s"), - tapname); - goto cleanup; - } + if ((fd = open(tapname, O_RDWR)) < 0) { + virReportSystemError(errno, + _("cannot open macvtap tap device %1$s"), + tapname); + goto cleanup; } + tapfd[i] = fd; } ret = 0; -- 2.52.0
participants (2)
-
Laine Stump -
Michal Privoznik