On Mon, Jan 05, 2026 at 14:27:22 -0600, Wesley Hershberger via Devel wrote:
> Introduce a read-only `tapfd` element for interfaces of type network,
> bridge, direct, and ethernet, which contains the path in `/dev` to the
> backing tapfd for that interface. 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]>
> ---
> src/conf/domain_conf.c | 10 ++++++++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 9 +++++++++
> src/security/security_apparmor.c | 1 +
> src/security/virt-aa-helper.c | 5 +++++
> 5 files changed, 26 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 541dad5bdc..bfeed3dc96 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2936,6 +2936,9 @@ virDomainNetDefFree(virDomainNetDef *def)
> g_free(def->virtio);
> g_free(def->coalesce);
> g_free(def->sourceDev);
> + if (def->tapfdpath) {
> + g_free(def->tapfdpath);
> + }
'g_free' is a no-op if argument is NULL so no check is needed similarly
to all other calls above.
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 98229d7cf9..96352821d7 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8880,9 +8880,18 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
>
> for (i = 0; i < tapfdSize; i++) {
> g_autofree char *name = g_strdup_printf("tapfd-%s%zu",
> net->info.alias, i);
> + g_autofree char *procpath = NULL;
> int fd = tapfd[i]; /* we want to keep the array intact for
> security labeling*/
>
> netpriv->tapfds = g_slist_prepend(netpriv->tapfds,
> qemuFDPassDirectNew(name, &fd));
> +
> + /* Include tapfd path in the domstatus XML */
> + procpath = g_strdup_printf("/proc/self/fd/%d", tapfd[i]);
So this resolves to /proc/PID_OF_VIRTQEMUD_PROCESS/fd/NNN. virtqemud
will close the FD as soon as we pass it to qemu and additionally
virtqemud might restart where the path stored in the status XML (which
is mainly used to re-load after restart) would point to the old PID.
Thus this makes no sense to be stored in status XML.
You might want to store the path once it's passed to qemu inside the
qemu processe's fd list, but I'd argue that qemu ought to have access
to everything in there anyways since libvirt actively passed it there.
> +
> + if (virFileResolveLink(procpath, &net->tapfdpath) < 0) {
> + /* it's a deleted file, presumably. Ignore? */
'deleted file' ? These werent files originally even when they come from
e.g. opening /dev/net/tun.
> + VIR_WARN("could not find path for descriptor %s, skipping",
> procpath);
Please avoid VIR_WARN.
> + }
> }
>
> netpriv->tapfds = g_slist_reverse(netpriv->tapfds);
So with other security drivers the one-time setup of seclabels passed to
qemu is done in qemuSecuritySetTapFDLabel() in this function. Other
dirvers can do it one-time because it applies on the FDs passed to qemu
and doesn't need to be changed.
If apparmor requires to be able to reference the FD paths at any later
time it will need to refer to the FDs under the qemu pid (which is not
know at this point since the qemu process did not start) rather than the
pid of the process the qemu driver runs in because that one can re-start
and also will necessarily close the FDs once qemu starts.
