August 2019 - Libguestfs - libguestfs List Archives

[nbdkit PATCH] offset: Better handling of parameters
by Eric Blake 29 Aug '19

29 Aug '19

The man page claims both offset and range are optional (matching the code), but the --help text claims offset is mandatory, and the comment to the no-op offset_config_complete claims we require both parameters. We did not check for an offset larger than the underlying size when there was no range, and even when there is a range, we were not careful about integer overflow (offset=5E range=5E happily claims to export a 5E image; but all bets are off if you later try to access beyond the real underlying size). And in several cases, such as if the plugin's get_size fails but range was not provided, we are not returning -1 for failure. Fixes: 3db69f56 Signed-off-by: Eric Blake <eblake(a)redhat.com> --- The first post-1.14 bugfix. Too bad I didn't spot it earlier today. filters/offset/offset.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/filters/offset/offset.c b/filters/offset/offset.c index 7df1ed13..1039a577 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -64,16 +64,9 @@ offset_config (nbdkit_next_config *next, void *nxdata, return next (nxdata, key, value); } -/* Check the user did pass both parameters. */ -static int -offset_config_complete (nbdkit_next_config_complete *next, void *nxdata) -{ - return next (nxdata); -} - #define offset_config_help \ - "offset=<OFFSET> (required) The start offset to serve.\n" \ - "range=<LENGTH> The total size to serve." + "offset=<OFFSET> The start offset to serve (default 0).\n" \ + "range=<LENGTH> The total size to serve (default rest of file)." /* Get the file size. */ static int64_t @@ -82,16 +75,23 @@ offset_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, { int64_t real_size = next_ops->get_size (nxdata); + if (real_size == -1) + return -1; + if (range >= 0) { - if (offset + range > real_size) { + if (offset > real_size - range) { nbdkit_error ("offset+range is larger than the real size " "of the underlying file or device"); return -1; } return range; } - else - return real_size - offset; + else if (offset > real_size) { + nbdkit_error ("offset is larger than the real size " + "of the underlying file or device"); + return -1; + } + return real_size - offset; } /* Read data. */ @@ -176,7 +176,6 @@ static struct nbdkit_filter filter = { .longname = "nbdkit offset filter", .version = PACKAGE_VERSION, .config = offset_config, - .config_complete = offset_config_complete, .config_help = offset_config_help, .get_size = offset_get_size, .pread = offset_pread, -- 2.21.0

2 1

ANNOUNCE: libnbd 1.0 & nbdkit 1.14 - high performance NBD client and server
by Richard W.M. Jones 28 Aug '19

28 Aug '19

