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

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to