[nbdkit PATCH] offset: Better handling of parameters
by Eric Blake
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
4 years, 8 months
ANNOUNCE: libnbd 1.0 & nbdkit 1.14 - high performance NBD client and server
by Richard W.M. Jones
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
4 years, 8 months
[PATCH nbdkit] freebsd: In nbdkit_nanosleep, fallback to calling nanosleep(2).
by Richard W.M. Jones
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
4 years, 8 months
[nbdkit PATCH] offset, partition: Fix .extents with non-zero offset
by Eric Blake
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
4 years, 8 months
[PATCH nbdkit] server: Try hard to maintain invariant that fds 0, 1 and 2 are always open.
by Richard W.M. Jones
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
4 years, 8 months
[nbdkit PATCH] server: Enforce sane stdin/out/err
by Eric Blake
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
4 years, 8 months
[PATCH disk-sync 0/5] Misc cleanups and convert inner loop to asynch.
by Richard W.M. Jones
This is based on top of:
https://github.com/nertpinx/v2v-conversion-host/commit/0bb2efdcacd975a2ca...
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.
4 years, 8 months
[nbdkit PATCH v2 00/17] fd leak safety
by Eric Blake
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
4 years, 8 months
[nbdkit PATCH] filters: Bump API version
by Eric Blake
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
4 years, 8 months
[nbdkit PATCH] tests: Test for faster shutdown
by Eric Blake
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
4 years, 9 months