Re: [Libguestfs] [PATCH nbdkit 2/2] file: Allow a file descriptor to be passed to the plugin

On Wed, Aug 17, 2022 at 04:38:11PM +0100, Richard W.M. Jones wrote: > This allows you to pass in a file descriptor to the plugin, eg: > > $ exec 6<>/var/tmp/fedora-36.img > $ ./nbdkit file fd=6 --run 'nbdinfo $uri' > > Note this was previously possible on most platforms using /dev/fd/<N>, > but not all platforms that have file descriptors support this > (although it is POSIX) and it seems better to make this explicit. The parenthetical feels odd here. /dev/fd/<N> is not POSIX; what IS a POSIX feature is the ability to pass an open file descriptor by inheritance.

> @@ -202,6 +208,17 @@ file_config (const char *key, const char *value) > if (!directory) > return -1; > } > + else if (strcmp (key, "fd") == 0) { > + if (mode != mode_none) goto wrong_mode; > + mode = mode_filedesc; > + assert (filedesc == -1); > + if (nbdkit_parse_int ("fd", value, &filedesc) == -1) > + return -1; > + if (filedesc <= 2) { > + nbdkit_error ("file descriptor must be >= 3"); Is it worth using STDERR_FILENO instead of open-coding 2? I'll be okay if your answer is that this is one place where <unistd.h> has too much indirection ;)

> @@ -408,59 +436,69 @@ file_open (int readonly) ... > + case mode_filedesc: > + h->fd = dup (filedesc); > + if (h->fd == -1) { > + nbdkit_error ("dup fd=%d: %m", filedesc); > + free (h); > + return NULL; > + } > + file = "<file descriptor>"; /* This is needed for error messages. */ > + break; Is it worth trying to figure out if the FD has O_RDWR privileges and set h->can_write accordingly, to match...

> default: abort (); > } > > - h = malloc (sizeof *h); > - if (h == NULL) { > - nbdkit_error ("malloc: %m"); > - if (dfd != -1) > - close (dfd); > - return NULL; > - } > - > - flags = O_CLOEXEC|O_NOCTTY; > - if (readonly) { > - flags |= O_RDONLY; > - h->can_write = false; > - } > - else { > - flags |= O_RDWR; > - h->can_write = true; > - } > - > - h->fd = openat (dfd, file, flags); > - if (h->fd == -1 && !readonly) { > - nbdkit_debug ("open O_RDWR failed, falling back to read-only: %s: %m", > - file); > - flags = (flags & ~O_ACCMODE) | O_RDONLY; > - h->fd = openat (dfd, file, flags); > - h->can_write = false; > - } ... what we do for normal files? Overall looks like a nice addition. ACK series