9:27 a.m.
On Wed, Nov 15, 2023 at 01:52:48PM -0600, Jonathon Jongsma wrote:
> 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.
Yes, I feel the same tiny bit of unease. I think it will be OK though.
For upgrades, we have to consider three basic cases:
1. user doesn't make any configuration changes
- by default nbdkit will not be used. When the default changes in the
future, nbdkit will start being used after upgrade.
I assume that you've already verified that the switch from not using
nbdkit to using it is transparent, right? As in, if the change
happens during a libvirt upgrade existing VMs will not be affected
and will continue to use the QEMU backend until they're destroyed and
started again; similarly, migrating from a host where nbdkit is
disabled to one where it is enabled, or viceversa, works fine.
2. user explicitly enables nbdkit
- if the user decides to enable nbdkit explicitly via config file,
libvirt will start using nbdkit to serve remote disks. In order for
this to work, the user would have had to install a custom selinux
policy. When the default changes in the future (because the selinux
policy is included downstream) libvirt will continue using nbdkit for
remote disks after upgrade
3. user explicitly disables nbdkit
- there is very little reason for the user to explicitly disable this
configure option until there is a widely-available selinux policy that
makes it possible to *enable* the option. I can think of two potential
reaons it might be done:
A) the user wanted to test the nbdkit feature so they explicitly
enabled it in the config file. After finding out that it failed due
to missing selinux policies, the user explicitly disabled it in the
config file rather than just deleting the configuration option.
B) the user doesn't want nbdkit to be used even if the default
changes in the future.
The only slightly problematic option for upgrades is 3A. If a user
disabled the option and then upgraded to a libvirt version that
defaulted to enabling nbdkit, that new default would not be used. If the
downstream distribution then removed the qemu block drivers for curl and
ssh, remote disks may no longer work. But since there's very little
reason for a user to explicitly disable this configure option while it
is disabled by default, I don't see this as a scenario that is likely to
happen to anybody aside from a developer or two.
Agreed.
+++ b/meson.build
@@ -1012,6 +1012,10 @@ if not conf.has('WITH_NBDKIT')
+# default value for storage_use_nbdkit config option
+use_nbdkit_default = get_option('nbdkit_config_default')
+conf.set10('USE_NBDKIT_DEFAULT', use_nbdkit_default)
I think this would work better as a feature instead of a boolean.
Right now the sane default would probably still be to keep it off,
but later we can make things better by e.g. enabling it by default
when AppArmor is used, after we've merged the necessary changes into
our AppArmor profile of course. Later still, we will be able to just
match the status of the nbdkit option unless explicitly told
otherwise.
We should also reject senseless combinations such as
-Dnbdkit=disabled and -Dnbdkit_config_default=enabled, and mention
whether or not nbdkit is enabled by default in the meson
configuration summary.
+++ b/src/qemu/qemu.conf.in
@@ -974,3 +974,13 @@
+# 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 to access the remote server directly.
+#
+# Possible values are 0 or 1. Default value is @USE_NBDKIT_DEFAULT@.
Since we're planning to make this enabled by default over time, I
think it would be nice to inform users, lest they get caught
unprepared. Something as simple as
Please note that the default might change in future releases.
could do.
+++ b/src/qemu/qemu_domain.c
@@ -10333,6 +10333,9 @@ qemuDomainPrepareStorageSourceNbdkit(virStorageSource *src,
{
g_autoptr(qemuNbdkitCaps) nbdkit = NULL;
+ if (!cfg->storageUseNbdkit)
+ return false;
This does the right thing if nbdkit support is compiled in but
disabled at the configuration file level. However, what about the
opposite scenario, where someone tries to enable nbdkit in the
configuration file but it's not been compiled in? We'd be silently
ignoring an explicit admin request.
Perhaps I'm overthinking this, and we don't go to such length for
existing options controlling the use of features that can be disabled
at compile time. Feel free to ignore this bit of feedback if that's
the case.
--
Andrea Bolognani / Red Hat / Virtualization