On Tue, Apr 14, 2020 at 09:05:57AM -0500, Eric Blake wrote:
On 4/14/20 2:47 AM, Richard W.M. Jones wrote:
>On Mon, Apr 13, 2020 at 07:28:59PM -0500, Eric Blake wrote:
>[...]
>
>This patch is fine and can be pushed if you want, but I've got some
>small comments.
>
>>+If C<nbdkit_stdio_safe> returns true, the value of the configuration
>>+parameter may be used to trigger reading additional data through stdin
>>+(such as a password or inline script).
>
>I wonder if we want to say "returns 1" rather than true, to give
>ourselves wiggle room in future in case we suddenly decided that we
>needed this to return an error indication? On the other hand, maybe
>errors can never happen in any conceivable situation.
It might be possible to change the function to return -1 when called
at the wrong time (at .get_ready or later). As written, the patch
merely documents that this function is not useful at that point.
But for now, I didn't see the point in making that change, but
merely leaving it for the future.
It's still wrong for the plugin to read from stdin or write to stdout
even after .get_ready, so AFAICT checking nbdkit_stdio_safe() is
reasonable [shouldn't return -1] after get_ready.
Anyway you decide what's best. Patch series over all is fine.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/