Hi, Cedric Bosdonnat: > On Tue, 2017-12-12 at 15:01 +0100, intrigeri wrote: >> Cédric Bosdonnat: >> > This commit helps users allowing access to their images by adding their >> > own rules in apparmor.d/local/usr.lib.libvirt.virt-aa-helper. >> > […] >> > profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-helper { >> > #include <abstractions/base> >> > + #include <local/usr.lib.libvirt.virt-aa-helper> >> >> The packaging helper we use in Debian adds exactly the same line at >> the *end* of the profile (and actually, at the end of almost every >> AppArmor profile included in Debian and derivatives); I don't know why >> it's added at the end and not at the beginning. I suspect Jamie will >> know better. >> >> If there's no strong reason to add this line in the beginning of the >> profile, I suggest we add it at the end instead, so we avoid changing >> behaviour subtly once this gets merged upstream and we drop the >> Debian-specific line. >> >> Other than this, ACK from me on the proposed profile modifications. > I'm perfectly fine in having that include at the end of the profile. I'll > push with that change. Thanks! This will allow us to remove half of apparmor_profiles_local_include.patch we carry in Debian. Now, two follow-up questions: 1. Doing the same for usr.sbin.libvirtd? The apparmor_profiles_local_include.patch Debian patch does the same for usr.sbin.libvirtd: diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index fa4ebb3..5505bf6 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -90,4 +90,7 @@ /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix, } + + # Site-specific additions and overrides. See local/README for details. + #include <local/usr.sbin.libvirtd> } Cédric and others, what about upstreaming this as well? 2. Impact on packaging for distros that were already managing this local file themselves & differently … i.e. I guess mostly Debian and derivatives, that use dh_apparmor. If I got the build system changes right, local/usr.lib.libvirt.virt-aa-helper will be created at installation time so in practice it'll be shipped by distro packages. I *think* it's not a problem with dh_apparmor because the generated postinst scripts only manage that file if it does not exist yet (so it should be a no-op if dpkg has already installed it), and similarly the code for purging in postrm should work just fine if dpkg has already deleted it. But local/usr.lib.libvirt.virt-aa-helper becomes a conffile, which previously it was not managed by dpkg. I don't know how this is handled by dpkg. I suspect it might be easier to comment out: INSTALL_DATA_LOCAL += install-apparmor-local UNINSTALL_LOCAL += uninstall-apparmor-local … and keep the current behaviour, for consistency with how all other local/* override files are handled in Debian/Ubuntu. Guido, Ubuntu packagers, what do you think? Cheers, -- intrigeri -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list