On Wed, Nov 08, 2023 at 03:39:22PM -0600, Jonathon Jongsma wrote: > Currently when we build with nbdkit support, libvirt will always try to > use nbdkit to access remote disk sources when it is available. But > without an up-to-date selinux policy allowing this, it will fail. > Because the required selinux policies are not yet widely available, we > have disabled nbdkit support on rpm builds for all distributions before > Fedora 40. > > Unfortunately, this makes it more difficult to test nbdkit support. > After someone updates to the necessary selinux policies, they would also > need to rebuild libvirt to enable nbdkit support. By introducing a > configure option (storage_use_nbdkit), we can build packages with nbdkit > support but have it disabled by default. > > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > --- > > Suggested as an option for making testing easier by Andrea Bolognani This is what the Suggested-by tag exists for ;) Anyway, this looks reasonable. In addition to making life easier for those testing the SELinux (and AppArmor!) policy changes during this transitional period, making it possible for the local admin to opt out of nbdkit usage sounds like it could be useful in at least some scenarios. The main concern I have is how this will be handled for upgrades. At some point we want to make nbdkit the default, right? But that would mean changing how existing installations behave. I guess that is fine in this case? Performing the switch transparently was always the plan after all... I still feel a tiny bit uneasy about that though. > +++ b/libvirt.spec.in > -# We should only enable nbdkit support if the OS ships a SELinux policy that > -# allows libvirt to launch it. Right now that's not the case anywhere, but > -# things should be fine by the time Fedora 40 is released. > -# > -# TODO: add RHEL 9 once a minor release that contains the necessary SELinux > -# bits exists (we only support the most recent minor release) > %if %{with_qemu} > - %if 0%{?fedora} >= 40 > - %define with_nbdkit 0%{!?_without_nbdkit:1} > - %endif > + %define with_nbdkit 0%{!?_without_nbdkit:1} > %endif Once the updated SELinux policy hits Fedora 40 (if it hasn't already) we're going to want nbdkit support to be enabled by default (not just available) there, no? So I think we need something along the lines of %if %{with_qemu} %define with_nbdkit 0%{!?_without_nbdkit:1} %if 0%{?fedora} >= 40 # and later RHEL 9 too %define with_nbdkit_enabled 0%{!?_without_nbdkit_enabled:1} %endif %endif > +++ b/src/qemu/qemu.conf.in > +# Using nbdkit to access remote disk sources > +# > +# If this is set then libvirt will use nbdkit to access remote disk sources > +# when available. nbdkit will export an NBD share to qemu rather than having > +# qemu attempt access the remote server directly. > +# > +# Possible values are 0 or 1. Disabled by default. > +# > +# storage_use_nbdkit = 1 ... and then to reflect the actual compile-time default here. There's precedent for that, see @QEMU_USER@ and @QEMU_GROUP@. Also s/qemu/QEMU/g in the comment :) > +++ b/src/qemu/qemu_conf.h > @@ -230,6 +230,8 @@ struct _virQEMUDriverConfig { > + bool storageUseNbdkit; This works fine as long as the default is false, but in order to make it possible to change it via a compile-time knob virQEMUDriverConfigNew() will need to be updated too. -- Andrea Bolognani / Red Hat / Virtualization _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx