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/ _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