I'm pleased to announce the joint release of libnbd 1.0 and nbdkit 1.14. These are a high performance NBD client library and server. Key features of libnbd: * Synchronous API for ease of use. * Asynchronous API for writing non-blocking, multithreaded clients. You can mix both APIs freely. * High performance. * Minimal dependencies for the basic library. * Well-documented, stable API. * Bindings in several programming languages. * Shell (nbdsh) for command line and scripting. Git: https://github.com/libguestfs/libnbd Download: http://download.libguestfs.org/libnbd/1.0-stable/ Fedora: https://koji.fedoraproject.org/koji/packageinfo?packageID=28807 Key features of nbdkit: * Multithreaded NBD server written in C with good performance. * Minimal dependencies for the basic server. * Liberal license (BSD) allows nbdkit to be linked to proprietary libraries or included in proprietary code. * Well-documented, simple plugin API with a stable ABI guarantee. Lets you export “unconventional” block devices easily. * You can write plugins in C, Lua, Perl, Python, OCaml, Ruby, Rust, shell script or Tcl. * Filters can be stacked in front of plugins to transform the output. Git: https://github.com/libguestfs/nbdkit Download: http://download.libguestfs.org/nbdkit/1.14-stable/ Fedora: https://koji.fedoraproject.org/koji/packageinfo?packageID=16469 Release notes for nbdkit 1.14: Server performance improvements: Nagle's algorithm is disabled; instead MSG_MORE is used to control when outgoing packets are sent. Ramdisk plugins now support more efficient zeroing. (Eric Blake). Plugins can now select their thread model at run time. Thread model selection is not finalized until after the config stage (Eric Blake). The server supports NBD_CMD_CACHE for prefetching. Plugins and filters have been updated to implement this where it makes sense (Eric Blake). Low level pthread lock failures now call abort(). This should never happen in real situations (Eric Blake). The server will not advertize multi-conn support if the internal thread model is serialized, avoiding a possible deadlock in some clients (Eric Blake). New server option ‘--no-sr’ can be used to disable structured replies (Eric Blake). The server will now refuse to start if stdin/stdout/stderr are closed, to avoid potential issues with file descriptor handling in plugins (Eric Blake). ‘$uri’ can be used to specify an NBD URI in ‘--run’ parameters (Eric Blake). New ‘stats’ filter displays elapsed statistics about NBD operations, such as the number of bytes read and written. New ‘nocache’ filter which disables cache requests, used to determine how effective they are (Eric Blake). New ‘noparallel’ filter which can be used to override the plugin’s own choice of thread model. Used to determine how the thread model affects performance, or to serialize plugins if required (Eric Blake). New ‘cacheextents’ filter to cache extents requests, especially useful with VDDK which has a slow implementation of extents (Martin Kletzander). In the ‘full’, ‘memory’, ‘null’, ‘pattern’ and ‘random’ plugins, the size= prefix can be omitted, ie: nbdkit memory 1G (instead of size=1G) The ‘nbd’ plugin has multiple enhancements: It may be built using libnbd instead of constructing raw NBD packets; NBD_OPT_GO is supported; add structured reads; implement NBD_CMD_BLOCK_STATUS; support TCP sockets; forward NBD_CMD_CACHE; retry connections; shared connections; magic ‘uri’ parameter; TLS support (Eric Blake). The ‘vddk’ plugin now reports hole extents correctly when using the ‘single-link’ flag (Martin Kletzander). The ‘cache’ and ‘cow’ filters now avoid copying data through a bounce buffer when it is already sufficiently aligned (Eric Blake). Filters (such as the delay and rate filter) which sleep no longer cause long delays when the server is shut down (Eric Blake). Multiple fixes to the ‘rust’ plugin (Martin Kletzander). Multiple enhancements and clean ups to the test suite which should make tests better and more reliable. OCaml plugins can now use ‘parse_size’, ‘parse_bool’, ‘read_password’ calls, and there is also a new example plugin. On platforms which lack atomic O_CLOEXEC support the most serialized thread model is always selected to avoid leaking file descriptors (Eric Blake). -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

1 0

[PATCH nbdkit] freebsd: In nbdkit_nanosleep, fallback to calling nanosleep(2).
by Richard W.M. Jones 28 Aug '19

28 Aug '19

Rather than failing to compile on platforms which lack POLLRDHUP such as FreeBSD, simply fallback to the old method of sleeping. This leaves the porting suggestions as a comment in case someone wants to implement a better solution for particular platforms. --- server/public.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/server/public.c b/server/public.c index a992df1..630de9b 100644 --- a/server/public.c +++ b/server/public.c @@ -304,6 +304,16 @@ nbdkit_realpath (const char *path) int nbdkit_nanosleep (unsigned sec, unsigned nsec) { + struct timespec ts; + + if (sec >= INT_MAX - nsec / 1000000000) { + nbdkit_error ("sleep request is too long"); + errno = EINVAL; + return -1; + } + ts.tv_sec = sec + nsec / 1000000000; + ts.tv_nsec = nsec % 1000000000; + #if defined HAVE_PPOLL && defined POLLRDHUP /* End the sleep early if any of these happen: * - nbdkit has received a signal to shut down the server @@ -311,7 +321,6 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) * NBD_CMD_DISC or a problem with the connection * - the input socket detects POLLRDHUP/POLLHUP/POLLERR */ - struct timespec ts; struct connection *conn = threadlocal_get_conn (); struct pollfd fds[] = { [0].fd = quit_fd, @@ -323,14 +332,6 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) }; sigset_t all; - if (sec >= INT_MAX - nsec / 1000000000) { - nbdkit_error ("sleep request is too long"); - errno = EINVAL; - return -1; - } - ts.tv_sec = sec + nsec / 1000000000; - ts.tv_nsec = nsec % 1000000000; - /* Block all signals to this thread during the poll, so we don't * have to worry about EINTR */ @@ -354,9 +355,13 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) nbdkit_error ("aborting sleep to shut down"); errno = ESHUTDOWN; return -1; + #else -# error "Please port this to your platform" - /* Porting ideas, in order of preference: + /* The fallback path simply calls ordinary nanosleep, and will + * cause long delays on server shutdown. + * + * If however you want to port this to your platform, then + * porting ideas, in order of preference: * - POSIX requires pselect; it's a bit clunkier to set up than poll, * but the same ability to atomically mask all signals and operate * on struct timespec makes it similar to the preferred ppoll interface @@ -364,8 +369,13 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec) * a recalculation of the timeout to still reach the end time (masking * signals in that case is not safe, as it is a non-atomic race) */ - nbdkit_error ("nbdkit_nanosleep not yet ported to systems without ppoll"); - errno = ENOSYS; - return -1; + int r; + + r = nanosleep (&ts, NULL); + if (r == -1 && errno != EINTR && errno != EAGAIN) { + nbdkit_error ("nanosleep: %m"); + return -1; + } + return 0; #endif } -- 2.22.0

2 2

[nbdkit PATCH] offset, partition: Fix .extents with non-zero offset
by Eric Blake 28 Aug '19

28 Aug '19

When querying the extents of the underlying plugin, we should only translate the starting offset, and let the plugin report for at least as many bytes as our range permits. Otherwise, short-changing the range causes bad behavior such as returning 0 extents, or even failing the creation of an extents tracker: $ cat script case "$1" in get_size) echo 1m;; can_extents) ;; extents) echo 0 1m;; *) exit 2 ;; esac $ nbdkit -U - --filter=offset sh script offset=64k \ --run 'qemu-io -r -f raw -c map $nbd' nbdkit: sh[1]: error: extents: plugin must return at least one extent 896 KiB (0xe0000) bytes allocated at offset 0 bytes (0x0) nbdkit: sh[1]: error: extents: plugin must return at least one extent qemu-io: Failed to get allocation status: Invalid argument $ nbdkit -U - --filter=offset sh script offset=640k \ --run 'qemu-io -r -f raw -c map $nbd' nbdkit: sh[1]: error: nbdkit_extents_new: start (655360) >= end (393216) qemu-io: Failed to get allocation status: Invalid argument Fixes: 1a5e2d9c, 624abb36 Signed-off-by: Eric Blake <eblake(a)redhat.com> --- At least we're catching this now before 1.14. docs/nbdkit-filter.pod | 2 +- filters/offset/offset.c | 4 ++-- filters/partition/partition.c | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 6e2bea61..cfd664eb 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -560,7 +560,7 @@ from the layer below. Without error checking it would look like this: int64_t size; size = next_ops->get_size (nxdata); - extents2 = nbdkit_extents_new (offset + shift, size - shift); + extents2 = nbdkit_extents_new (offset + shift, size); next_ops->extents (nxdata, count, offset + shift, flags, extents2, err); for (i = 0; i < nbdkit_extents_count (extents2); ++i) { e = nbdkit_get_extent (extents2, i); diff --git a/filters/offset/offset.c b/filters/offset/offset.c index efe5c6d1..00122770 100644 --- a/filters/offset/offset.c +++ b/filters/offset/offset.c @@ -140,9 +140,9 @@ offset_extents (struct nbdkit_next_ops *next_ops, void *nxdata, size_t i; CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL; struct nbdkit_extent e; - int64_t real_size = next_ops->get_size (nxdata); + int64_t real_size = range >= 0 ? offset + range : next_ops->get_size (nxdata); - extents2 = nbdkit_extents_new (offs + offset, real_size - offset); + extents2 = nbdkit_extents_new (offs + offset, real_size); if (extents2 == NULL) { *err = errno; return -1; diff --git a/filters/partition/partition.c b/filters/partition/partition.c index 56ad05e2..b1b1945d 100644 --- a/filters/partition/partition.c +++ b/filters/partition/partition.c @@ -230,9 +230,8 @@ partition_extents (struct nbdkit_next_ops *next_ops, void *nxdata, size_t i; CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL; struct nbdkit_extent e; - int64_t real_size = next_ops->get_size (nxdata); - extents2 = nbdkit_extents_new (offs + h->offset, real_size - h->offset); + extents2 = nbdkit_extents_new (offs + h->offset, h->offset + h->range); if (extents2 == NULL) { *err = errno; return -1; -- 2.21.0

