[Libguestfs] [PATCH nbdkit 2/4] server: Add nbdkit_shutdown() call.

[Eric Blake]

On 3/4/20 9:17 AM, Richard W.M. Jones wrote:
> Plugins and filters may call this to initiate an asynchronous shutdown
> of the server.  This would only be used in the connected phase —
> plugins should still call exit(3) directly for configuration failure.
> 
> It is equivalent to sending a kill signal to self, but it's cleaner to
> have an API for this and better for potential future non-POSIX platforms.
> ---
>   docs/nbdkit-plugin.pod       | 19 +++++++-
>   include/nbdkit-common.h      |  1 +
>   include/nbdkit-plugin.h      |  2 +-
>   tests/Makefile.am            | 20 ++++++++
>   server/nbdkit.syms           |  3 +-
>   server/quit.c                |  8 +++-
>   tests/test-shutdown.sh       | 71 ++++++++++++++++++++++++++++
>   tests/test-shutdown-plugin.c | 89 ++++++++++++++++++++++++++++++++++++
>   8 files changed, 209 insertions(+), 4 deletions(-)
> 

> +++ b/tests/test-shutdown.sh

> +# Start nbdkit with the shutdown plugin.
> +start_nbdkit -P shutdown.pid -U $sock $plugin
> +
> +pid=`cat shutdown.pid`
> +
> +# Reads are fine, nbdkit should still be running afterwards.
> +for i in 1 2 3; do
> +    qemu-img info "nbd:unix:$sock"

qemu-img should support nbd+unix:///?socket=$sock URIs; any reason we 
aren't using them here?

> +++ b/tests/test-shutdown-plugin.c
> +
> +static int64_t
> +shutdown_get_size (void *handle)
> +{
> +  return INT64_C (1024*1024);

This looks fishy.  POSIX says that:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdint.h.html#tag_13_48
"The argument in any instance of these macros shall be an unsuffixed 
integer constant with a value that does not exceed the limits for the 
corresponding type.
...For example, if uint_least64_t is a name for the type unsigned long 
long, then UINT64_C(0x123) might expand to the integer constant 0x123ULL."

In fact, in glibc's /usr/include/stdint.h, we have:

# if __WORDSIZE == 64
#  define INT64_C(c)    c ## L
# else
#  define INT64_C(c)    c ## LL
# endif

Your code works by sheer luck (expanding to the three tokens '1024', 
'*', '1024L'), which in turn results in an expression with 64-bit type, 
but in general, INT64_C should ONLY be used on a single literal value, 
not an arbitrary expression.

But you really don't need to use INT64_C; we can just rely on C's 
automatic type promotion to turn our 'int' expression 1024*1024 into the 
proper int64_t return value.

Otherwise, this is a nice addition!

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