On 1/15/26 08:49, Peter Krempa wrote: > On Thu, Jan 15, 2026 at 08:38:36 +0100, Michal Prívozník via Devel wrote: >> On 1/14/26 20:39, Erik Huelsmann wrote: >>> Hi Michal, > > [...] > >>>> Another alternative is to move profile generation into its separate >>>> step. So then we'd have: >>>> >>>> qemuProcessPrepareDomain(): >>>> 1) gen seclabel /* here only SELinux/DAC drivers would do something >>>> useful. For AppArmor it'd be a NOP. */ >>>> 2) generate all the paths >>>> 3) write profile /* only AppArmor would act upon, for SELinux and >>>> AppArmor it'd be nop. */ >>>> >>>> Let me see if that'd would fix the issue. >>> >>> From the fact that you submitted the patches, I think I understand the >>> change above didn't work. >> >> It did, but I'm not sure about all the implications of it. I mean, if >> there's something, hidden under a dozen of function calls from >> qemuProcessPrepareDomain() that expects seclabel to be generated, then >> moving the generation at the end is no good. > > I'm not exactly sure how the apparmour profile is used in this case, > because in qemuProcessPrepareDomain() the qemu process certainly is not > around. What *can* be around are some various helper processes that are > launched (e.g. 'numad' is invoked, some vhost-user backend binaries are > invoked (at least for capability detection but I remember one case where > some "useful" setup was done there too, at least historically)).
Arguably, that setup might be moved to host prep phase. BTW: I've checked and although we do run binaries, plain virCommandRun() is used, i.e. not qemuSecurityCommandRun() which would execute those binaries with already generated seclabel. > > So at the very least, with this move libvirt must be able to do the > things above and hopefully I didn't overlook anything. Agreed. > > That's the reason why I didn't put an R-b on those patches yet, because > at least code-wise they look good to me. Another idea I had to fix this issue is: with AppArmor libvirt generates basically two files (both under /etc/apparmor.d/libvirt/): 1) libvirt-$UUID 2) libvirt-$UUID.files where the first one basically just includes abstractions/libvirt-qemu AND libvirt-$UUID.files. The libvirt-$UUID.files then contains all those domain specific paths, like: /run/libvirt/qemu/swtpm/1-guest-swtpm.sock /var/lib/libvirt/qemu/domain-1-guest/fs0-fs.sock and so on (intentionally picked interesting paths to illustrate my point). Thing is, this list of paths is constructed in two ways: a) in qemuProcessPrepareDomain() during qemuSecurityGenLabel() from domain XML (we know this is problematic), b) in qemuProcessPrepareHost() where qemuSecurityDomainSetPathLabel() ends up executing virt-aa-helper in such fashion, that the domain XML on its input is ignored, but desired path is appended into the libvirt-$UUID.files. In the end, the libvirt-$UUID.files is a list of paths with mixed sources (some come from domain XML, others from runtime decisions). With that, it's impossible to keep the profile true. So what if we split the file? We'd have: 1) libvirt-$UUID 2) libvirt-$UUID.files 3) libvirt-$UUID.internal Now, libvirt-$UUID would just include the other two files, libvirt-$UUID.files would contain all the paths collected from domain XML and thus can be regenerated whenever domain XML changes, and finally, libvirt-$UUID.internal would then contain those specific, internal paths like /var/lib/libvirt/qemu/domain-1-guest/master-key.aes for instance. But this is much much bigger task. So let's just stick with my patches for now and if we ever run into problem, we can revisit this idea. Michal
