Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-10 Thread Richard W.M. Jones 
On Wed, Nov 09, 2022 at 01:47:15PM -0600, Eric Blake wrote:
> On Wed, Nov 02, 2022 at 01:36:26PM +, Richard W.M. Jones wrote:
> > > > -=item 4, 5, 6, 7
> > > > +=item S<4>
> > > 
> > > The S<> notation seems new here (so it's going to be inconsistent with
> > > the rest of this file, I think).
> > 
> > I was going to mention this too.  The S<> notation is used to insert
> > non-breaking spaces (for output formats that support it) in a span of
> > text so that it won't be folded over multiple lines.  AFAIK it
> > shouldn't have any effect here.
> 
> Ah, but it does:
> 
> Pod input around line 121: Expected text after =item, not a number
> 
> The use of S<> is there to keep the pod formatter happy when =item's
> sole argument would otherwise look like a number instead of arbitrary
> text.

Oh indeed!  I had forgotten about this weirdness of pod.

$ cat test.pod
=over 4

=item 2

A

=item 3

B
=back

$ pod2text test.pod 
2   A

test.pod around line 7: Expected text after =item, not a number
3   B

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-10 Thread Richard W.M. Jones 
On Thu, Nov 10, 2022 at 02:23:39PM +0100, Laszlo Ersek wrote:
> On 11/09/22 20:47, Eric Blake wrote:
> > On Wed, Nov 02, 2022 at 01:36:26PM +, Richard W.M. Jones wrote:
>  -=item 4, 5, 6, 7
>  +=item S<4>
> >>>
> >>> The S<> notation seems new here (so it's going to be inconsistent with
> >>> the rest of this file, I think).
> >>
> >> I was going to mention this too.  The S<> notation is used to insert
> >> non-breaking spaces (for output formats that support it) in a span of
> >> text so that it won't be folded over multiple lines.  AFAIK it
> >> shouldn't have any effect here.
> > 
> > Ah, but it does:
> > 
> > Pod input around line 121: Expected text after =item, not a number
> > 
> > The use of S<> is there to keep the pod formatter happy when =item's
> > sole argument would otherwise look like a number instead of arbitrary
> > text.
> > 
> 
> Sigh. :)
> 
> Thanks for the reminder. I've now found that
> 
> ["docs/virt-v2v-output-local.pod" in the v2v tree] is a good
> demonstration for numbered lists, bullet lists, and "option" list.
> 
> Interestingly, perlpod(1) does not recommend S<> for this kind of
> escaping, it recommends Z<>:
> 
>•   And perhaps most importantly, keep the items
>consistent: either use "=item *" for all of them, to
>produce bullets; or use "=item 1.", "=item 2.", etc.,
>to produce numbered lists; or use "=item foo", "=item
>bar", etc.--namely, things that look nothing like
>bullets or numbers.  (If you have a list that contains
>both: 1) things that don't look like bullets nor
>numbers,  plus 2) things that do, you should preface
>the bullet- or number-like items with "Z<>".  See Z<>
>below for an example.)
> 
>If you start with bullets or numbers, stick with them,
>as formatters use the first "=item" type to decide how
>to format the list.
> ...
>"S" -- text contains non-breaking spaces
>This means that the words in text should not be broken
>across lines.  Example: "S<$x ? $y : $z>".
> ...
>"Z<>" -- a null (zero-effect) formatting code
> ...
>Another use is to indicate that stuff in "=item
>Z<>stuff..."  is not to be considered to be a bullet or
>number.  For example, without the "Z<>", the line
> 
> =item Z<>500 Server error
> 
>could possibly be parsed as an item in a numbered list when
>it isn't meant to be.
> ...
> 
> More interestingly, *this particular use* of Z<> is not documented in
> the RHEL7 manual of perlpod(1) -- the above quote is from Fedora 35! So
> that lack of specific documentation on RHEL7 might be why we chose to
> wrap the digits in S<>, rather than to isolate them with Z<>.

Actually the first I've heard of Z<> formatting.  Could be a recent
addition as you say.  We do need to keep things working on RHEL 7 (as
far as possible without heroics).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-10 Thread Laszlo Ersek 
On 11/09/22 20:47, Eric Blake wrote:
> On Wed, Nov 02, 2022 at 01:36:26PM +, Richard W.M. Jones wrote:
 -=item 4, 5, 6, 7
 +=item S<4>
