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