Re: [Libguestfs] [PATCH nbdkit 1/9] server: Add libnbdkit.so.

Eric Blake
2020-08-18T12:48:43Z

On 8/18/20 5:50 AM, Richard W.M. Jones wrote:
> This essentially turns the whole of nbdkit into a library.  There is a
> rump nbdkit program left which simply contains a main() function that
> forwards to the nbdkit_main() function in the library.
>
> The reason for this is to allow nbdkit to be compiled on Windows,
> because Windows does not allow shared libraries to contain any
> undefined symbols.  nbdkit previously relied on undefined symbols in
> all plugins and filters (eg. nbdkit_parse_int) which get resolved at
> run time by the nbdkit program.  In order to make this work on Windows
> we need to have a new library (libnbdkit.so on Linux, but probably
> called something like LIBNBDKIT.DLL on Windows) which will contain
> these symbols, and plugins can then be compiled with -lnbdkit so they
> will link with this library, hence no undefined symbols.

This is a lot nicer than the last attempt we made several months ago to
move only a bare minimum into a library.

>
> Note this change is backwards compatible.  Because the rump nbdkit
> binary links with -lnbdkit it still contains and reexports all the
> symbols, so plugins and filters which don't explicitly link with
> -lnbdkit will still be loadable (on existing platforms).
> ---
>   server/Makefile.am                 | 21 ++++++++------
>   server/main.c                      |  2 +-
>   server/nbdkit.c                    | 46 ++++++++++++++++++++++++++++++
>   server/nbdkit.syms                 |  1 +
>   tests/test-nbdkit-backend-debug.sh | 24 ++++++++--------
>   wrapper.c                          | 10 +++++--
>   6 files changed, 80 insertions(+), 24 deletions(-)

ACK.

> -nbdkit_LDFLAGS = \
> +libnbdkit_la_LDFLAGS = \
> +	-shared $(NO_UNDEFINED_ON_WINDOWS) \

On first read, I didn't see where this was declared; but then I saw you
have already pushed some preliminary cleanups, including adding
NO_UNDEFINED_ON_WINDOWS in 9d052c1d.

> +++ b/server/nbdkit.c

> +#include <config.h>
> +
> +#include <stdio.h>
> +
> +extern int nbdkit_main (int argc, char *argv[]);

A bit odd to declare this in a .c; but I don't see any existing decent
.h to put it in, nor is it worth adding a new one just for this.  So it
is fine right here.

> +
> +int
> +main (int argc, char *argv[])
> +{
> +  /* This does nothing except call into the real main function in
> +   * libnbdkit.so.
> +   */
> +  return nbdkit_main (argc, argv);
> +}
> diff --git a/server/nbdkit.syms b/server/nbdkit.syms
> index a67669b7..a516cc0f 100644
> --- a/server/nbdkit.syms
> +++ b/server/nbdkit.syms
> @@ -54,6 +54,7 @@
>       nbdkit_extents_new;
>       nbdkit_get_extent;
>       nbdkit_is_tls;
> +    nbdkit_main;

Do we want to export it as _nbdkit_main, to make it obvious that plugins
shouldn't try calling it?  That's cosmetic if you think it is worth it.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org