Hi all, Wondering if this patch was missed or if it's just pretty far down somebody's backlog. I'd appreciate a review if anyone has time for it.
Thanks much! ~Wesley On Mon, Feb 2, 2026 at 3:24 PM Wesley Hershberger <[email protected]> wrote: > > Introduce a read-only `tapfd` element for direct interfaces (macvtap), > which contains the path to the backing tapfd for that interface > (e.g. `/dev/tapXX`). > > The element is only included when the domain is being formatted for > internal consumption (VIR_DOMAIN_DEF_FORMAT_STATUS) and is not accepted > in user-provided XML (!VIR_DOMAIN_DEF_PARSE_INACTIVE). > > This is used by the AppArmor security driver when re-generating profiles. > > Partial-Resolves: #692 > Bug-Ubuntu: https://bugs.launchpad.net/bugs/2126574 > Signed-off-by: Wesley Hershberger <[email protected]> > --- > This submission is a partial revision of a previous series with a fix > for the macvtap component of gitlab#692 [1][2]. I haven't had bandwidth > to resolve the blockcommit component since the complexity there is > somewhat higher (and is also lower priority for us). > > I kept the separate `tapfd` element rather than reusing the existing > `backend` element (virDomainNetBackend.tap) to avoid making a > user-visible change [3]. I'd be happy to use the existing field instead > if you think that would make more sense. > > I opted not to introduce/modify a security driver API for FD+path as the > patch here is sufficient to resolve the bug, but would be willing to do > so if that would make this change more palatable. > > I've opened a MR to libvirt-tck with test cases that demonstrate the > bugs that this fixes [4]. apparmor/110-macvtap.t passes with this patch > applied. > > Thanks for the reviews and continued consideration. > ~Wesley > > [1] > https://lists.libvirt.org/archives/list/[email protected]/thread/UNNBQCMTOCLILQFBDG75734OCQZIXWQF/ > [2] https://gitlab.com/libvirt/libvirt/-/issues/692 > [3] > https://libvirt.org/formatdomain.html#setting-network-backend-specific-options > [4] https://gitlab.com/libvirt/libvirt-tck/-/merge_requests/73 > --- > Changes in v2: > - Drop `virt-aa-helper: Ask for no deny rule...` as it was applied > - Drop `qemu: Store blockcommit permissions...` due to unresolved concerns > - Pass tapfd path through netdef instead of resolving from fd > - Link to v1: > https://lore.kernel.org/r/[email protected] > --- > src/conf/domain_conf.c | 8 ++++++++ > src/conf/domain_conf.h | 1 + > src/hypervisor/domain_interface.c | 2 +- > src/lxc/lxc_process.c | 1 + > src/qemu/qemu_interface.c | 1 + > src/security/security_apparmor.c | 1 + > src/security/virt-aa-helper.c | 5 +++++ > src/util/virnetdevmacvlan.c | 18 +++++++++++------- > src/util/virnetdevmacvlan.h | 4 +++- > 9 files changed, 32 insertions(+), 9 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index > 02e23f78667a775637c710b651ba5fc7a127226f..1d7921e0de6f097ffaf86a9197d629e67dc213d7 > 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2939,6 +2939,7 @@ virDomainNetDefFree(virDomainNetDef *def) > g_free(def->virtio); > g_free(def->coalesce); > g_free(def->sourceDev); > + g_free(def->tapfdpath); > > virNetDevIPInfoClear(&def->guestIP); > virNetDevIPInfoClear(&def->hostIP); > @@ -10440,6 +10441,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, > return NULL; > } > > + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { > + def->tapfdpath = virXPathString("string(./tapfd/@path)", ctxt); > + } > + > if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0) > return NULL; > > @@ -25664,6 +25669,9 @@ virDomainNetDefFormat(virBuffer *buf, > if (def->mtu) > virBufferAsprintf(buf, "<mtu size='%u'/>\n", def->mtu); > > + if (def->tapfdpath && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)) > + virBufferAsprintf(buf, "<tapfd path='%s'/>\n", def->tapfdpath); > + > virDomainNetDefCoalesceFormatXML(buf, def->coalesce); > > virDomainDeviceInfoFormat(buf, &def->info, flags | > VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index > 66dc4e3417b8cb5bce60217a4e529add61149962..ba2bf1f750dcd7f4f25ef3bf55fd63629d3b5222 > 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1203,6 +1203,7 @@ struct _virDomainNetDef { > char *downscript; > char *domain_name; /* backend domain name */ > char *ifname; /* interface name on the host (<target dev='x'/>) */ > + char *tapfdpath; /* Path in /dev for macvtap (<tapfd path="/dev/tapXX">) > */ > virTristateBool managed_tap; > virNetDevIPInfo hostIP; > char *ifname_guest_actual; > diff --git a/src/hypervisor/domain_interface.c > b/src/hypervisor/domain_interface.c > index > 5bc698d2727e1142e9c5dc30ac00975f268f98e8..37e3d453a03943ee5729ad2d4b087b5e0ca37408 > 100644 > --- a/src/hypervisor/domain_interface.c > +++ b/src/hypervisor/domain_interface.c > @@ -111,7 +111,7 @@ virDomainInterfaceEthernetConnect(virDomainDef *def, > > if (virNetDevMacVLanIsMacvtap(net->ifname)) { > auditdev = net->ifname; > - if (virNetDevMacVLanTapOpen(net->ifname, tapfd, tapfdSize) < 0) > + if (virNetDevMacVLanTapOpen(net->ifname, tapfd, tapfdSize, > &net->tapfdpath) < 0) > goto cleanup; > if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, > > virDomainInterfaceIsVnetCompatModel(net)) < 0) { > diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c > index > 1bca9e8daea2cb8f63bcf5c0a735252ff57af6f1..c731b28871b18329e633c42f2141d22063208d9f > 100644 > --- a/src/lxc/lxc_process.c > +++ b/src/lxc/lxc_process.c > @@ -379,6 +379,7 @@ virLXCProcessSetupInterfaceDirect(virLXCDriver *driver, > VIR_NETDEV_VPORT_PROFILE_OP_CREATE, > cfg->stateDir, > NULL, 0, > + &net->tapfdpath, > macvlan_create_flags) < 0) > return NULL; > > diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c > index > 23a23d201aec31a36431646551ae03a233606e30..edc53d53b3b34afbfb8662e809bc0898076fdfc5 > 100644 > --- a/src/qemu/qemu_interface.c > +++ b/src/qemu/qemu_interface.c > @@ -81,6 +81,7 @@ qemuInterfaceDirectConnect(virDomainDef *def, > &res_ifname, > vmop, cfg->stateDir, > tapfd, tapfdSize, > + &net->tapfdpath, > macvlan_create_flags) < 0) > goto cleanup; > > diff --git a/src/security/security_apparmor.c > b/src/security/security_apparmor.c > index > 934acfb46198401d84d47cc6266a9403eda5a3b0..dec271721641c811944f98464cebebca6ed6a159 > 100644 > --- a/src/security/security_apparmor.c > +++ b/src/security/security_apparmor.c > @@ -157,6 +157,7 @@ load_profile(virSecurityManager *mgr G_GNUC_UNUSED, > > if (virDomainDefFormatInternal(def, NULL, &buf, > VIR_DOMAIN_DEF_FORMAT_SECURE | > + VIR_DOMAIN_DEF_FORMAT_STATUS | > VIR_DOMAIN_DEF_FORMAT_VOLUME_TRANSLATED) > < 0) > return -1; > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index > f4ec6b7826ba532f0dbac2dcd4ed89f7f98e6be6..e904d5e8292ae5e7d4acbec2062a91861a9535f5 > 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -1176,6 +1176,11 @@ get_files(vahControl * ctl) > vhu->type) != 0) > return -1; > } > + > + if (net->tapfdpath) { > + if (vah_add_file(&buf, net->tapfdpath, "rwk") != 0) > + return -1; > + } > } > > for (i = 0; i < ctl->def->nmems; i++) { > diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c > index > cde9d70eefd047dc5c16056f6697cf4d05bc0795..fcf63e08ff07eca22a122a2532cea1b0c9a95c9e > 100644 > --- a/src/util/virnetdevmacvlan.c > +++ b/src/util/virnetdevmacvlan.c > @@ -152,24 +152,24 @@ int virNetDevMacVLanDelete(const char *ifname) > int > virNetDevMacVLanTapOpen(const char *ifname, > int *tapfd, > - size_t tapfdSize) > + size_t tapfdSize, > + char **tapname) > { > int retries = 10; > int ret = -1; > int ifindex; > size_t i = 0; > - g_autofree char *tapname = NULL; > > if (virNetDevGetIndex(ifname, &ifindex) < 0) > return -1; > > - tapname = g_strdup_printf("/dev/tap%d", ifindex); > + *tapname = g_strdup_printf("/dev/tap%d", ifindex); > > for (i = 0; i < tapfdSize; i++) { > int fd = -1; > > while (fd < 0) { > - if ((fd = open(tapname, O_RDWR)) >= 0) { > + if ((fd = open(*tapname, O_RDWR)) >= 0) { > tapfd[i] = fd; > } else if (retries-- > 0) { > /* may need to wait for udev to be done */ > @@ -178,7 +178,7 @@ virNetDevMacVLanTapOpen(const char *ifname, > /* However, if haven't succeeded, quit. */ > virReportSystemError(errno, > _("cannot open macvtap tap device > %1$s"), > - tapname); > + *tapname); > goto cleanup; > } > } > @@ -188,6 +188,7 @@ virNetDevMacVLanTapOpen(const char *ifname, > > cleanup: > if (ret < 0) { > + g_free(*tapname); > while (i--) > VIR_FORCE_CLOSE(tapfd[i]); > } > @@ -659,6 +660,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char > *ifnameRequested, > char *stateDir, > int *tapfd, > size_t tapfdSize, > + char **tapfdpath, > unsigned int flags) > { > g_autofree char *ifname = NULL; > @@ -729,7 +731,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char > *ifnameRequested, > } > > if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { > - if (virNetDevMacVLanTapOpen(ifname, tapfd, tapfdSize) < 0) > + if (virNetDevMacVLanTapOpen(ifname, tapfd, tapfdSize, tapfdpath) < 0) > goto disassociate_exit; > > if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0) > @@ -888,7 +890,8 @@ int virNetDevMacVLanDelete(const char *ifname > G_GNUC_UNUSED) > int > virNetDevMacVLanTapOpen(const char *ifname G_GNUC_UNUSED, > int *tapfd G_GNUC_UNUSED, > - size_t tapfdSize G_GNUC_UNUSED) > + size_t tapfdSize G_GNUC_UNUSED, > + char **tapname G_GNUC_UNUSED) > { > virReportSystemError(ENOSYS, "%s", > _("Cannot create macvlan devices on this > platform")); > @@ -917,6 +920,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char > *ifname G_GNUC_UNUSED, > char *stateDir G_GNUC_UNUSED, > int *tapfd G_GNUC_UNUSED, > size_t tapfdSize G_GNUC_UNUSED, > + char **tapfdpath G_GNUC_UNUSED, > unsigned int unused_flags > G_GNUC_UNUSED) > { > virReportSystemError(ENOSYS, "%s", > diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h > index > 31e4804cdc0d7c4beb74ba66d204d0ff7ad83151..7424b8796529d6c6d1909eee81c88e8ded0ea84b > 100644 > --- a/src/util/virnetdevmacvlan.h > +++ b/src/util/virnetdevmacvlan.h > @@ -72,13 +72,15 @@ int virNetDevMacVLanCreateWithVPortProfile(const char > *ifname, > char *stateDir, > int *tapfd, > size_t tapfdSize, > + char **tapfdpath, > unsigned int flags) > ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6) > ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(10) G_GNUC_WARN_UNUSED_RESULT; > > int virNetDevMacVLanTapOpen(const char *ifname, > int *tapfd, > - size_t tapfdSize) > + size_t tapfdSize, > + char **tapname) > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) > G_GNUC_WARN_UNUSED_RESULT; > > > --- > base-commit: 74fc02d792f7ee55d2e0a7b9ad4e6d751c36ceb8 > change-id: 20260105-apparmor-races-d03238ee4d93 > > Best regards, > -- > Wesley Hershberger <[email protected]> >
