Re: [libvirt PATCH] qemu: add runtime config option for nbdkit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux