[Libguestfs] [PATCH nbdkit v3] tmpdisk: Pass any parameters as shell variables to the command.

[Eric Blake]

On 4/8/20 4:19 AM, Richard W.M. Jones wrote:
> This allows us to be much more flexible about what commands can be
> used.  It also means we do not need to encode any special behaviour
> for type or label parameters.
> ---
>   plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod | 115 ++++++++++----
>   tests/Makefile.am                         |   2 +
>   plugins/tmpdisk/tmpdisk.c                 | 184 ++++++++++++++++------
>   plugins/tmpdisk/default-command.sh.in     |   6 +
>   tests/test-tmpdisk-command.sh             |  52 ++++++
>   5 files changed, 278 insertions(+), 81 deletions(-)
> 

> +=head2 Serve a fresh operating system to each client
> +
> + nbdkit tmpdisk 16G os=fedora-31 \
> +     command=' virt-builder -o "$disk" --size ${size}b "$os" '
> +
> +=head2 Serve a throwaway snapshot of a base image to each client
> +
> +You could create a base image using L<virt-builder(1)> or a similar
> +tool, and then serve different throwaway snapshots to each client:
> +
> + virt-builder fedora-31 -o /var/tmp/base.img
> + nbdkit tmpdisk size=0 base=/var/tmp/base.img \
> +     command=' cp --sparse=always "$base" "$disk" '
> +

Nice contrast on how to cache the startup costs.


>   
> +/* For block devices, stat->st_size is not the true size. */
> +static int64_t
> +block_device_size (int fd)
> +{
> +  off_t size;
> +
> +  size = lseek (fd, 0, SEEK_END);
> +  if (size == -1) {
> +    nbdkit_error ("lseek: %m");
> +    return -1;
> +  }
> +
> +  return size;
> +}

As you've mentioned elsewhere, we may want to add something in common/ 
that manages common tasks for disk images (size of a block device, 
handling .zero, ...); but for now the duplication is fine.


> +  /* The command could set $disk to a regular file or a block device
> +   * (or a symlink to either), so we must check that here.
> +   */
> +  if (S_ISBLK (statbuf.st_mode)) {
> +    h->size = block_device_size (h->fd);
> +    if (h->size == -1)
> +      goto error;
> +  }
> +  else                          /* Regular file. */
> +    h->size = statbuf.st_size;
> +  nbdkit_debug ("tmpdisk: requested_size = %" PRIi64 ", size = %" PRIi64,
> +                requested_size, h->size);
> +
>     /* We don't need the disk to appear in the filesystem since we hold
>      * a file descriptor and access it through that, so unlink the disk.
>      * This also ensures it is always cleaned up.
>      */
>     unlink (disk);
> +  rmdir (dir);

If the user creates a symlink, we don't remove the original - but that's 
not our problem. I think this works just fine as written.

> +++ b/tests/test-tmpdisk-command.sh

> +source ./functions.sh
> +set -e
> +set -x
> +
> +requires nbdsh -c 'exit (not h.supports_uri ())'
> +
> +# - If multiple parameters appear, last one is used.
> +# - Test quoting.
> +# - size=0 because we ignore it in the command itself.
> +nbdkit -f -v -U - tmpdisk 0 a=2 a=1 b=1024 c="a ' b ' c" \
> +       command='
> +set -x
> +set -e
> +if [ $a -ne 1 ]; then exit 1; fi
> +if [ "$c" != "a '\'' b '\'' c" ]; then exit 1; fi
> +truncate -s $b "$disk"
> +' \
> +       --run '
> +nbdsh -u "$uri" -c "assert h.get_size() == 1024"
> +'

Nice.  LGTM; the patch is now ready to go.

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