In the just-added nbdkit-sh-plugin.pod, you documented that unlike other
plugins (where introspection or struct member population) can be used to
determine which functionalities are supported, the shell plugin has to
implement can_write and friends to return a special status 2 to document
not supported, and 3 to indicate false, in part because you can't detect
if a pwrite function is present without running it.
But since pwrite normally takes additional parameters, could we instead
document that nbdkit calling '/path/to/script pwrite <handle>' while
intentionally omitting <count> and <offset> is sufficient for that to
return specific status (2 if pwrite in general is not handled, 3 if it
is handled but not appropriate for the given handle, and 0 if it works),
without needing a 'can_write' entry point? I guess the only difference
is that all the logic would go through the single pwrite interface and
you wouln't need a can_write.
Also, I spotted a potential bug in sh.c:
+/* This is the generic function that calls the script. It can
+ * optionally write to the script's stdin and read from the script's
+ * stdout and stderr. It returns the raw error code and does no error
+ * processing.
+ */
+static int
+call3 (const char *wbuf, size_t wbuflen, /* sent to stdin */
+ char **rbuf, size_t *rbuflen, /* read from stdout */
+ char **ebuf, size_t *ebuflen, /* read from stderr */
+ const char **argv) /* script + parameters */
+{
...
+
+ execvp (argv[0], (char **) argv);
+ perror (argv[0]);
+ _exit (EXIT_FAILURE);
+ }
perror() is not async-safe, but since nbdkit may be multithreaded,
calling a non-async-safe function in between fork() and successful
exec/exit() can deadlock or cause other problems. I don't know if it is
worth trying to make that code safer, though.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization:
qemu.org |
libvirt.org