[Libguestfs] [nbdkit PATCH 2/4] file: Add .list_exports support

Eric Blake eblake at redhat.com
Fri Aug 7 13:08:18 UTC 2020


On 8/6/20 9:23 PM, Eric Blake wrote:
> Add a new mode to the file plugin, using directory=DIR instead of
> [file=]FILE, to allow it to serve all regular/block files in a given
> directory, as well as advertising the names of those files it will be
> serving.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---

In addition to your review, I'm also addressing:


> 
> +static int file_list_exports (int readonly, int default_only,
> +                              struct nbdkit_exports *exports)

Formatting - this file prefers:

static int
file_list_exports (...)

> +{
> +  struct dirent *entry;
> +  struct stat sb;
> +  int fd;
> +
> +  if (!directory)
> +    return nbdkit_add_export (exports, "", NULL);
> +
> +  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
> +  rewinddir (dir);
> +  fd = dirfd (dir);
> +  if (fd == -1) {
> +    nbdkit_error ("dirfd: %m");
> +    return -1;
> +  }
> +  errno = 0;
> +  while ((entry = readdir (dir)) != NULL) {
> +    /* TODO: Optimize with d_type and/or statx when present? */
> +    if (fstatat (fd, entry->d_name, &sb, 0) == 0 &&
> +        (S_ISREG (sb.st_mode) || S_ISBLK (sb.st_mode))) {
> +      if (nbdkit_add_export (exports, entry->d_name, NULL) == -1) {
> +        close (fd);

Bug - the fd obtained from dirfd() is closed by closedir(); closing it 
ourselves is a double-close which may affect further operation on the 
DIR or interfere with other code.  We weren't tripping on the bug 
because most clients don't call NBD_OPT_LIST more than once, but such a 
client would make it more obvious.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




More information about the Libguestfs mailing list