>>>
>>> The S<> notation seems new here (so it's going to be inconsistent with
>>> the rest of this file, I think).
>>
>> I was going to mention this too.  The S<> notation is used to insert
>> non-breaking spaces (for output formats that support it) in a span of
>> text so that it won't be folded over multiple lines.  AFAIK it
>> shouldn't have any effect here.
> 
> Ah, but it does:
> 
> Pod input around line 121: Expected text after =item, not a number
> 
> The use of S<> is there to keep the pod formatter happy when =item's
> sole argument would otherwise look like a number instead of arbitrary
> text.
> 

Sigh. :)

Thanks for the reminder. I've now found that

["docs/virt-v2v-output-local.pod" in the v2v tree] is a good
demonstration for numbered lists, bullet lists, and "option" list.

Interestingly, perlpod(1) does not recommend S<> for this kind of
escaping, it recommends Z<>:

   •   And perhaps most importantly, keep the items
   consistent: either use "=item *" for all of them, to
   produce bullets; or use "=item 1.", "=item 2.", etc.,
   to produce numbered lists; or use "=item foo", "=item
   bar", etc.--namely, things that look nothing like
   bullets or numbers.  (If you have a list that contains
   both: 1) things that don't look like bullets nor
   numbers,  plus 2) things that do, you should preface
   the bullet- or number-like items with "Z<>".  See Z<>
   below for an example.)

   If you start with bullets or numbers, stick with them,
   as formatters use the first "=item" type to decide how
   to format the list.
...
   "S" -- text contains non-breaking spaces
   This means that the words in text should not be broken
   across lines.  Example: "S<$x ? $y : $z>".
...
   "Z<>" -- a null (zero-effect) formatting code
...
   Another use is to indicate that stuff in "=item
   Z<>stuff..."  is not to be considered to be a bullet or
   number.  For example, without the "Z<>", the line

=item Z<>500 Server error

   could possibly be parsed as an item in a numbered list when
   it isn't meant to be.
...

More interestingly, *this particular use* of Z<> is not documented in
the RHEL7 manual of perlpod(1) -- the above quote is from Fedora 35! So
that lack of specific documentation on RHEL7 might be why we chose to
wrap the digits in S<>, rather than to isolate them with Z<>.

Laszlo
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-09 Thread Eric Blake 
On Wed, Nov 02, 2022 at 01:36:26PM +, Richard W.M. Jones wrote:
> > > -=item 4, 5, 6, 7
> > > +=item S<4>
> > 
> > The S<> notation seems new here (so it's going to be inconsistent with
> > the rest of this file, I think).
> 
> I was going to mention this too.  The S<> notation is used to insert
> non-breaking spaces (for output formats that support it) in a span of
> text so that it won't be folded over multiple lines.  AFAIK it
> shouldn't have any effect here.

Ah, but it does:

Pod input around line 121: Expected text after =item, not a number

The use of S<> is there to keep the pod formatter happy when =item's
sole argument would otherwise look like a number instead of arbitrary
text.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-04 Thread Eric Blake 
On Wed, Nov 02, 2022 at 01:43:13PM +, Richard W.M. Jones wrote:
> On Tue, Nov 01, 2022 at 02:56:09PM -0500, Eric Blake wrote:
> ...
> > +=item S<6>
> > +
> > +Triggers a call to the C function C with C
> > +set to false, which requests a soft disconnect of the current client
> > +(future client requests are rejected with C without calling
> > +into the plugin, but current requests may complete).  Since the client
> > +will likely get the response to this command, nbdkit then treats empty
> > +stderr as success for the current callback, and parses non-empty
> > +stderr as if the script had exited with code S<1>.
> 
> OK .. seems like complicated behaviour, but I can sort of see the
> reasoning behind it.
> 
> I do wonder if we just want to use separate codes for the "soft
> disconnect + OK" and the "soft disconnect + error" cases.  We have
> reserved more so we won't run out :-)

Given that both you and Laszlo requested it, I will respin along those
lines.

> 
> > +=item 7-15
> > +
> > +These exit codes are reserved for future use.  Note that versions of
> > +nbdkit E 1.34 documented that codes S<8> through S<15> behaved
> 
> This is actually a case where S<> around the nbdkit E 1.34 makes
> sense.  But it should be removed around S<8> etc and in the next line
> below.