2 1

[PATCH nbdkit] server: Try hard to maintain invariant that fds 0, 1 and 2 are always open.
by Richard W.M. Jones 27 Aug '19

27 Aug '19

https://www.redhat.com/archives/libguestfs/2019-August/thread.html#00347 Thanks: Eric Blake and Daniel P. Berrangé --- common/utils/utils.h | 1 + server/connections.c | 4 ++-- server/crypto.c | 5 +++-- server/main.c | 23 +++++++++++++++++++++++ common/utils/utils.c | 29 +++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/common/utils/utils.h b/common/utils/utils.h index ebd5f66..a77d2cd 100644 --- a/common/utils/utils.h +++ b/common/utils/utils.h @@ -38,5 +38,6 @@ extern void uri_quote (const char *str, FILE *fp); extern int exit_status_to_nbd_error (int status, const char *cmd); extern int set_cloexec (int fd); extern int set_nonblock (int fd); +extern void close_or_nullify_fd (int fd); #endif /* NBDKIT_UTILS_H */ diff --git a/server/connections.c b/server/connections.c index c173df8..f57ab3e 100644 --- a/server/connections.c +++ b/server/connections.c @@ -489,7 +489,7 @@ static void raw_close (struct connection *conn) { if (conn->sockin >= 0) - close (conn->sockin); + close_or_nullify_fd (conn->sockin); if (conn->sockout >= 0 && conn->sockin != conn->sockout) - close (conn->sockout); + close_or_nullify_fd (conn->sockout); } diff --git a/server/crypto.c b/server/crypto.c index 9cd1bb0..6f97f2c 100644 --- a/server/crypto.c +++ b/server/crypto.c @@ -47,6 +47,7 @@ #include <assert.h> #include "internal.h" +#include "utils.h" #ifdef HAVE_GNUTLS @@ -404,9 +405,9 @@ crypto_close (struct connection *conn) gnutls_bye (session, GNUTLS_SHUT_RDWR); if (sockin >= 0) - close (sockin); + close_or_nullify_fd (sockin); if (sockout >= 0 && sockin != sockout) - close (sockout); + close_or_nullify_fd (sockout); gnutls_deinit (session); conn->crypto_session = NULL; diff --git a/server/main.c b/server/main.c index 65025a6..65bef30 100644 --- a/server/main.c +++ b/server/main.c @@ -60,6 +60,7 @@ static struct backend *open_filter_so (struct backend *next, size_t i, const cha static void start_serving (void); static void write_pidfile (void); static bool is_config_key (const char *key, size_t len); +static void open_std_file_descriptors (void); struct debug_flag *debug_flags; /* -D */ bool exit_with_parent; /* --exit-with-parent */ @@ -149,6 +150,13 @@ main (int argc, char *argv[]) size_t i; const char *magic_config_key; + /* Ensures that fds 0, 1 and 2 are open (on /dev/null if nothing + * else). This is so that nbdkit and plugins can assume these file + * descriptors are always open, which makes certain code easier to + * write. + */ + open_std_file_descriptors (); + threadlocal_init (); /* The default setting for TLS depends on whether we were @@ -930,3 +938,18 @@ is_config_key (const char *key, size_t len) return true; } + +static void +open_std_file_descriptors (void) +{ + int fds[] = { STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO }; + int i, fd, fl; + + for (i = 0; i < sizeof fds / sizeof fds[0]; ++i) { + fd = fds[i]; + fl = fcntl (fd, F_GETFL, NULL); + if (fl == -1 && errno == EBADF) + /* This is best effort - don't fail. */ + open ("/dev/null", fd == STDIN_FILENO ? O_RDONLY : O_WRONLY); + } +} diff --git a/common/utils/utils.c b/common/utils/utils.c index 32bc96a..c818d4c 100644 --- a/common/utils/utils.c +++ b/common/utils/utils.c @@ -190,3 +190,32 @@ set_nonblock (int fd) { } return fd; } + +/* Calls close (fd). As a special case if fd is stdin/stdout/stderr + * then it reopens the file descriptor on /dev/null. This allows us + * to maintain certain invariants for nbdkit and plugins. In all + * cases this ignores errors. + */ +void +close_or_nullify_fd (int fd) +{ + int flags; + int nfd; + + close (fd); + + if (fd == STDIN_FILENO) + flags = O_RDONLY; + else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) + flags = O_WRONLY; + else + return; + + nfd = open ("/dev/null", flags); + if (nfd == -1) + return; + if (nfd != fd) { + dup2 (nfd, fd); + close (nfd); + } +} -- 2.22.0

