Re: [Libguestfs] [PATCH nbdkit 9/9] eval, sh: Define $nbdkit_safe_stdio = 0|1 in scripts.
On 4/15/20 11:16 AM, Richard W.M. Jones wrote:
In the subject, you describe $nbdkit_safe_stdio, but in the patch body...
---
plugins/eval/nbdkit-eval-plugin.pod | 11 +++--------
plugins/sh/nbdkit-sh-plugin.pod | 18 +++++++++++++++++-
plugins/sh/call.c | 8 ++++++--
tests/test-single-sh.sh | 4 ++++
4 files changed, 30 insertions(+), 11 deletions(-)
The patch does not work as intended as currently written: when we invoke
/path/to/script config $key $value, we have already set up stdin/stdout
to our own pipes [1]. For .config and .config_complete, reading will
always see EOF (no other callback needs to interact with the original
stdin, and callbacks like .pwrite actually use the pipe for data). If
we want to allow a script to read a password from stdin, we need to
preserve the original fd to .config and .config_complete rather than
passing in an empty pipe (but those are the only two callbacks where it
makes sense, and even then, only when we did not use script=- or when -s
is in effect).
[1] ./nbdkit eval config='ls -l /proc/$$/fd >/dev/tty' a=b
total 0
lr-x------. 1 eblake eblake 64 Apr 15 13:03 0 -> 'pipe:[1669308]'
l-wx------. 1 eblake eblake 64 Apr 15 13:03 1 -> 'pipe:[1669309]'
l-wx------. 1 eblake eblake 64 Apr 15 13:03 2 -> 'pipe:[1669310]'
lr-x------. 1 eblake eblake 64 Apr 15 13:03 255 ->
/tmp/nbdkitevalVUNZ1F/config
+++ b/plugins/sh/nbdkit-sh-plugin.pod
@@ -119,7 +119,14 @@ These exit codes are reserved for future use.
=back
-=head2 Temporary directory
+=head2 Environment variables passed to the script
+
+When running the script, certain environment variables are set by the
+plugin:
+
+=over 4
+
+=item C<$tmpdir>
A fresh script is invoked for each method call (ie. scripts are
stateless), so if the script needs to store state it has to store it
@@ -131,6 +138,15 @@ the script. This directory persists for the lifetime of nbdkit and
is
deleted when nbdkit exits. The name of the directory is passed to
each script invocation in the C<$tmpdir> environment variable.
+=item C<$nbdkit_stdio_safe>
...this name makes more sense (matching the public API), but disagrees
with the commit title.
Side thought: Both the eval and sh plugins already pass on all
unrecognized key=value pairs through to the .config callback, and error
out if the config callback returns missing. But right now, if a script
wants to set up an environment variable that will still be present to
affect later calls, it has to track things itself (perhaps by having the
.config callback append to $tmpdir/vars, then all other callbacks start
by 'source $tmpdir/vars'). Would it make sense to reserve a special
exit value from the .config callback for the case where the script wants
the current key=value passed to config to be preserved to all future
callbacks? Or even state that the config callback exiting with status 2
(for missing) is NOT an error, but does the key=value preservation
automatically?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org