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