2 1

[nbdkit PATCH] server: Enforce sane stdin/out/err
by Eric Blake 27 Aug '19

27 Aug '19

Refuse to run if stdin/out/err start life closed. stdin/out are essential when using -s (if they aren't present, we want to fail fast), and should remain unused otherwise (but ensuring they are open means we don't have to worry about other fd creation events accidentally colliding). We also want to ensure that nothing opens into the slot for stderr, as any error message we produce might then corrupt the wrong file. https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/xstdopen.texi is an interesting read on the matters of dealing with closed fds; but as we are unable to use gnulib for licensing reasons, we'll just roll our own solution. Note that checking that we have valid fds on entry covers most cases, but there is still one more situation to worry about (in fact, the one that triggered this patch in the first place), namely, when -s implies that our socket is fd 0 and 1, but we are executing cleanup code after closing the connection to the client. For a simple script ("case $1 in get_size) echo 1m; *) exit 2;; esac"), the following program would fail an assertion, because the .close callback of the sh plugin ended up with a pipe() call unexpectedly re-establishing stdin/out in the parent process: $ nbdsh -c 'h.connect_command (["nbdkit","sh","script", "-s","--exit-with-parent"])' -c 'print("%r"%h.get_size())' 1048576 nbdkit: call.c:155: call3: Assertion `in_fd[0] > STDERR_FILENO && in_fd[1] > STDERR_FILENO && out_fd[0] > STDERR_FILENO && out_fd[1] > STDERR_FILENO && err_fd[0] > STDERR_FILENO && err_fd[1] > STDERR_FILENO' failed. With this patch, the assertion can remain in place. Reported-by: Richard W.M. Jones <rjones(a)redhat.com> Signed-off-by: Eric Blake <eblake(a)redhat.com> --- This is my counter-proposal patch which leaves the assertion in the sh plugin untouched, without any fd shuffling, by instead ensuring that nbdkit never leaves 0/1/2 vacant. server/internal.h | 1 + server/connections.c | 12 ++++++++++++ server/main.c | 11 +++++++++++ 3 files changed, 24 insertions(+) diff --git a/server/internal.h b/server/internal.h index 22e13b6d..a9692bbc 100644 --- a/server/internal.h +++ b/server/internal.h @@ -92,6 +92,7 @@ extern bool no_sr; extern const char *port; extern bool readonly; extern const char *run; +extern bool listen_stdin; extern const char *selinux_label; extern int threads; extern int tls; diff --git a/server/connections.c b/server/connections.c index c173df8d..0184afea 100644 --- a/server/connections.c +++ b/server/connections.c @@ -366,6 +366,18 @@ free_connection (struct connection *conn) threadlocal_set_conn (NULL); conn->close (conn); + if (listen_stdin) { + int fd; + + /* Restore something to stdin/out so the rest of our code can + * continue to assume that all new fds will be above stderr. + * Swap directions to get EBADF on improper use of stdin/out. + */ + fd = open ("/dev/null", O_WRONLY | O_CLOEXEC); + assert (fd == 0); + fd = open ("/dev/null", O_RDONLY | O_CLOEXEC); + assert (fd == 1); + } /* Don't call the plugin again if quit has been set because the main * thread will be in the process of unloading it. The plugin.unload diff --git a/server/main.c b/server/main.c index 65025a62..124e19b7 100644 --- a/server/main.c +++ b/server/main.c @@ -149,6 +149,17 @@ main (int argc, char *argv[]) size_t i; const char *magic_config_key; + /* Refuse to run if stdin/out/err are closed, whether or not -s is used. */ + if (fcntl (STDERR_FILENO, F_GETFL) == -1) { + /* Nowhere we can report the error. Oh well. */ + exit (EXIT_FAILURE); + } + if (fcntl (STDIN_FILENO, F_GETFL) == -1 || + fcntl (STDOUT_FILENO, F_GETFL) == -1) { + perror ("expecting stdin/stdout to be opened"); + exit (EXIT_FAILURE); + } + threadlocal_init (); /* The default setting for TLS depends on whether we were -- 2.21.0

1 0

[PATCH disk-sync 0/5] Misc cleanups and convert inner loop to asynch.
by Richard W.M. Jones 27 Aug '19

27 Aug '19

This is based on top of: https://github.com/nertpinx/v2v-conversion-host/commit/0bb2efdcacd975a2cae7… The first 4 patches are fairly uncontroversial miscellaneous cleanups. Patch 5 is the interesting one. (Note it doesn't quite work yet, so it's for discussion only.) Patch 5 converts the inner loop to use asynchronous libnbd calls. performance improves quite a bit for me -- about 13 minutes down to 5 minutes for an "initial" run on a moderate sized Linux VM. We do this by changing the read call from nbd.pread to nbd.aio_pread and moving the writing code into a completion callback which runs when the NBD_CMD_READ has been executed by nbdkit. The problem with this patch, which is why I say it's for discussion only, is that we need to change it to throttle the number of commands in flight. Issuing large numbers of commands isn't in itself a problem. However with each command is an associated NBD.Buffer, and so the effect at the moment is that we need to allocate enough memory up front to store the whole disk image(!) By throttling the commands we can control exactly how much memory is used, and indeed control the trade-off between memory and parallelism. I checked the MD5 of the disk before and after and they were unchanged (my VM is turned off). Rich.

2 9

[nbdkit PATCH v2 00/17] fd leak safety
by Eric Blake 27 Aug '19

27 Aug '19

This is a major rewrite compared to my v1 series, where I've tried a lot harder to ensure that we still accommodate building on Haiku (although I have not actually yet fired up a Haiku VM to try it for myself). I also managed to make the sh plugin fully parallel, on capable platforms. See also my question on patch 10 on whether I've picked the best naming convention. Eric Blake (17): maint: Rename server/utils.c to server/public.c plugins: Expose more thread_model details in --dump-plugin server: Add utility functions for setting CLOEXEC and NONBLOCK Revert "RHEL 5: Define O_CLOEXEC and SOCK_CLOEXEC." build: Audit existing use of SOCK_CLOEXEC cow, cache: Better mkostemp fallback build: Audit for use of pipe2 rate: Atomically set CLOEXEC on fds server: Use atomic CLOEXEC for nbdkit_read_password plugins: Add .fork_safe field server: Atomically set CLOEXEC on accept fds filters: Set CLOEXEC on files opened during .config python: Use CLOEXEC on script sh: Use pipe2 with CLOEXEC when possible sh: Enable parallel thread model, when possible sh: Test for fd leaks temp debug docs/nbdkit-plugin.pod | 32 ++++-- plugins/sh/nbdkit-sh-plugin.pod | 10 +- configure.ac | 2 + common/utils/utils.h | 2 + include/nbdkit-plugin.h | 1 + server/internal.h | 14 +-- common/utils/utils.c | 64 ++++++++++++ filters/cache/blk.c | 12 ++- filters/cow/blk.c | 12 ++- filters/log/log.c | 22 ++++- filters/rate/rate.c | 20 +++- filters/stats/stats.c | 21 +++- filters/xz/xzfile.c | 4 - plugins/curl/curl.c | 1 + plugins/data/data.c | 1 + plugins/example2/example2.c | 5 +- plugins/example3/example3.c | 1 + plugins/ext2/ext2.c | 1 + plugins/file/file.c | 5 +- plugins/floppy/floppy.c | 1 + plugins/full/full.c | 1 + plugins/guestfs/guestfs-plugin.c | 1 + plugins/gzip/gzip.c | 1 + plugins/iso/iso.c | 1 + plugins/libvirt/libvirt-plugin.c | 1 + plugins/linuxdisk/linuxdisk.c | 1 + plugins/memory/memory.c | 1 + plugins/nbd/nbd-standalone.c | 1 + plugins/nbd/nbd.c | 32 ++++++ plugins/null/null.c | 1 + plugins/partitioning/partitioning.c | 1 + plugins/pattern/pattern.c | 1 + plugins/python/python.c | 18 +++- plugins/random/random.c | 1 + plugins/sh/call.c | 34 ++++++- plugins/sh/sh.c | 9 +- plugins/split/split.c | 5 +- plugins/ssh/ssh.c | 1 + plugins/streaming/streaming.c | 5 +- plugins/zero/zero.c | 1 + server/locks.c | 4 +- server/plugins.c | 63 ++++++++---- server/{utils.c => public.c} | 19 +++- server/quit.c | 18 ++++ server/sockets.c | 40 +++++++- server/{test-utils.c => test-public.c} | 0 tests/test-layers.c | 4 +- tests/test-socket-activation.c | 12 +-- tests/test-streaming.c | 5 +- tests/web-server.c | 7 +- .gitignore | 2 +- server/Makefile.am | 18 ++-- tests/Makefile.am | 2 + tests/test-dump-plugin.sh | 32 +++++- tests/test-parallel-file.sh | 3 + tests/test-parallel-nbd.sh | 5 +- tests/test-parallel-sh.sh | 131 +++++++++++++++++++++++++ 57 files changed, 605 insertions(+), 108 deletions(-) rename server/{utils.c => public.c} (94%) rename server/{test-utils.c => test-public.c} (100%) create mode 100755 tests/test-parallel-sh.sh -- 2.20.1

2 31

[nbdkit PATCH] filters: Bump API version
by Eric Blake 27 Aug '19

27 Aug '19

We do not promise API compatibility for filters between stable releases of nbdkit, however, we should at least ensure that when we do break API, that we refuse to load a filter compiled against one version of nbdkit with another server running a different API. A single bump once per stable release is good enough (rather than once per API change). We did this correctly for commits b0ce4411/cb309687/df0cc21d (bumping to API version 2 for the combined changes between v1.2 and v1.4), but failed to do so for f184fdc3 (affecting v1.10), 4ca66f70 (affecting v1.12), or 5ee7bd29/ee61d232 (affecting the upcoming v1.14). So do it retroacively now, as well as backporting intermediate bumps to affected stable branches. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm pushing this soon. include/nbdkit-filter.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 94f17789..1ebd1cb6 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -43,7 +43,7 @@ extern "C" { #endif -#define NBDKIT_FILTER_API_VERSION 2 +#define NBDKIT_FILTER_API_VERSION 5 struct nbdkit_extent { uint64_t offset; @@ -100,8 +100,10 @@ struct nbdkit_filter { */ int _api_version; - /* Because there is no ABI guarantee, new fields may be added - * where logically appropriate. */ + /* Because there is no ABI guarantee, new fields may be added where + * logically appropriate, as long as we correctly bump + * NBDKIT_FILTER_API_VERSION once per stable release. + */ const char *name; const char *longname; const char *version; -- 2.21.0

