similar to: IRC question about consuming stdin

Trying to read a password or inline script from stdin when the -s option was used to connect to our client over stdin is not going to work. It would be nice to fail such usage as soon as possible, by giving plugins a means to decide if it is safe to use stdio during .config. Update nbdkit_read_password and the sh plugin to take advantage of the new entry point. Signed-off-by: Eric Blake

Trying to read a password or inline script from stdin when the -s option was used to connect to our client over stdin is not going to work. It would be nice to fail such usage as soon as possible, by giving plugins a means to decide if it is safe to use stdio during .config. Update nbdkit_read_password and the sh plugin to take advantage of the new entry point. Also, treat 'nbdkit -s

$ ./nbdkit ssh host=localhost /nosuchfile password=-0 --run 'qemu-img info $nbd' abc fcntl: Bad file descriptor The reason for this is that we close the file descriptor after reading the password. Closing stdin causes bad stuff to happen. --- docs/nbdkit-plugin.pod | 5 +++++ server/public.c | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git

As shown in the previous patch, plugins may choose to use stdin or stdout during .config. But from .get_ready onwards, well-written plugins shouldn't be needing any further use of stdin/out. We already swapped stdin/out to /dev/null while daemonizing, but did not do do during -f or --run, which leads to some surprising inconsistency when trying to debug a plugin that works in the foreground

As shown in the previous patch, plugins may choose to use stdin or stdout during .config. But from .get_ready onwards, well-written plugins shouldn't be needing any further use of stdin/out. We already swapped stdin/out to /dev/null while daemonizing, but did not do do during -f or --run, which leads to some surprising inconsistency when trying to debug a plugin that works in the foreground

On Wed, Aug 30, 2023 at 05:21:19PM -0500, Eric Blake wrote: > I hit another transient failure in libnbd CI when a poorly-written > eval script did not consume all of stdin during .pwrite. As behaving > as a data sink can be a somewhat reasonable feature of a > quickly-written sh or eval plugin, we should not be so insistent as > treating an EPIPE failure as an immediate return of

On 8/31/23 00:21, Eric Blake wrote: > I hit another transient failure in libnbd CI when a poorly-written > eval script did not consume all of stdin during .pwrite. As behaving > as a data sink can be a somewhat reasonable feature of a > quickly-written sh or eval plugin, we should not be so insistent as > treating an EPIPE failure as an immediate return of EIO to the client. >

On Thu, Aug 31, 2023 at 10:40:53AM +0200, Laszlo Ersek wrote: > On 8/31/23 00:21, Eric Blake wrote: > > I hit another transient failure in libnbd CI when a poorly-written > > eval script did not consume all of stdin during .pwrite. As behaving > > as a data sink can be a somewhat reasonable feature of a > > quickly-written sh or eval plugin, we should not be so

On Thu, Aug 31, 2023 at 12:05:17PM +0200, Laszlo Ersek wrote: > On 8/31/23 11:47, Richard W.M. Jones wrote: > > On Thu, Aug 31, 2023 at 11:12:59AM +0200, Laszlo Ersek wrote: > >> On 8/31/23 10:02, Richard W.M. Jones wrote: > >>> > >>> On Wed, Aug 30, 2023 at 05:21:19PM -0500, Eric Blake wrote: > >>>> I hit another transient failure in libnbd

I hit another transient failure in libnbd CI when a poorly-written eval script did not consume all of stdin during .pwrite. As behaving as a data sink can be a somewhat reasonable feature of a quickly-written sh or eval plugin, we should not be so insistent as treating an EPIPE failure as an immediate return of EIO to the client. Signed-off-by: Eric Blake <eblake at redhat.com> --- I

On Thu, Aug 31, 2023 at 11:18:27AM +0200, Laszlo Ersek wrote: > On 8/31/23 10:55, Richard W.M. Jones wrote: > > On Thu, Aug 31, 2023 at 10:40:53AM +0200, Laszlo Ersek wrote: > >> On 8/31/23 00:21, Eric Blake wrote: > >>> I hit another transient failure in libnbd CI when a poorly-written > >>> eval script did not consume all of stdin during .pwrite. As

On 8/31/23 10:55, Richard W.M. Jones wrote: > On Thu, Aug 31, 2023 at 10:40:53AM +0200, Laszlo Ersek wrote: >> On 8/31/23 00:21, Eric Blake wrote: >>> I hit another transient failure in libnbd CI when a poorly-written >>> eval script did not consume all of stdin during .pwrite. As behaving >>> as a data sink can be a somewhat reasonable feature of a

On 8/31/23 10:02, Richard W.M. Jones wrote: > > On Wed, Aug 30, 2023 at 05:21:19PM -0500, Eric Blake wrote: >> I hit another transient failure in libnbd CI when a poorly-written >> eval script did not consume all of stdin during .pwrite. As behaving >> as a data sink can be a somewhat reasonable feature of a >> quickly-written sh or eval plugin, we should not be so

Refuse to run if stdin/out/err start life closed. stdin/out are essential when using -s (if they aren't present, we want to fail fast), and should remain unused otherwise (but ensuring they are open means we don't have to worry about other fd creation events accidentally colliding). We also want to ensure that nothing opens into the slot for stderr, as any error message we produce might

This command fails with an incorrect error message: $ nbdkit ssh host=localhost /nosuchfile password=- --run 'qemu-img info $nbd' </dev/null password: nbdkit: error: could not read password from stdin: Inappropriate ioctl for device The error (ENOTTY Inappropriate ioctl for device) is actually a leftover errno from the previous isatty call. This happens because getline(3) can

On Thu, Aug 31, 2023 at 11:12:59AM +0200, Laszlo Ersek wrote: > On 8/31/23 10:02, Richard W.M. Jones wrote: > > > > On Wed, Aug 30, 2023 at 05:21:19PM -0500, Eric Blake wrote: > >> I hit another transient failure in libnbd CI when a poorly-written > >> eval script did not consume all of stdin during .pwrite. As behaving > >> as a data sink can be a

See comment for explanation and https://listman.redhat.com/archives/libguestfs/2023-August/032468.html --- tests/Makefile.am | 2 + plugins/sh/call.c | 33 +++++++----- tests/test-sh-pwrite-ignore-stdin.sh | 77 ++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index

VDDK plugin + --run was broken because of the following sequence of events: (1) After .config_complete, server redirects stdin/stdout to /dev/null. (2) Server then calls vddk_get_ready which reexecs. (3) We restart the server from the top, but with stdin/stdout redirected to /dev/null. So saved_stdin/saved_stdout save /dev/null. (4) run_command is called which "restores"

Enhance the testsuite to ensure we don't regress with recent changes to stdin/out handling. This adds: - test-single-sh.sh: prove that 'nbdkit -s sh script' is viable - test-stdio.sh: create plugin that checks stdin/out match /dev/null, then run it with -s, --run, -f Signed-off-by: Eric Blake <eblake@redhat.com> --- tests/Makefile.am | 23 ++++++

In v2: - use int instead of bool in the public header - split the tests from the code - don't overload test-layers; instead, add new tests - add a missing fflush exposed by the new tests - other minor cleanups Eric Blake (3): server: Add nbdkit_stdio_safe server: Sanitize stdin/out before running plugin code server: More tests of stdin/out handling docs/nbdkit-plugin.pod |