I'll fix the pre-existing inconsistent use of S<> as part of the respin.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-02 Thread Richard W.M. Jones 
On Tue, Nov 01, 2022 at 02:56:09PM -0500, Eric Blake wrote:
...
> +=item S<6>
> +
> +Triggers a call to the C function C with C
> +set to false, which requests a soft disconnect of the current client
> +(future client requests are rejected with C without calling
> +into the plugin, but current requests may complete).  Since the client
> +will likely get the response to this command, nbdkit then treats empty
> +stderr as success for the current callback, and parses non-empty
> +stderr as if the script had exited with code S<1>.

OK .. seems like complicated behaviour, but I can sort of see the
reasoning behind it.

I do wonder if we just want to use separate codes for the "soft
disconnect + OK" and the "soft disconnect + error" cases.  We have
reserved more so we won't run out :-)

> +=item 7-15
> +
> +These exit codes are reserved for future use.  Note that versions of
> +nbdkit E 1.34 documented that codes S<8> through S<15> behaved

This is actually a case where S<> around the nbdkit E 1.34 makes
sense.  But it should be removed around S<8> etc and in the next line
below.

> +like code S<1>; although it is unlikely that many scripts relied on
> +this similarity in practice.

[...]

> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d59b797c..3994fc6a 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -768,12 +768,14 @@ TESTS += \
>   test-eval-file.sh \
>   test-eval-exports.sh \
>   test-eval-cache.sh \
> + test-eval-disconnect.sh \
>   $(NULL)
>  EXTRA_DIST += \
>   test-eval.sh \
>   test-eval-file.sh \
>   test-eval-exports.sh \
>   test-eval-cache.sh \
> + test-eval-disconnect.sh \
>   $(NULL)