2 2

[nbdkit PATCH] tests: Test for faster shutdown
by Eric Blake 23 Aug '19

23 Aug '19

The test relies on the timeout program. Also, since the nbdkit_nanosleep code relies on the Linux extension POLLRDHUP to detect early client closure, we may have to relax that part of the test when porting to platforms that lack ppoll/POLLRDHUP. (That is, while we should still be able to let a signal to the server shut down nbdkit quickly, it's harder to let a client close()ing its end cause nbdkit to shut down a single connection quickly when limited to a poll that can't identify EOF as a way to break a poll). Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm pushing this along with the nbdkit_nanosleep patches, now that I'm reliably getting the test to fail in isolation and to pass with the sleep patches applied. However, I suspect that we will definitely need to fix things for other platforms, and/or skip this test if we can't easily make other platforms match what Linux can do with POLLRDHUP. tests/Makefile.am | 4 ++- tests/test-shutdown.sh | 76 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100755 tests/test-shutdown.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index 3d78e7a2..bc308c5d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -110,6 +110,7 @@ EXTRA_DIST = \ test-rate-dynamic.sh \ test.rb \ test-readahead-copy.sh \ + test-shutdown.sh \ test-ssh.sh \ test.tcl \ test-shebang-perl.sh \ @@ -955,7 +956,8 @@ if HAVE_GUESTFISH TESTS += test-cow.sh endif HAVE_GUESTFISH -# delay filter test. +# delay filter tests. +TESTS += test-shutdown.sh LIBGUESTFS_TESTS += test-delay test_delay_SOURCES = test-delay.c test.h diff --git a/tests/test-shutdown.sh b/tests/test-shutdown.sh new file mode 100755 index 00000000..0ac9c1eb --- /dev/null +++ b/tests/test-shutdown.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019 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 -x + +requires qemu-io --version +requires timeout --version + +sock=`mktemp -u` +files="shutdown.pid $sock" +cleanup_fn rm -f $files +fail=0 + +# Create a server that delays reads and forces only one connection at a time. +# This tests that the delay filter's use of nbdkit_nanosleep is able to +# react to both connection death and server shutdown without finishing +# the entire delay duration. +start_nbdkit -P shutdown.pid -U $sock --filter=noparallel --filter=delay \ + null 1M serialize=connections rdelay=10 + +# Early client death should not stall connection of second client. +trap '' ERR +timeout 1s qemu-io -f raw "nbd+unix:///?socket=$sock" -c 'r 0 512' </dev/null +test $? = 124 || { + echo "Unexpected status; qemu-io should have been killed for timing out" + fail=1 +} +timeout 1s qemu-io -f raw "nbd+unix:///?socket=$sock" -c 'quit' </dev/null +test $? = 0 || { + echo "Unexpected status; nbdkit was not responsive to allow second qemu-io" + fail=1 +} + +# The server's response to shutdown signals should not stall on delays +qemu-io -f raw "nbd+unix:///?socket=$sock" -c 'r 0 512' </dev/null & +pid=$! +sleep 1 +kill -s INT "$(cat "$pidfile")" +sleep 1 +kill -s 0 "$(cat "$pidfile")" && { + echo "Unexpected status; nbdkit didn't react fast enough to signal" + fail=1 +} +wait $pid + +exit $fail -- 2.20.1

1 1