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/abae4deaa68b9123a59d58b5a0ba2b1...
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