>  # file plugin test.
> diff --git a/plugins/sh/call.h b/plugins/sh/call.h
> index 76de23a3..44767e81 100644
> --- a/plugins/sh/call.h
> +++ b/plugins/sh/call.h
> @@ -1,5 +1,5 @@
>  /* nbdkit
> - * Copyright (C) 2018 Red Hat Inc.
> + * Copyright (C) 2018-2022 Red Hat Inc.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions are
> @@ -51,7 +51,12 @@ typedef enum exit_code {
>OK = 0,
>ERROR = 1,   /* all script error codes are mapped to this */
>MISSING = 2, /* method missing */
> -  RET_FALSE = 3/* script exited with code 3 meaning false */
> +  RET_FALSE = 3,   /* script exited with code 3 meaning false */
> +  SHUTDOWN = 4,/* call nbdkit_shutdown() before parsing stderr */
> +  DISC_FORCE = 5,  /* call nbdkit_disconnect(true) */
> +  DISC_SOFT = 6,   /* call nbdkit_disconnect(false) */
> +  /* 7 is reserved since 1.8; handle like ERROR for now */
> +  /* 8-15 are reserved since 1.34; handle like ERROR for now */
>  } exit_code;
> 
>  extern exit_code call (const char **argv)
> diff --git a/plugins/sh/call.c b/plugins/sh/call.c
> index 2fa94d88..7d0f2b16 100644
> --- a/plugins/sh/call.c
> +++ b/plugins/sh/call.c
> @@ -1,5 +1,5 @@
>  /* nbdkit
> - * Copyright (C) 2018-2020 Red Hat Inc.
> + * Copyright (C) 2018-2022 Red Hat Inc.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions are
> @@ -397,17 +397,47 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to 
> stdin (can be NULL) */
>return ret;
>  }
> 
> -static void
> -handle_script_error (const char *argv0, string *ebuf)
> +/* Normalize return codes and parse error string. */
> +static exit_code
> +handle_script_error (const char *argv0, string *ebuf, exit_code code)
>  {
>int err;
>size_t skip = 0;
>char *p;
> 
> -  if (ebuf->len == 0) {
> +  switch (code) {
> +  case OK:
> +  case MISSING:
> +  case RET_FALSE:
> +/* Script successful. */
> +return code;
> +
> +  case SHUTDOWN:
> +/* Use trigger, then handle empty ebuf specially below */
> +nbdkit_shutdown ();
> +break;
> +
> +  case DISC_FORCE:
> +  case DISC_SOFT:
> +/* Use trigger, then handle empty ebuf specially below */
> +nbdkit_disconnect (code == DISC_FORCE);
> +break;
> +
> +  case ERROR:
> +  default:
> +/* Error even if ebuf is empty */
>  err = EIO;
> +goto parse;
> +  }
> +
> +  assert (code >= SHUTDOWN && code <= DISC_SOFT);
> +  if (ebuf->len == 0)
> +return OK;
> +  err = ESHUTDOWN;
> +
> + parse:
> +  if (ebuf->len == 0)
>  goto no_error_message;
> -  }

I guess if we had the separate codes then we wouldn't need the goto?

>/* Recognize the errno values that match NBD protocol errors */
>if (ascii_strncasecmp (ebuf->ptr, "EPERM", 5) == 0) {
> @@ -502,6 +532,7 @@ handle_script_error (const char *argv0, string *ebuf)
> 
>/* Set errno. */
>errno = err;
> +  return ERROR;
>  }
> 
>  /* Call the script with parameters.  Don't write to stdin or read from
> @@ -516,19 +547,7 @@ call (const char **argv)
>CLEANUP_FREE_STRING string ebuf = empty_vector;
> 
>r = call3 (NULL, 0, , , argv);

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-02 Thread Richard W.M. Jones 
On Wed, Nov 02, 2022 at 02:15:45PM +0100, Laszlo Ersek wrote:
> On 11/01/22 20:56, Eric Blake wrote:
> > Make it possible for the sh and eval plugins to disconnect a client or
> > shut down the entire nbdkit server by use of special return values.
> > Prior to this patch we had reserved 4-7 for future use; this defines
> > 4-6, and extends the set of reserved return values to 7-15.  We figure
> > it is unlikely that anyone is using status 8-15 with the intent that
> > it behaves identically to status 1.
> > 
> > For the testsuite, I only covered the eval plugin; but since it shares
> > common code with the sh plugin, both styles should work.
> > ---
> > 
> > Finally got the testsuite additions for this in a state that I like.
> > 
> >  plugins/sh/nbdkit-sh-plugin.pod |  37 ++-
> >  tests/Makefile.am   |   2 +
> >  plugins/sh/call.h   |   9 +-
> >  plugins/sh/call.c   |  85 +++
> >  tests/test-eval-disconnect.sh   | 185 
> >  5 files changed, 268 insertions(+), 50 deletions(-)
> >  create mode 100755 tests/test-eval-disconnect.sh
> > 
> > diff --git a/plugins/sh/nbdkit-sh-plugin.pod 
> > b/plugins/sh/nbdkit-sh-plugin.pod
> > index 2a55fdc9..37139e1b 100644
> > --- a/plugins/sh/nbdkit-sh-plugin.pod
> > +++ b/plugins/sh/nbdkit-sh-plugin.pod
> > @@ -96,4 +96,4 @@ The script should exit with specific exit codes:
> > 
> >  The method was executed successfully.
> > 
> > -=item 1 and 8-127
> > +=item 1 and 16-255
> > 
> >  There was an error.  The script may print on stderr an errno name,
> >  optionally followed by whitespace and a message, for example:
> > @@ -123,9 +123,38 @@ The requested method is not supported by the script.
> > 
> >  For methods which return booleans, this code indicates false.
> > 
> > -=item 4, 5, 6, 7
> > +=item S<4>
> 
> The S<> notation seems new here (so it's going to be inconsistent with
> the rest of this file, I think).

I was going to mention this too.  The S<> notation is used to insert
non-breaking spaces (for output formats that support it) in a span of
text so that it won't be folded over multiple lines.  AFAIK it
shouldn't have any effect here.

For some reason this existing list uses:

  =item S<0>

but I think that must be a mistake.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-02 Thread Laszlo Ersek 
On 11/01/22 20:56, Eric Blake wrote:
> Make it possible for the sh and eval plugins to disconnect a client or
> shut down the entire nbdkit server by use of special return values.
> Prior to this patch we had reserved 4-7 for future use; this defines
> 4-6, and extends the set of reserved return values to 7-15.  We figure
> it is unlikely that anyone is using status 8-15 with the intent that
> it behaves identically to status 1.
> 
> For the testsuite, I only covered the eval plugin; but since it shares
> common code with the sh plugin, both styles should work.
> ---
> 
> Finally got the testsuite additions for this in a state that I like.
> 
>  plugins/sh/nbdkit-sh-plugin.pod |  37 ++-
>  tests/Makefile.am   |   2 +
>  plugins/sh/call.h   |   9 +-
>  plugins/sh/call.c   |  85 +++
>  tests/test-eval-disconnect.sh   | 185 
>  5 files changed, 268 insertions(+), 50 deletions(-)
>  create mode 100755 tests/test-eval-disconnect.sh
> 
> diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
> index 2a55fdc9..37139e1b 100644
> --- a/plugins/sh/nbdkit-sh-plugin.pod
> +++ b/plugins/sh/nbdkit-sh-plugin.pod
> @@ -96,4 +96,4 @@ The script should exit with specific exit codes:
> 
>  The method was executed successfully.
> 
> -=item 1 and 8-127
> +=item 1 and 16-255
> 
>  There was an error.  The script may print on stderr an errno name,
>  optionally followed by whitespace and a message, for example:
> @@ -123,9 +123,38 @@ The requested method is not supported by the script.
> 
>  For methods which return booleans, this code indicates false.
> 
> -=item 4, 5, 6, 7
> +=item S<4>

The S<> notation seems new here (so it's going to be inconsistent with
the rest of this file, I think).

> 
> -These exit codes are reserved for future use.
> +Triggers a call to the C function C, which requests
> +an asynchronous exit of the nbdkit server (disconnecting all clients).
> +Since the client may get a response before shutdown is complete,
> +nbdkit then treats empty stderr as success for the current callback,
> +and parses non-empty stderr as if the script had exited with code
> +S<1>.

Exit codes 0 and 1 permit the following as well: the shell script may
output some preliminary diagnostics to stderr, but then exit with code
0. In that case, the command is considered successsful, and the script's
stderr is thrown away by nbdkit.

Should we parallel that here as well? That would require two separate
codes, like 4 and 5. They'd match error codes 0 and 1, except an async
shutdown would be initiated as well.

> +
> +=item S<5>
> +
> +Triggers a call to the C function C with C
> +set to true, which requests an abrupt disconnect of the current
> +client.  The contents of stderr are irrelevant with this status, since
> +the client will not get a response.
> +
> +=item S<6>
> +
> +Triggers a call to the C function C with C
> +set to false, which requests a soft disconnect of the current client
> +(future client requests are rejected with C without calling
> +into the plugin, but current requests may complete).  Since the client
> +will likely get the response to this command, nbdkit then treats empty
> +stderr as success for the current callback, and parses non-empty
> +stderr as if the script had exited with code S<1>.

Same comment as for code 4.

I think allowing a script to output log messages but ultimately to
succeed is nice. (Including the case where "succeed" entails shutting
down the server.)

> +
> +=item 7-15
> +
> +These exit codes are reserved for future use.  Note that versions of
> +nbdkit E 1.34 documented that codes S<8> through S<15> behaved
> +like code S<1>; although it is unlikely that many scripts relied on
> +this similarity in practice.
> 
>  =back
> 
> @@ -578,4 +607,4 @@ Richard W.M. Jones
> 
>  =head1 COPYRIGHT
> 
> -Copyright (C) 2018-2020 Red Hat Inc.
> +Copyright (C) 2018-2022 Red Hat Inc.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d59b797c..3994fc6a 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -768,12 +768,14 @@ TESTS += \
>   test-eval-file.sh \
>   test-eval-exports.sh \
>   test-eval-cache.sh \
> + test-eval-disconnect.sh \
>   $(NULL)
>  EXTRA_DIST += \
>   test-eval.sh \
>   test-eval-file.sh \
>   test-eval-exports.sh \
>   test-eval-cache.sh \
> + test-eval-disconnect.sh \
>   $(NULL)
> 
>  # file plugin test.
> diff --git a/plugins/sh/call.h b/plugins/sh/call.h
> index 76de23a3..44767e81 100644
> --- a/plugins/sh/call.h
> +++ b/plugins/sh/call.h
> @@ -1,5 +1,5 @@
>  /* nbdkit
> - * Copyright (C) 2018 Red Hat Inc.
> + * Copyright (C) 2018-2022 Red Hat Inc.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions are
> @@ -51,7 +51,12 @@ typedef enum exit_code {
>OK = 0,
>ERROR = 1,   /* all script error

[Libguestfs] [nbdkit PATCH] sh: Add exit status triggers for nbdkit_{shutdown, disconnect}

2022-11-01 Thread Eric Blake 
Make it possible for the sh and eval plugins to disconnect a client or
shut down the entire nbdkit server by use of special return values.
Prior to this patch we had reserved 4-7 for future use; this defines
4-6, and extends the set of reserved return values to 7-15.  We figure
it is unlikely that anyone is using status 8-15 with the intent that
it behaves identically to status 1.

For the testsuite, I only covered the eval plugin; but since it shares
common code with the sh plugin, both styles should work.
---

Finally got the testsuite additions for this in a state that I like.

 plugins/sh/nbdkit-sh-plugin.pod |  37 ++-
 tests/Makefile.am   |   2 +
 plugins/sh/call.h   |   9 +-
 plugins/sh/call.c   |  85 +++
 tests/test-eval-disconnect.sh   | 185 
 5 files changed, 268 insertions(+), 50 deletions(-)
 create mode 100755 tests/test-eval-disconnect.sh

diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
index 2a55fdc9..37139e1b 100644
--- a/plugins/sh/nbdkit-sh-plugin.pod
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -96,4 +96,4 @@ The script should exit with specific exit codes:

 The method was executed successfully.

-=item 1 and 8-127
+=item 1 and 16-255

 There was an error.  The script may print on stderr an errno name,
 optionally followed by whitespace and a message, for example:
@@ -123,9 +123,38 @@ The requested method is not supported by the script.

 For methods which return booleans, this code indicates false.

-=item 4, 5, 6, 7
+=item S<4>

-These exit codes are reserved for future use.
+Triggers a call to the C function C, which requests
+an asynchronous exit of the nbdkit server (disconnecting all clients).
+Since the client may get a response before shutdown is complete,
+nbdkit then treats empty stderr as success for the current callback,
+and parses non-empty stderr as if the script had exited with code
+S<1>.
+
+=item S<5>
+
+Triggers a call to the C function C with C
+set to true, which requests an abrupt disconnect of the current
+client.  The contents of stderr are irrelevant with this status, since
+the client will not get a response.
+
+=item S<6>
+
+Triggers a call to the C function C with C
+set to false, which requests a soft disconnect of the current client
+(future client requests are rejected with C without calling
+into the plugin, but current requests may complete).  Since the client
+will likely get the response to this command, nbdkit then treats empty
+stderr as success for the current callback, and parses non-empty
+stderr as if the script had exited with code S<1>.
+
+=item 7-15
+
+These exit codes are reserved for future use.  Note that versions of
+nbdkit E 1.34 documented that codes S<8> through S<15> behaved
+like code S<1>; although it is unlikely that many scripts relied on
+this similarity in practice.

 =back

@@ -578,4 +607,4 @@ Richard W.M. Jones

 =head1 COPYRIGHT

-Copyright (C) 2018-2020 Red Hat Inc.
+Copyright (C) 2018-2022 Red Hat Inc.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d59b797c..3994fc6a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -768,12 +768,14 @@ TESTS += \
test-eval-file.sh \
test-eval-exports.sh \
test-eval-cache.sh \
+   test-eval-disconnect.sh \
$(NULL)
 EXTRA_DIST += \
test-eval.sh \
test-eval-file.sh \
test-eval-exports.sh \
test-eval-cache.sh \
+   test-eval-disconnect.sh \
$(NULL)

 # file plugin test.
diff --git a/plugins/sh/call.h b/plugins/sh/call.h
index 76de23a3..44767e81 100644
--- a/plugins/sh/call.h
+++ b/plugins/sh/call.h
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2022 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -51,7 +51,12 @@ typedef enum exit_code {
   OK = 0,
   ERROR = 1,   /* all script error codes are mapped to this */
   MISSING = 2, /* method missing */
-  RET_FALSE = 3/* script exited with code 3 meaning false */
+  RET_FALSE = 3,   /* script exited with code 3 meaning false */
+  SHUTDOWN = 4,/* call nbdkit_shutdown() before parsing stderr */
+  DISC_FORCE = 5,  /* call nbdkit_disconnect(true) */
+  DISC_SOFT = 6,   /* call nbdkit_disconnect(false) */
+  /* 7 is reserved since 1.8; handle like ERROR for now */
+  /* 8-15 are reserved since 1.34; handle like ERROR for now */
 } exit_code;

 extern exit_code call (const char **argv)
diff --git a/plugins/sh/call.c b/plugins/sh/call.c
index 2fa94d88..7d0f2b16 100644
--- a/plugins/sh/call.c
+++ b/plugins/sh/call.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018-2020 Red Hat Inc.
+ * Copyright (C) 2018-2022 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following