Re: Re: [libvirt PATCH v3] qemu: add runtime config option for nbdkit
On Wed, Jan 03, 2024 at 04:52:00PM -0600, Jonathon Jongsma wrote:
On 1/3/24 12:37 PM, Andrea Bolognani wrote:
> When parsing the configuration file, we shouldn't make this part
> conditional. If the user tries to enable nbdkit usage with a build of
> libvirt that doesn't include support, it's a *good thing* to report
> an error rather than silently ignoring that request. Hence, we want
> something like
>
> #if !WITH_NDBKIT
> if (cfg->storageUseNbdkit) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> "%s",
> _("ndbkit support is not compiled in"));
> return -1;
> }
> #endif /* WITH_NBDKIT */
>
> here.
Once again, i modeled it on the WITH_IP configure options. In that case, we
do not return an error if the WITH_IP configuration options are specified
when the functionality is compiled out. But I'm happy to do it this way as
well.
In that case, however, whether the IP-related options are accepted
does not depend on some choice the user/packager made at build time:
the monolithic daemon will always accept them, the modular ones
won't. There's much less opportunity for confusion. So yeah, it's
similar enough that it makes sense to consider implementing the same
behavior, but IMO different enough that ultimately a separate
handling is preferrable.
> Since I'm not 100% sure everything I've explained will
come through
> successfully (feeling a bit tired right now) I've just pushed a
> commit with all the changes I'm suggesting here:
>
>
https://gitlab.com/abologna/libvirt/-/commit/abae4deaa68b9123a59d58b5a0ba...
>
That commit looks find to me. Do you want to squash it? or would you like me
to?
Please squash it in, consider maybe writing a better error message
for the "nbdkit not compiled in" case, and post v4 to the list.
--
Andrea Bolognani / Red Hat / Virtualization