This creates filesystems on demand. A client simply connects with a desired export name and a new export is created. The export is persistent (until deleted by the server admin), and clients may disconnect and reconnect. In some respects this is similar to the nbdkit-tmpdisk-plugin, or nbdkit-file-plugin with the dir= option. --- plugins/ondemand/nbdkit-ondemand-plugin.pod | 190 ++++++ plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod | 3 +- configure.ac | 2 + plugins/ondemand/Makefile.am | 81 +++ tests/Makefile.am | 10 + plugins/ondemand/ondemand.c | 634 ++++++++++++++++++++ plugins/ondemand/default-command.sh.in | 57 ++ tests/test-ondemand-list.sh | 66 ++ tests/test-ondemand.sh | 74 +++ .gitignore | 1 + TODO | 7 + 11 files changed, 1124 insertions(+), 1 deletion(-) diff --git a/plugins/ondemand/nbdkit-ondemand-plugin.pod b/plugins/ondemand/nbdkit-ondemand-plugin.pod new file mode 100644 index 00000000..be46b1c2 --- /dev/null +++ b/plugins/ondemand/nbdkit-ondemand-plugin.pod @@ -0,0 +1,190 @@ +=head1 NAME + +nbdkit-ondemand-plugin - create filesystems on demand + +=head1 SYNOPSIS + + nbdkit ondemand dir=EXPORTSDIR [size=]SIZE + { [type=ext4|xfs|vfat|...] [label=LABEL] + | command=COMMAND [VAR=VALUE ...] } + +=head1 DESCRIPTION + +This is a plugin for L<nbdkit(1)> which creates persistent filesystems +on demand. Clients may simply connect to the server, requesting a +particular export name, and a new filesystem is created if it does not +exist already. Clients can also disconnect and reconnect with the +same export name and the same filesystem will still be available. +Filesystems are stored in a directory on the server, so they also +persist over nbdkit and server restarts. + +Each filesystem is locked while it is in use by a client, preventing +two clients from accessing the same filesystem (which would cause +corruption). + +Similar plugins include L<nbdkit-file-plugin(1)> which can serve a +predefined set of exports (clients cannot create more), +L<nbdkit-tmpdisk-plugin(1)> which creates a fresh temporary filesystem +for each client, and L<nbdkit-linuxdisk-plugin(1)> which exports a +single filesystem from a local directory on the server. + +When a new export name is requested by a client, a sparse file of the +same name is created in C<dir=EXPORTSDIR> on the server. The file +will be formatted with L<mkfs(8)>. The size of the file is currently +fixed by the C<size=SIZE> parameter, but we intend to make this +client-configurable in future. The filesystem type and label may also +be specified, otherwise C<ext4> and no label is used. + +Export names must be E<le> C<NAME_MAX> (usually 255) bytes in length +and must not contain certain characters including C<.>, C</> and C<:>. +There may be other limitations added in future. Client requests which +do not obey these restrictions are rejected. As a special case, +export name C<""> is mapped to the file name F<default>. + +=head2 Security considerations + +You should B<only> use this in an environment where you trust all your +clients, since clients can use this plugin to consume arbitrary +amounts of disk space by creating unlimited exports. It is therefore +best to take steps to limit where clients can connect from using +L<nbdkit-ip-filter(1)>, firewalls, or TLS client certificates. + +=head2 The command parameter + +Instead of running mkfs you can run an arbitrary command (a shell +script fragment) to create the disk. + +The other parameters to the plugin are turned into shell variables +passed to the command. For example C<type> becomes the shell variable +C<$type>, etc. Any parameters you want can be passed to the plugin +and will be turned into shell variables (not only C<type> and +C<label>) making this a very flexible method to create filesystems and +disks of all kinds. + +Two special variables are also passed to the shell script fragment: + +=over 4 + +=item C<$disk> + +The absolute path of the disk file. This is partially controlled by +the client so you should quote it carefully. This file is not +pre-created, the command must create it for example using: + + truncate -s $size "$disk" + +=item C<$size> + +The virtual size in bytes. This is the C<size> parameter, converted +to bytes. Note the final size served to the client is whatever disk +size C<command> creates. + +=back + +=head1 EXAMPLE + +Run the server like this: + + mkdir /var/tmp/exports + nbdkit ondemand dir=/var/tmp/exports 1G + +Clients can connect and create 1G ext4 filesystems on demand using +commands such as these (note the different export names): + + nbd-client -b 512 server /dev/nbd0 -N export1 + mount /dev/nbd0 /mnt + + guestfish --format=raw -a nbd://localhost/export2 -m /dev/sda + + qemu-img info nbd:localhost:10809:exportname=export2 + +On the server you would see two filesystems created: + + $ ls -l /var/tmp/exports + -rw-rw-r--. 1 rjones rjones 1073741824 Aug 13 21:40 export1 + -rw-rw-r--. 1 rjones rjones 1073741824 Aug 13 21:40 export2 + +The plugin does not clean these up. If they are no longer needed then +the server admin should delete them (or use a tmp cleaner). + +=head1 PARAMETERS + +=over 4 + +=item B<command='>COMMANDB<'> + +Instead of running L<mkfs(8)> to create the initial filesystem, run +C<COMMAND> (a shell script fragment which usually must be quoted to +protect it from the shell). See L</The command parameter> and +L</EXAMPLES> sections above. + +=item B<dir=>EXPORTSDIR + +The directory where filesystems are saved. When first using this +plugin you should point this to an empty directory. When clients +connect, filesystems are created here. + +This parameter is required. + +=item B<label=>LABEL + +Select the filesystem label. The default is not set. + +=item [B<size=>]SIZE + +Specify the virtual size of all of the filesystems. + +If using C<command>, this is only a suggested size. The actual size +of the resulting disk will be the size of the disk created by +C<command>. + +This parameter is required. + +C<size=> is a magic config key and may be omitted in most cases. +See L<nbdkit(1)/Magic parameters>. + +=item B<type=>FS + +Select the filesystem type. The default is C<ext4>. Most +non-networked, non-cluster filesystem types supported by the +L<mkfs(8)> command can be used here. + +=back + +=head1 FILES + +=over 4 + +=item F<$plugindir/nbdkit-ondemand-plugin.so> + +The plugin. + +Use C<nbdkit --dump-config> to find the location of C<$plugindir>. + +=back + +=head1 VERSION + +C<nbdkit-ondemand-plugin> first appeared in nbdkit 1.22. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-plugin(3)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-ip-filter(1)>, +L<nbdkit-limit-filter(1)>, +L<nbdkit-linuxdisk-plugin(1)>, +L<nbdkit-memory-plugin(1)>, +L<nbdkit-tmpdisk-plugin(1)>, +L<nbdkit-tls(1)>, +L<mkfs(8)>, +L<mke2fs(8)>. + +=head1 AUTHORS + +Richard W.M. Jones + +=head1 COPYRIGHT + +Copyright (C) 2018-2020 Red Hat Inc. diff --git a/plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod b/plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod index be56d68a..9cecad34 100644 --- a/plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod +++ b/plugins/tmpdisk/nbdkit-tmpdisk-plugin.pod @@ -13,7 +13,8 @@ nbdkit-tmpdisk-plugin - create a fresh temporary filesystem for each client This L<nbdkit(1)> plugin is used for creating temporary filesystems for thin clients. Each time a client connects it will see a fresh, empty filesystem for its exclusive use. B<The filesystem is deleted> -when the client disconnects. +when the client disconnects. If you want a persistent filesystem, use +L<nbdkit-ondemand-plugin(1)> instead. When a new client connects, a blank, sparse file of the required size is created in C<$TMPDIR> (or F</var/tmp>). L<mkfs(8)> is then run on diff --git a/configure.ac b/configure.ac index fee6fe9a..5d74251a 100644 --- a/configure.ac +++ b/configure.ac @@ -81,6 +81,7 @@ non_lang_plugins="\ memory \ nbd \ null \ + ondemand \ partitioning \ pattern \ random \ @@ -1119,6 +1120,7 @@ AC_CONFIG_FILES([Makefile plugins/nbd/Makefile plugins/null/Makefile plugins/ocaml/Makefile + plugins/ondemand/Makefile plugins/partitioning/Makefile plugins/pattern/Makefile plugins/perl/Makefile diff --git a/plugins/ondemand/Makefile.am b/plugins/ondemand/Makefile.am new file mode 100644 index 00000000..a1bc00d6 --- /dev/null +++ b/plugins/ondemand/Makefile.am @@ -0,0 +1,81 @@ +# nbdkit +# Copyright (C) 2017-2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = \ + default-command.sh.in \ + nbdkit-ondemand-plugin.pod \ + $(NULL) + +# The default command we use (if we don't use command=) comes from a +# shell script which is turned into a C source file. +BUILT_SOURCES = default-command.c + +default-command.c: default-command.sh.in Makefile + rm -f $@ $@-t + echo 'const char *command =' > $@-t + $(SED) -e '/^#/d' -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $< >> $@-t + echo ';' >> $@-t + mv $@-t $@ + +plugin_LTLIBRARIES = nbdkit-ondemand-plugin.la + +nbdkit_ondemand_plugin_la_SOURCES = \ + default-command.c \ + ondemand.c \ + $(top_srcdir)/include/nbdkit-plugin.h \ + $(NULL) + +nbdkit_ondemand_plugin_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_ondemand_plugin_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_ondemand_plugin_la_LDFLAGS = \ + -module -avoid-version -shared $(SHARED_LDFLAGS) \ + -Wl,--version-script=$(top_srcdir)/plugins/plugins.syms \ + $(NULL) +nbdkit_ondemand_plugin_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-ondemand-plugin.1 +CLEANFILES += $(man_MANS) + +nbdkit-ondemand-plugin.1: nbdkit-ondemand-plugin.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index ca1e76d1..96c8b900 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -757,6 +757,16 @@ test_null_SOURCES = test-null.c test_null_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS) test_null_LDADD = $(LIBNBD_LIBS) +# ondemand plugin test. +TESTS += \ + test-ondemand.sh \ + test-ondemand-list.sh \ + $(NULL) +EXTRA_DIST += \ + test-ondemand.sh \ + test-ondemand-list.sh \ + $(NULL) + # partitioning plugin test. TESTS += \ test-partitioning1.sh \ diff --git a/plugins/ondemand/ondemand.c b/plugins/ondemand/ondemand.c new file mode 100644 index 00000000..f85d630d --- /dev/null +++ b/plugins/ondemand/ondemand.c @@ -0,0 +1,634 @@ +/* nbdkit + * Copyright (C) 2017-2020 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdbool.h> +#include <stdint.h> +#include <inttypes.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <dirent.h> +#include <errno.h> +#include <assert.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/wait.h> + +#include <pthread.h> + +#define NBDKIT_API_VERSION 2 +#include <nbdkit-plugin.h> + +#include "cleanup.h" +#include "utils.h" + +static char *dir; /* dir parameter */ +static DIR *exportsdir; /* opened exports dir */ +static int64_t requested_size = -1; /* size parameter on the command line */ + +/* Shell variables. */ +static struct var { + struct var *next; + const char *key, *value; +} *vars, *last_var; + +/* This comes from default-command.c which is generated from + * default-command.sh.in. + */ +extern const char *command; + +static void +ondemand_unload (void) +{ + struct var *v, *v_next; + + for (v = vars; v != NULL; v = v_next) { + v_next = v->next; + free (v); + } + + closedir (exportsdir); + free (dir); +} + +static int +ondemand_config (const char *key, const char *value) +{ + if (strcmp (key, "command") == 0) { + command = value; + } + else if (strcmp (key, "size") == 0) { + requested_size = nbdkit_parse_size (value); + if (requested_size == -1) + return -1; + } + else if (strcmp (key, "dir") == 0) { + dir = nbdkit_realpath (value); + if (dir == NULL) + return -1; + } + + /* This parameter cannot be set on the command line since it is used + * to pass the disk name to the command. + */ + else if (strcmp (key, "disk") == 0) { + nbdkit_error ("'disk' parameter cannot be set on the command line"); + return -1; + } + + /* Any other parameter will be forwarded to a shell variable. */ + else { + struct var *new_var; + + new_var = malloc (sizeof *new_var); + if (new_var == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } + + new_var->next = NULL; + new_var->key = key; + new_var->value = value; + + /* Append it to the linked list. */ + if (vars == NULL) { + assert (last_var == NULL); + vars = last_var = new_var; + } + else { + assert (last_var != NULL); + last_var->next = new_var; + last_var = new_var; + } + } + + return 0; +} + +static int +ondemand_config_complete (void) +{ + if (dir == NULL || requested_size == -1) { + nbdkit_error ("dir and size parameters are required"); + return -1; + } + + return 0; +} + +static int +ondemand_get_ready (void) +{ + exportsdir = opendir (dir); + if (exportsdir == NULL) { + nbdkit_error ("opendir: %s: %m", dir); + return -1; + } + + return 0; +} + +#define ondemand_config_help \ + "dir=<EXPORTSDIR> (required) Directory containing filesystems.\n" \ + "size=<SIZE> (required) Virtual filesystem size.\n" \ + "label=<LABEL> The filesystem label.\n" \ + "type=ext4|... The filesystem type.\n" \ + "command=<COMMAND> Alternate command instead of mkfs." + +/* Because we rewind the exportsdir handle, we need a lock to protect + * list_exports from being called in parallel. + */ +static pthread_mutex_t exports_lock = PTHREAD_MUTEX_INITIALIZER; + +static int +ondemand_list_exports (int readonly, int default_only, + struct nbdkit_exports *exports) +{ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&exports_lock); + struct dirent *d; + + /* First entry should be the default export. XXX Should we check if + * the "default" file was created? I don't think we need to. + */ + if (nbdkit_add_export (exports, "", NULL) == -1) + return -1; + if (default_only) return 0; + + /* Read the rest of the exports. */ + rewinddir (exportsdir); + + /* XXX Output is not sorted. Does it matter? */ + while (errno = 0, (d = readdir (exportsdir)) != NULL) { + /* Skip all dot files. "." anywhere in the export name is + * rejected by the plugin, so commands can use dot files to "hide" + * files in the export dir (eg. if needing to keep state). + */ + if (d->d_name[0] == '.') + continue; + + /* Skip the "default" filename which refers to the "" export. */ + if (strcmp (d->d_name, "default") == 0) + continue; + + if (nbdkit_add_export (exports, d->d_name, NULL) == -1) + return -1; + } + + /* Did readdir fail? */ + if (errno != 0) { + nbdkit_error ("readdir: %s: %m", dir); + return -1; + } + + return 0; +} + +struct handle { + int fd; + int64_t size; + const char *exportname; + bool can_punch_hole; +}; + +/* Since clients that want multi-conn should all pass the same export + * name, this is safe. + */ +static int +ondemand_can_multi_conn (void *handle) +{ + return 1; +} + +static int +ondemand_can_trim (void *handle) +{ +#ifdef FALLOC_FL_PUNCH_HOLE + return 1; +#else + return 0; +#endif +} + +static int +ondemand_can_fua (void *handle) +{ + return NBDKIT_FUA_NATIVE; +} + +/* This creates and runs the full "mkfs" (or whatever) command. */ +static int +run_command (const char *disk) +{ + FILE *fp; + CLEANUP_FREE char *cmd = NULL; + size_t len = 0; + int r; + struct var *v; + + fp = open_memstream (&cmd, &len); + if (fp == NULL) { + nbdkit_error ("open_memstream: %m"); + return -1; + } + + /* Avoid stdin/stdout leaking (because of nbdkit -s). */ + fprintf (fp, "exec </dev/null >/dev/null\n"); + + /* Set the standard shell variables. */ + fprintf (fp, "disk="); + shell_quote (disk, fp); + putc ('\n', fp); + fprintf (fp, "size=%" PRIi64 "\n", requested_size); + putc ('\n', fp); + + /* The other parameters/shell variables. */ + for (v = vars; v != NULL; v = v->next) { + /* Keys probably can never contain shell-unsafe chars (because of + * nbdkit's own restrictions), but quoting it makes it safe. + */ + shell_quote (v->key, fp); + putc ('=', fp); + shell_quote (v->value, fp); + putc ('\n', fp); + } + putc ('\n', fp); + + /* The command. */ + fprintf (fp, "%s", command); + + if (fclose (fp) == EOF) { + nbdkit_error ("memstream failed"); + return -1; + } + + r = system (cmd); + if (r == -1) { + nbdkit_error ("failed to execute command: %m"); + return -1; + } + if (WIFEXITED (r) && WEXITSTATUS (r) != 0) { + nbdkit_error ("command exited with code %d", WEXITSTATUS (r)); + return -1; + } + else if (WIFSIGNALED (r)) { + nbdkit_error ("command killed by signal %d", WTERMSIG (r)); + return -1; + } + else if (WIFSTOPPED (r)) { + nbdkit_error ("command stopped by signal %d", WSTOPSIG (r)); + return -1; + } + + return 0; +} + +/* 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; +} + +static void * +ondemand_open (int readonly) +{ + struct handle *h; + CLEANUP_FREE char *disk = NULL; + int flags, err; + struct stat statbuf; + struct flock lock; + + h = malloc (sizeof *h); + if (h == NULL) { + nbdkit_error ("malloc: %m"); + goto error; + } + h->fd = -1; + h->size = -1; + h->can_punch_hole = true; + + /* This is safe since we're only storing it in the handle, so only + * for the lifetime of this connection. + */ + h->exportname = nbdkit_export_name (); + if (!h->exportname) { + nbdkit_error ("internal error: expected nbdkit_export_name () != NULL"); + goto error; + } + if (strcmp (h->exportname, "") == 0) + h->exportname = "default"; + + /* Verify that the export name is valid. */ + if (strlen (h->exportname) > NAME_MAX || + strchr (h->exportname, '.') || + strchr (h->exportname, '/') || + strchr (h->exportname, ':')) { + nbdkit_error ("invalid exportname ‘%s’ rejected", h->exportname); + goto error; + } + + /* Try to open the filesystem. */ + if (readonly) + flags = O_RDONLY | O_CLOEXEC; + else + flags = O_RDWR | O_CLOEXEC; + h->fd = openat (dirfd (exportsdir), h->exportname, flags); + if (h->fd == -1) { + if (errno != ENOENT) { + nbdkit_error ("open: %s/%s: %m", dir, h->exportname); + goto error; + } + + /* Create the filesystem. */ + if (asprintf (&disk, "%s/%s", dir, h->exportname) == -1) { + nbdkit_error ("asprintf: %m"); + goto error; + } + + /* Now run the mkfs command. */ + if (run_command (disk) == -1) + goto error; + + h->fd = openat (dirfd (exportsdir), h->exportname, flags); + if (h->fd == -1) { + nbdkit_error ("open: %s/%s: %m", dir, h->exportname); + goto error; + } + } + + /* Lock the file to prevent filesystem corruption. It's safe for + * all clients to be reading. If a client wants to write it must + * have exclusive access. + * + * This uses a currently Linux-specific extension. It requires + * Linux >= 3.15 (released in 2014, later backported to RHEL 7). + * There is no sensible way to do this in pure POSIX. + */ +#ifdef F_OFD_SETLK + memset (&lock, 0, sizeof lock); + if (readonly) + lock.l_type = F_RDLCK; + else + lock.l_type = F_WRLCK; + lock.l_whence = SEEK_SET; + lock.l_start = 0; + lock.l_len = 0; + if (fcntl (h->fd, F_OFD_SETLK, &lock) == -1) { + if (errno == EACCES || errno == EAGAIN) { + nbdkit_error ("%s: filesystem is locked by another client", + h->exportname); + /* XXX Would be nice if NBD protocol supported some kind of "is + * locked" indication. If it did we could use it here. + */ + errno = EINVAL; + goto error; + } + else { + nbdkit_error ("fcntl: %s/%s: %m", dir, h->exportname); + goto error; + } + } +#endif + + /* Find the size of the disk. */ + if (fstat (h->fd, &statbuf) == -1) { + nbdkit_error ("fstat: %s: %m", disk); + goto error; + } + + /* 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 ("ondemand: requested_size = %" PRIi64 ", size = %" PRIi64, + requested_size, h->size); + + /* Return the handle. */ + return h; + + error: + err = errno; + if (h) { + if (h->fd >= 0) + close (h->fd); + free (h); + } + errno = err; + return NULL; +} + +static void +ondemand_close (void *handle) +{ + struct handle *h = handle; + + close (h->fd); + free (h); +} + +static int64_t +ondemand_get_size (void *handle) +{ + struct handle *h = handle; + + return h->size; +} + +/* Read data from the file. */ +static int +ondemand_pread (void *handle, void *buf, + uint32_t count, uint64_t offset, + uint32_t flags) +{ + struct handle *h = handle; + + while (count > 0) { + ssize_t r = pread (h->fd, buf, count, offset); + if (r == -1) { + nbdkit_error ("pread: %m"); + return -1; + } + if (r == 0) { + nbdkit_error ("pread: unexpected end of file"); + return -1; + } + buf += r; + count -= r; + offset += r; + } + + return 0; +} + +/* Flush the file to disk. */ +static int +ondemand_flush (void *handle, uint32_t flags) +{ + struct handle *h = handle; + + if (fdatasync (h->fd) == -1) { + nbdkit_error ("fdatasync: %m"); + return -1; + } + + return 0; +} + +/* Write data to the file. */ +static int +ondemand_pwrite (void *handle, const void *buf, + uint32_t count, uint64_t offset, + uint32_t flags) +{ + struct handle *h = handle; + + while (count > 0) { + ssize_t r = pwrite (h->fd, buf, count, offset); + if (r == -1) { + nbdkit_error ("pwrite: %m"); + return -1; + } + buf += r; + count -= r; + offset += r; + } + + if ((flags & NBDKIT_FLAG_FUA) && ondemand_flush (handle, 0) == -1) + return -1; + + return 0; +} + +#if defined (FALLOC_FL_PUNCH_HOLE) +static int +do_fallocate (int fd, int mode, off_t offset, off_t len) +{ + int r = fallocate (fd, mode, offset, len); + if (r == -1 && errno == ENODEV) { + /* kernel 3.10 fails with ENODEV for block device. Kernel >= 4.9 fails + * with EOPNOTSUPP in this case. Normalize errno to simplify callers. + */ + errno = EOPNOTSUPP; + } + return r; +} + +static bool +is_enotsup (int err) +{ + return err == ENOTSUP || err == EOPNOTSUPP; +} +#endif + +/* Punch a hole in the file. */ +static int +ondemand_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) +{ +#ifdef FALLOC_FL_PUNCH_HOLE + struct handle *h = handle; + int r; + + if (h->can_punch_hole) { + r = do_fallocate (h->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, + offset, count); + if (r == -1) { + /* Trim is advisory; we don't care if it fails for anything other + * than EIO or EPERM. + */ + if (errno == EPERM || errno == EIO) { + nbdkit_error ("fallocate: %m"); + return -1; + } + + if (is_enotsup (EOPNOTSUPP)) + h->can_punch_hole = false; + + nbdkit_debug ("ignoring failed fallocate during trim: %m"); + } + } +#endif + + if ((flags & NBDKIT_FLAG_FUA) && ondemand_flush (handle, 0) == -1) + return -1; + + return 0; +} + +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL + +static struct nbdkit_plugin plugin = { + .name = "ondemand", + .version = PACKAGE_VERSION, + + .unload = ondemand_unload, + .config = ondemand_config, + .config_complete = ondemand_config_complete, + .config_help = ondemand_config_help, + .magic_config_key = "size", + .get_ready = ondemand_get_ready, + + .list_exports = ondemand_list_exports, + + .can_multi_conn = ondemand_can_multi_conn, + .can_trim = ondemand_can_trim, + .can_fua = ondemand_can_fua, + .get_size = ondemand_get_size, + + .open = ondemand_open, + .close = ondemand_close, + .pread = ondemand_pread, + .pwrite = ondemand_pwrite, + .flush = ondemand_flush, + .trim = ondemand_trim, + + .errno_is_preserved = 1, +}; + +NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/ondemand/default-command.sh.in b/plugins/ondemand/default-command.sh.in new file mode 100644 index 00000000..d60402b5 --- /dev/null +++ b/plugins/ondemand/default-command.sh.in @@ -0,0 +1,57 @@ +# nbdkit +# -*- mode: shell-script -*- +# Copyright (C) 2017-2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +# If not set, default to ext4. +type="${type:-ext4}" + +labelopt='-L' + +case "$type" in + ext?) + extra='-F' ;; + *fat|msdos) + extra='-I' ;; + ntfs) + extra='-Q -F' + labelopt='-n' ;; + xfs) + extra='-f' ;; +esac + +# Create the output disk. +truncate -s $size "$disk" + +if [ "x$label" = "x" ]; then + mkfs -t "$type" $extra "$disk" +else + mkfs -t "$type" $extra $labelopt "$label" "$disk" +fi diff --git a/tests/test-ondemand-list.sh b/tests/test-ondemand-list.sh new file mode 100755 index 00000000..5e848e8d --- /dev/null +++ b/tests/test-ondemand-list.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +source ./functions.sh +set -e +set -x + +requires qemu-nbd --version + +dir=`mktemp -d` +cleanup_fn rm -rf $dir + +out=test-ondemand-list.out +rm -f $out +cleanup_fn rm -f $out + +# Put some files into the exports directory to pretend that we're +# restarting nbdkit after a previous run. +touch $dir/default +touch $dir/export1 +touch $dir/export2 +touch $dir/export3 + +export LANG=C +nbdkit -U - ondemand dir=$dir size=1M \ + --run 'qemu-nbd -k $unixsocket -L' > $out +cat $out + +# We should have 4 exports, since "default" file is the same as the +# default export. +grep "exports available: 4" $out + +# Check the 4 exports are present. +grep "export: ''" $out +grep "export: 'export1'" $out +grep "export: 'export2'" $out +grep "export: 'export3'" $out diff --git a/tests/test-ondemand.sh b/tests/test-ondemand.sh new file mode 100755 index 00000000..758c2565 --- /dev/null +++ b/tests/test-ondemand.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2020 Red Hat Inc. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# * Neither the name of Red Hat nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +source ./functions.sh +set -e +set -x + +requires guestfish --version +requires qemu-img --version + +dir=`mktemp -d` +cleanup_fn rm -rf $dir + +sock=`mktemp -u` +files="ondemand.pid $sock" +rm -f $files +cleanup_fn rm -f $files + +# Run nbdkit. +start_nbdkit -P ondemand.pid -U $sock --log=stderr ondemand dir=$dir size=100M + +# Simply querying an export will create the filesystem. +qemu-img info nbd:unix:$sock +qemu-img info nbd:unix:$sock:exportname=test + +test -f $dir/default +test -f $dir/test + +# These should fail because the exportname is invalid. +if qemu-img info nbd:unix:$sock:exportname=/bad || + qemu-img info nbd:unix:$sock:exportname=.bad || + qemu-img info nbd:unix:$sock:exportname=bad. || + qemu-img info nbd:unix:$sock:exportname=bad:bad +then + echo "$0: expected failure trying to create bad exportname" + exit 1 +fi + +# Check the filesystem is persistent. +guestfish --format=raw -a "nbd://?socket=$sock" -m /dev/sda <<EOF + write /test.txt "hello" +EOF +guestfish --ro --format=raw -a "nbd://?socket=$sock" -m /dev/sda <<EOF + cat /test.txt +EOF diff --git a/.gitignore b/.gitignore index 255a97a5..2c463909 100644 --- a/.gitignore +++ b/.gitignore @@ -77,6 +77,7 @@ plugins/*/*.3 /plugins/golang/examples/*/nbdkit-*-plugin.h /plugins/golang/examples/*/nbdkit-*-plugin.so /plugins/ocaml/nbdkit-ocamlexample-plugin.so +/plugins/ondemand/default-command.c /plugins/rust/Cargo.lock /plugins/rust/target /plugins/tmpdisk/default-command.c diff --git a/TODO b/TODO index c329382d..d5802974 100644 --- a/TODO +++ b/TODO @@ -154,6 +154,13 @@ nbdkit-torrent-plugin: * The C++ could be a lot more natural. At the moment it's a kind of “C with C++ extensions”. +nbdkit-ondemand-plugin: + +* Implement more callbacks, eg. .zero + +* Allow client to select size up to a limit, eg. by sending export + names like ‘export:4G’. + Suggestions for language plugins -------------------------------- -- 2.27.0
On 8/14/20 12:20 PM, Richard W.M. Jones wrote:> This creates filesystems on demand. A client simply connects with a > desired export name and a new export is created. The export is > persistent (until deleted by the server admin), and clients may > disconnect and reconnect. In some respects this is similar to the > nbdkit-tmpdisk-plugin, or nbdkit-file-plugin with the dir= option. > ---> + > +This is a plugin for L<nbdkit(1)> which creates persistent filesystems > +on demand. Clients may simply connect to the server, requesting a > +particular export name, and a new filesystem is created if it does not > +exist already. Clients can also disconnect and reconnect with the > +same export name and the same filesystem will still be available. > +Filesystems are stored in a directory on the server, so they also > +persist over nbdkit and server restarts. > + > +Each filesystem is locked while it is in use by a client, preventing > +two clients from accessing the same filesystem (which would cause > +corruption).A rather crucial difference from the file plugin dir= mode ;)> + > +Similar plugins include L<nbdkit-file-plugin(1)> which can serve a > +predefined set of exports (clients cannot create more),Hmm - I wonder if it is worth a filter that runs a script any time an .open fails. That script could send email to an administrator, or even create a filesystem on the user's behalf; then run that filter in front of the file plugin to let the user create images after all. The exclusion between clients is an interesting trick to figure out, though (the existing --filter=noparallel is too strict - you want parallel clients visiting different files, but not parallel clients visiting the same file).> +=head2 Security considerations > + > +You should B<only> use this in an environment where you trust all your > +clients, since clients can use this plugin to consume arbitrary > +amounts of disk space by creating unlimited exports. It is therefore > +best to take steps to limit where clients can connect from using > +L<nbdkit-ip-filter(1)>, firewalls, or TLS client certificates.Yep, a definite need for this warning.> +++ b/plugins/ondemand/ondemand.c> +/* Because we rewind the exportsdir handle, we need a lock to protect > + * list_exports from being called in parallel. > + */ > +static pthread_mutex_t exports_lock = PTHREAD_MUTEX_INITIALIZER;An alternative is to diropen() each time .list_exports gets called. Either way should work, though. I still have some pending patches to improve .list_exports (split out a .default_export function, add an is_tls parameter), so there may be some churn in this area (for that matter, I have not yet pushed my patches for .list_exports in the file plugin, because I was trying to minimize churn while working on pending patches). We'll just have to see how it goes; I don't mind rebasing.> + > +static int > +ondemand_list_exports (int readonly, int default_only, > + struct nbdkit_exports *exports) > +{ > + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&exports_lock); > + struct dirent *d; > + > + /* First entry should be the default export. XXX Should we check if > + * the "default" file was created? I don't think we need to. > + */ > + if (nbdkit_add_export (exports, "", NULL) == -1) > + return -1; > + if (default_only) return 0; > + > + /* Read the rest of the exports. */ > + rewinddir (exportsdir); > + > + /* XXX Output is not sorted. Does it matter? */ > + while (errno = 0, (d = readdir (exportsdir)) != NULL) { > + /* Skip all dot files. "." anywhere in the export name is > + * rejected by the plugin, so commands can use dot files to "hide" > + * files in the export dir (eg. if needing to keep state). > + */ > + if (d->d_name[0] == '.') > + continue;This skips dot files (leading dot); but not those containing '.' or ':' elsewhere. Did you want to skip more files, so that you aren't advertising stuff that can't pass open?> + > + /* Skip the "default" filename which refers to the "" export. */ > + if (strcmp (d->d_name, "default") == 0) > + continue; > + > + if (nbdkit_add_export (exports, d->d_name, NULL) == -1) > + return -1; > + } > + > + /* Did readdir fail? */ > + if (errno != 0) { > + nbdkit_error ("readdir: %s: %m", dir); > + return -1; > + } > + > + return 0; > +} > + > +struct handle { > + int fd; > + int64_t size; > + const char *exportname; > + bool can_punch_hole; > +}; > + > +/* Since clients that want multi-conn should all pass the same export > + * name, this is safe. > + */ > +static int > +ondemand_can_multi_conn (void *handle) > +{ > + return 1; > +}Hmm. Are you permitting multiple clients to the same export, or did you decide that was too likely to cause fs corruption, and locking out users on the same export? The docs above said otherwise, making this look wrong.> + > +static int > +ondemand_can_trim (void *handle) > +{ > +#ifdef FALLOC_FL_PUNCH_HOLE > + return 1; > +#else > + return 0; > +#endif > +}The more plugins we add that touch the file system, the more it seems like it is worth our while to factor out helper libraries.> + > + /* Lock the file to prevent filesystem corruption. It's safe for > + * all clients to be reading. If a client wants to write it must > + * have exclusive access.Ah, you aren't locking out parallel readers, but rather doing .pwrite locking. Interesting.> + * > + * This uses a currently Linux-specific extension. It requires > + * Linux >= 3.15 (released in 2014, later backported to RHEL 7). > + * There is no sensible way to do this in pure POSIX. > + */ > +#ifdef F_OFD_SETLK > + memset (&lock, 0, sizeof lock); > + if (readonly) > + lock.l_type = F_RDLCK; > + else > + lock.l_type = F_WRLCK; > + lock.l_whence = SEEK_SET; > + lock.l_start = 0; > + lock.l_len = 0; > + if (fcntl (h->fd, F_OFD_SETLK, &lock) == -1) { > + if (errno == EACCES || errno == EAGAIN) { > + nbdkit_error ("%s: filesystem is locked by another client", > + h->exportname); > + /* XXX Would be nice if NBD protocol supported some kind of "is > + * locked" indication. If it did we could use it here.Yeah, I've thought about the idea of adding a way for clients to advertise their intent for read vs. read-write during handshaking.> +++ b/plugins/ondemand/default-command.sh.in > @@ -0,0 +1,57 @@ > +# nbdkit > +# -*- mode: shell-script -*- > +# Copyright (C) 2017-2020 Red Hat Inc. > +#No bash shebang line, but it looks portable to POSIX /bin/sh.> +++ b/tests/test-ondemand.sh> +++ b/TODO > @@ -154,6 +154,13 @@ nbdkit-torrent-plugin: > * The C++ could be a lot more natural. At the moment it's a kind of > “C with C++ extensions”. > > +nbdkit-ondemand-plugin: > + > +* Implement more callbacks, eg. .zero > + > +* Allow client to select size up to a limit, eg. by sending export > + names like ‘export:4G’.I like the idea for using exportnames to pass in additional information. Overall looks pretty reasonable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2020-Aug-15 21:13 UTC
Re: [Libguestfs] [PATCH nbdkit] New ondemand plugin.
On Sat, Aug 15, 2020 at 03:41:39PM -0500, Eric Blake wrote:> On 8/14/20 12:20 PM, Richard W.M. Jones wrote: > >+Similar plugins include L<nbdkit-file-plugin(1)> which can serve a > >+predefined set of exports (clients cannot create more), > > Hmm - I wonder if it is worth a filter that runs a script any time > an .open fails. That script could send email to an administrator,But this would duplicate existing infrastructure which can send alerts on syslog messages. However nbdkit (and indeed other servers) could help by having better diagnostics. I wonder if there are extra fields we could be providing to journald.> >+++ b/plugins/ondemand/ondemand.c > > >+/* Because we rewind the exportsdir handle, we need a lock to protect > >+ * list_exports from being called in parallel. > >+ */ > >+static pthread_mutex_t exports_lock = PTHREAD_MUTEX_INITIALIZER; > > An alternative is to diropen() each time .list_exports gets called. > Either way should work, though.diropen == opendir?> I still have some pending patches to improve .list_exports (split > out a .default_export function, add an is_tls parameter), so there > may be some churn in this area (for that matter, I have not yet > pushed my patches for .list_exports in the file plugin, because I > was trying to minimize churn while working on pending patches). > We'll just have to see how it goes; I don't mind rebasing.Right - this is indeed one major reason I sent this for review - it's a new plugin which integrally uses list_exports.> >+ > >+static int > >+ondemand_list_exports (int readonly, int default_only, > >+ struct nbdkit_exports *exports) > >+{ > >+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&exports_lock); > >+ struct dirent *d; > >+ > >+ /* First entry should be the default export. XXX Should we check if > >+ * the "default" file was created? I don't think we need to. > >+ */ > >+ if (nbdkit_add_export (exports, "", NULL) == -1) > >+ return -1; > >+ if (default_only) return 0; > >+ > >+ /* Read the rest of the exports. */ > >+ rewinddir (exportsdir); > >+ > >+ /* XXX Output is not sorted. Does it matter? */ > >+ while (errno = 0, (d = readdir (exportsdir)) != NULL) { > >+ /* Skip all dot files. "." anywhere in the export name is > >+ * rejected by the plugin, so commands can use dot files to "hide" > >+ * files in the export dir (eg. if needing to keep state). > >+ */ > >+ if (d->d_name[0] == '.') > >+ continue; > > This skips dot files (leading dot); but not those containing '.' or > ':' elsewhere. Did you want to skip more files, so that you aren't > advertising stuff that can't pass open?Yes, that's a bug. Also we should skip filenames containg ':' anywhere.> >+/* Since clients that want multi-conn should all pass the same export > >+ * name, this is safe. > >+ */ > >+static int > >+ondemand_can_multi_conn (void *handle) > >+{ > >+ return 1; > >+} > > Hmm. Are you permitting multiple clients to the same export, or did > you decide that was too likely to cause fs corruption, and locking > out users on the same export? The docs above said otherwise, making > this look wrong.So I think this is wrong because my locking implementation will prevent the (single) client from connecting multiple times. That's a bug: we should allow the same client to connect multiple times, which I believe is safe. However that requires client UUID ... Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v