[Libguestfs] [nbdkit PATCH v2 1/2] sh: Advertise max known status in --dump-plugin

Thu Nov 10 14:15:12 UTC 2022

On 11/09/22 21:43, Eric Blake wrote:
> We are about to define specific meaning to more return status values.
> To make it easier to probe if nbdkit is new enough to honor such
> definitions (and thus whether it makes sense to write a script that
> tries to utilize such special return values), it is worth advertising
> the max known status supported by the given build of the sh/eval
> plugin.
> ---
>  plugins/eval/nbdkit-eval-plugin.pod |  3 +-
>  plugins/sh/nbdkit-sh-plugin.pod     |  5 ++++
>  tests/Makefile.am                   |  2 ++
>  plugins/sh/call.h                   |  6 ++--
>  plugins/sh/methods.c                |  6 +++-
>  tests/test-eval-dump-plugin.sh      | 43 +++++++++++++++++++++++++++++
>  6 files changed, 61 insertions(+), 4 deletions(-)
>  create mode 100755 tests/test-eval-dump-plugin.sh
> 
> diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod
> index fa9784ae..0c10ae43 100644
> --- a/plugins/eval/nbdkit-eval-plugin.pod
> +++ b/plugins/eval/nbdkit-eval-plugin.pod
> @@ -146,7 +146,8 @@ The script fragment behaves the same way as the corresponding method
>  in L<nbdkit-sh-plugin(1)>.  In particular, parameters are identical,
>  C<$tmpdir> is present and used in the same way, the exit code must be
>  one of the valid exit codes described in that manual page, and error
> -handling works the same way too.
> +handling works the same way too.  Likewise, B<nbdkit --dump-plugin
> +eval> includes a line for B<max_known_status=> in nbdkit E<ge> 1.34.
> 
>  Note that a C<config> callback will only handle keys not recognized as
>  callback names; when picking key=value pairs that you want your script
> diff --git a/plugins/sh/nbdkit-sh-plugin.pod b/plugins/sh/nbdkit-sh-plugin.pod
> index 2a55fdc9..1c539599 100644
> --- a/plugins/sh/nbdkit-sh-plugin.pod
> +++ b/plugins/sh/nbdkit-sh-plugin.pod
> @@ -129,6 +129,11 @@ These exit codes are reserved for future use.
> 
>  =back
> 
> +In nbdkit E<gt> 1.34, it is possible to probe whether additional exit
> +codes have been assigned meaning, by looking for the line
> +B<max_known_status=> in the output of B<nbdkit --dump-plugin sh>.  If
> +this line is not present, exit codes 4 and above behave like status 1.
> +
>  =head2 Temporary directory
> 
>  A fresh script is invoked for each method call (ie. scripts are
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d59b797c..a4e93686 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-dump-plugin.sh \
>  	$(NULL)
>  EXTRA_DIST += \
>  	test-eval.sh \
>  	test-eval-file.sh \
>  	test-eval-exports.sh \
>  	test-eval-cache.sh \
> +	test-eval-dump-plugin.sh \
>  	$(NULL)
> 
>  # file plugin test.
> diff --git a/plugins/sh/call.h b/plugins/sh/call.h
> index 76de23a3..fab77a3c 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,9 @@ 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 */
> +  /* Adjust methods.c:sh_dump_plugin when defining new codes */

We could introduce an "out-of-band" value for terminating the enum list
(such as SH_EXIT_CODE_MAX); it would never be handled as an actual exit
code, but could be referenced in sh_dump_plugin(). New exit codes would
be inserted just before it.

But if this "pattern" is not used in nbdkit already, then the current
comment will work perfectly well. (I'm mentioning it because it's
extensively used e.g. in edk2.)

> +  /* 4-7 is reserved since 1.8; handle like ERROR for now */
>  } exit_code;
> 
>  extern exit_code call (const char **argv)
> diff --git a/plugins/sh/methods.c b/plugins/sh/methods.c
> index 5c1ee5f8..4a75a3a0 100644
> --- a/plugins/sh/methods.c
> +++ b/plugins/sh/methods.c
> @@ -1,5 +1,5 @@
>  /* nbdkit
> - * Copyright (C) 2018-2021 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
> @@ -58,6 +58,10 @@ sh_dump_plugin (void)
>    const char *args[] = { script, method, NULL };
>    CLEANUP_FREE_STRING string o = empty_vector;
> 
> +  /* Dump information about the sh/eval features */
> +  printf ("max_known_status=%d\n", RET_FALSE);
> +
> +  /* Dump any additional information from the script */

Tricky comment; the "--dump-plugin" option with the "dump_plugin="
operand at the end of the test made me do a double-take :) The
documentation covers it though:

https://libguestfs.org/nbdkit-sh-plugin.3.html#Methods
https://libguestfs.org/nbdkit-plugin.3.html#dump_plugin

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Laszlo

>    if (script) {
>      /* Call dump_plugin method. */
>      switch (call_read (&o, args)) {
> diff --git a/tests/test-eval-dump-plugin.sh b/tests/test-eval-dump-plugin.sh
> new file mode 100755
> index 00000000..ba25efbc
> --- /dev/null
> +++ b/tests/test-eval-dump-plugin.sh
> @@ -0,0 +1,43 @@
> +#!/usr/bin/env bash
> +# nbdkit
> +# Copyright (C) 2022 Red Hat Inc.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are
> +# met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +#
> +# * Redistributions in binary form must reproduce the above copyright
> +# notice, this list of conditions and the following disclaimer in the
> +# documentation and/or other materials provided with the distribution.
> +#
> +# * Neither the name of Red Hat nor the names of its contributors may be
> +# used to endorse or promote products derived from this software without
> +# specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
> +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
> +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +# SUCH DAMAGE.
> +
> +# Check --dump-plugin operation using eval
> +
> +source ./functions.sh
> +set -e
> +set -x
> +
> +requires_plugin eval
> +
> +nbdkit --dump-plugin eval | grep '^max_known_status='
> +
> +nbdkit --dump-plugin eval dump_plugin='echo extra=stuff' | grep '^extra=stuff$'
> 