Re: [RFC PATCH 2/2] virt-aa-helper: Allow SR-IOV VF PCI for hostdev networks
On Tue, May 06, 2025 at 17:00:11 +0100, Tim Small via Devel wrote:
Add check for <forward type='hostdev'> networks which
were previously
neglected (as opposed to explicit PCI hostdev devices), so that they can
be granted the necessary permissions for PCI device access. The network
type lookup in-turn requires the helper to read libvirt.conf
Downstream bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993856
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
Your email domain uses DMARC so our mailing list applied
countermeasures. This means that your authorship of the patch would be
wrong. Please configure git to always add the extra 'From:' field:
https://www.libvirt.org/submitting-patches.html#git-configuration
Notably:
$ git config --global format.forceInBodyFrom true
.../apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 +++
src/security/virt-aa-helper.c | 11 +++++++++++
2 files changed, 14 insertions(+)
Also note that I'm no apparmor expert; I'm merely mentioning what I've
noticed.
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index e209a8bff7..3b3d733b5e 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -49,6 +49,9 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
@sysconfdir(a)/apparmor.d/libvirt/* r,
@sysconfdir@/apparmor.d/libvirt/libvirt-(a){UUID}* rw,
+ # allow network type lookup to check for forward type=hostdev networks
I presume this is needed to open outbound connections to the appropriate
sub-daemon; so not specific to hostdev lookup. Preferrably reword it so
that it states that the helper might be needing client connections.
+ @sysconfdir(a)/libvirt/libvirt.conf r,
+
# for backingstore -- allow access to non-hidden files in @{HOME} as well
# as storage pools
audit deny @{HOME}/.* mrwkl,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index fa69245324..7228292358 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1142,6 +1142,12 @@ get_files(vahControl * ctl)
vhu->type) != 0)
goto cleanup;
}
+ /* Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'> networks */
+ if (net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
The indentation is broken here.
As 'virDomainNetResolveActualType' opens connection and does a pretty
expensive lookup I'd suggest trying to check if given network is a
hostdev only when 'needsVfio' is still false.
Unfortunately this will need to be done for all the cases when this is
a non-hostdev ...
+ needsVfio = true;
+ }
}
for (i = 0; i < ctl->def->nmems; i++) {
@@ -1306,6 +1312,11 @@ get_files(vahControl * ctl)
if (!virDomainNetIsVirtioModel(net))
continue;
}
+ if (net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
here too
+ continue;
IIUC this shouldn't be needed. By definition hostdev-based networks
can't be 'virito' so they should have been already skipped by the above
condition.
+ }
needsvhost = true;
}
}
--
2.47.2