[Libguestfs] [PATCH nbdkit 5/9 patch split 2/5] lib: Move code for parsing, passwords and paths into libnbdkit.so.

[Eric Blake]

On 3/26/20 3:13 PM, Richard W.M. Jones wrote:
> ---

Just a reminder in case you want to improve the commit message.

>   lib/Makefile.am               |  27 ++
>   server/Makefile.am            |  24 --
>   server/nbdkit.syms            |  28 +--
>   server/public.c               | 459 +---------------------------------
>   {server => lib}/extents.c     |   4 +-
>   lib/parse.c                   | 341 +++++++++++++++++++++++++
>   lib/password.c                | 152 +++++++++++
>   lib/path.c                    |  97 +++++++
>   {server => lib}/test-public.c |   3 +-
>   .gitignore                    |   2 +-
>   lib/libnbdkit.syms            |  20 ++
>   11 files changed, 645 insertions(+), 512 deletions(-)

This one's still big, but not as daunting. Thanks for doing the split.


> +++ b/server/public.c
> @@ -36,7 +36,6 @@
>   
>   #include <config.h>
>   
> -#include <fcntl.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <stdbool.h>
> @@ -44,6 +43,7 @@
>   #include <inttypes.h>
>   #include <string.h>
>   #include <unistd.h>
> +#include <fcntl.h>
>   #include <limits.h>

Intentional churn?

> diff --git a/server/test-public.c b/lib/test-public.c
> similarity index 99%
> rename from server/test-public.c
> rename to lib/test-public.c
> index fe347d44..d680e7ad 100644
> --- a/server/test-public.c
> +++ b/lib/test-public.c
> @@ -41,7 +41,8 @@
>   #include <string.h>
>   #include <unistd.h>
>   
> -#include "internal.h"
> +#include "nbdkit-plugin.h"
> +#include "nbdkit-filter.h"
>   

I like how this .c file move worked out.

Should the test-public binary still be compiled from direct files under 
lib/, or should it be linked against libnbdkit.so?  If I read the 
Makefile changes correctly, you went with the former.  I'm wondering if 
we want to add some sort of 'assert(is_initialized)' to all of our 
public entry functions to ensure that no one is actually trying to use 
libnbdkit.so without having first gone through nbdkit_private_init; if 
we did that, then test-public should indeed build from direct files 
rather than trying to link against libnbdkit.so and bypass the 
initialization normally done by the nbdkit binary.


> +++ b/lib/libnbdkit.syms
> @@ -38,6 +38,26 @@
>   {
>     # The functions we want plugins and filters to call.
>     global:
> +    nbdkit_absolute_path;
> +    nbdkit_add_extent;
> +    nbdkit_extents_count;
> +    nbdkit_extents_free;
> +    nbdkit_extents_new;
> +    nbdkit_get_extent;
> +    nbdkit_parse_bool;
> +    nbdkit_parse_int16_t;
> +    nbdkit_parse_int32_t;
> +    nbdkit_parse_int64_t;
> +    nbdkit_parse_int8_t;
> +    nbdkit_parse_int;
> +    nbdkit_parse_size;
> +    nbdkit_parse_uint16_t;
> +    nbdkit_parse_uint32_t;
> +    nbdkit_parse_uint64_t;
> +    nbdkit_parse_uint8_t;
> +    nbdkit_parse_unsigned;
> +    nbdkit_read_password;
> +    nbdkit_realpath;

Are we brave enough to list nbdkit_*?  Or is being explicit on each 
symbol still a good idea?

Are we planning on having any nbdkit_debug_* symbols in libnbdkit.so (in 
addition to the ones in the nbdkit server proper), in which case those 
should be exported, too?

Otherwise, LGTM.

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