rpms/nbdkit
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix parsing of delay-* options

Browse Source 
resolves: rhbz#1991649
Fix assertion failure during server shutdown
Fix delay-close option
  resolves: rhbz#1991652
tests/test-debug-flags.sh: Don't use port 10809 during test
  resolves: rhbz#1991945
This commit is contained in:
Richard W.M. Jones 2021-08-10 16:09:59 +01:00
parent 31f0b80356
commit 704d2b18ae
12 changed files with 1098 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

157
0012-server-Make-debug-messages-atomic.patch Normal file
View File

@ -0,0 +1,157 @@
From 453a7611b625fc6f306a47ccc61bf3b83d75e522 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 5 Aug 2021 10:13:53 +0100
Subject: [PATCH] server: Make debug messages atomic
Previously we used flockfile around the separate fprintf statements
used to emit debug messages. However this does not make debug
messages atomic (only atomic with respect to other nbdkit threads).
It's easy to see this in strace:
1915655 write(2, "nbdkit: ", 8) = 8
1915655 write(2, "debug: ", 7) = 7
1915655 write(2, "nbdkit 1.27.3 (nbdkit-1.27.3-1.f"..., 36) = 36
1915655 write(2, "\n", 1) = 1
We require open_memstream already, so use this to print messages to a
string buffer which we can emit in a single write.
This fixes various tests which grep the log file looking for sentinel
messages. In particular a recent Fedora build failure in
test-nbdkit-backend-debug.sh was caused by this, but there are other
tests that fail occasionally for the same reason.
This change also tries harder to set errno correctly before expanding
the format string (for debug messages that contain %m).
Note that libnbd already emits atomic messages.
(cherry picked from commit 30eef3bf2d93b12072f91f95987bae33f9e3fe1a)
---
server/debug.c | 72 ++++++++++++++++++++++++++++++++------------------
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/server/debug.c b/server/debug.c
index eede5e16..07710581 100644
--- a/server/debug.c
+++ b/server/debug.c
@@ -40,23 +40,22 @@
#include "internal.h"
-/* Called with flockfile (stderr) taken. */
static void
-prologue (void)
+prologue (FILE *fp)
{
const char *name = threadlocal_get_name ();
size_t instance_num = threadlocal_get_instance_num ();
- fprintf (stderr, "%s: ", program_name);
+ fprintf (fp, "%s: ", program_name);
if (name) {
- fprintf (stderr, "%s", name);
+ fprintf (fp, "%s", name);
if (instance_num > 0)
- fprintf (stderr, "[%zu]", instance_num);
- fprintf (stderr, ": ");
+ fprintf (fp, "[%zu]", instance_num);
+ fprintf (fp, ": ");
}
- fprintf (stderr, "debug: ");
+ fprintf (fp, "debug: ");
}
/* Note: preserves the previous value of errno. */
@@ -64,20 +63,30 @@ NBDKIT_DLL_PUBLIC void
nbdkit_vdebug (const char *fs, va_list args)
{
int err = errno;
+ CLEANUP_FREE char *str = NULL;
+ size_t len = 0;
+ FILE *fp;
if (!verbose)
return;
-#ifdef HAVE_FLOCKFILE
- flockfile (stderr);
-#endif
- prologue ();
- vfprintf (stderr, fs, args);
+ fp = open_memstream (&str, &len);
+ if (fp == NULL) {
+ /* Try to emit what we can. */
+ errno = err;
+ vfprintf (stderr, fs, args);
+ return;
+ }
- fprintf (stderr, "\n");
-#ifdef HAVE_FUNLOCKFILE
- funlockfile (stderr);
-#endif
+ prologue (fp);
+
+ errno = err;
+ vfprintf (fp, fs, args);
+
+ fprintf (fp, "\n");
+ fclose (fp);
+
+ fputs (str, stderr);
errno = err;
}
@@ -86,25 +95,36 @@ nbdkit_vdebug (const char *fs, va_list args)
NBDKIT_DLL_PUBLIC void
nbdkit_debug (const char *fs, ...)
{
+ int err = errno;
va_list args;
- int err = errno;
+ CLEANUP_FREE char *str = NULL;
+ size_t len = 0;
+ FILE *fp;
if (!verbose)
return;
-#ifdef HAVE_FLOCKFILE
- flockfile (stderr);
-#endif
- prologue ();
+ fp = open_memstream (&str, &len);
+ if (fp == NULL) {
+ /* Try to emit what we can. */
+ va_start (args, fs);
+ errno = err;
+ vfprintf (stderr, fs, args);
+ va_end (args);
+ return;
+ }
+
+ prologue (fp);
va_start (args, fs);
- vfprintf (stderr, fs, args);
+ errno = err;
+ vfprintf (fp, fs, args);
va_end (args);
- fprintf (stderr, "\n");
-#ifdef HAVE_FUNLOCKFILE
- funlockfile (stderr);
-#endif
+ fprintf (fp, "\n");
+ fclose (fp);
+
+ fputs (str, stderr);
errno = err;
}
--
2.31.1

150
0013-cow-General-revision-and-updates-to-the-manual.patch Normal file
View File

@ -0,0 +1,150 @@
From d15e320d20260c14973ef84172ae8cbe337a2b48 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 5 Aug 2021 18:18:34 +0100
Subject: [PATCH] cow: General revision and updates to the manual
(cherry picked from commit ba5517b81307c228577cf3c54a651d044ac91a25)
---
filters/cow/nbdkit-cow-filter.pod | 74 ++++++++++++++++---------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/filters/cow/nbdkit-cow-filter.pod b/filters/cow/nbdkit-cow-filter.pod
index 01261429..7f861140 100644
--- a/filters/cow/nbdkit-cow-filter.pod
+++ b/filters/cow/nbdkit-cow-filter.pod
@@ -5,33 +5,23 @@ nbdkit-cow-filter - nbdkit copy-on-write (COW) filter
=head1 SYNOPSIS
nbdkit --filter=cow plugin [plugin-args...]
+ [cow-on-cache=false|true]
+ [cow-on-read=false|true|/PATH]
=head1 DESCRIPTION
C<nbdkit-cow-filter> is a filter that makes a temporary writable copy
-on top of a read-only plugin. It can be used to enable writes for
-plugins which only implement read-only access. Note that:
+on top of a plugin. It can also be used to enable writes for plugins
+which are read-only.
-=over 4
+The underlying plugin is opened read-only. This filter does not pass
+any writes or write-like operations (like trim and zero) through to
+the underlying plugin.
-=item *
-
-B<Anything written is thrown away as soon as nbdkit exits.>
-
-=item *
-
-All connections to the nbdkit instance see the same view of the disk.
-
-This is different from L<nbd-server(1)> where each connection sees its
-own copy-on-write overlay and simply disconnecting the client throws
-that away. It also allows us to create diffs, see below.
-
-=item *
-
-The plugin is opened read-only (as if the I<-r> flag was passed), but
-you should B<not> pass the I<-r> flag to nbdkit.
-
-=back
+B<Note that anything written is thrown away as soon as nbdkit exits.>
+If you want to save changes, either copy out the whole disk using a
+tool like L<nbdcopy(1)>, or use the method described in L</NOTES>
+below to create a diff.
Limitations of the filter include:
@@ -52,26 +42,26 @@ serve the same data to each client.
=over 4
-=item B<cow-on-cache=true>
-
-When the client issues a cache (prefetch) request, preemptively save
-the data from the plugin into the overlay.
-
=item B<cow-on-cache=false>
Do not save data from cache (prefetch) requests in the overlay. This
leaves the overlay as small as possible. This is the default.
-=item B<cow-on-read=true>
+=item B<cow-on-cache=true>
-When the client issues a read request, copy the data into the overlay
-so that the same data can be served more quickly later.
+When the client issues a cache (prefetch) request, preemptively save
+the data from the plugin into the overlay.
=item B<cow-on-read=false>
Do not save data from read requests in the overlay. This leaves the
overlay as small as possible. This is the default.
+=item B<cow-on-read=true>
+
+When the client issues a read request, copy the data into the overlay
+so that the same data can be served more quickly later.
+
=item B<cow-on-read=/PATH>
When F</PATH> (which must be an absolute path) exists, this behaves
@@ -83,18 +73,23 @@ behaviour while nbdkit is running.
=head1 EXAMPLES
+=head2 nbdkit --filter=cow file disk.img
+
Serve the file F<disk.img>, allowing writes, but do not save any
-changes into the file:
+changes into the file.
- nbdkit --filter=cow file disk.img
+=head2 nbdkit --filter=cow --filter=xz file disk.xz cow-on-read=true
L<nbdkit-xz-filter(1)> only supports read access, but you can provide
-temporary write access by doing (although this does B<not> save
-changes to the file):
+temporary write access by using the command above. Because xz
+decompression is slow, using C<cow-on-read=true> causes reads to be
+cached as well as writes, improving performance at the expense of
+using more temporary space. Note that writes are thrown away when
+nbdkit exits and do not get saved into the file.
- nbdkit --filter=cow --filter=xz file disk.xz
+=head1 NOTES
-=head1 CREATING A DIFF WITH QEMU-IMG
+=head2 Creating a diff with qemu-img
Although nbdkit-cow-filter itself cannot save the differences, it is
possible to do this using an obscure feature of L<qemu-img(1)>.
@@ -118,6 +113,14 @@ F<diff.qcow2> now contains the differences between the base
(F<disk.img>) and the changes stored in nbdkit-cow-filter. C<nbdkit>
can now be killed.
+=head2 Compared to nbd-server -c option
+
+All connections to the nbdkit instance see the same view of the disk.
+This is different from L<nbd-server(1)> I<-c> option where each
+connection sees its own copy-on-write overlay and simply disconnecting
+the client throws that away. It also allows us to create diffs as
+above.
+
=head1 ENVIRONMENT VARIABLES
=over 4
@@ -154,6 +157,7 @@ L<nbdkit-cache-filter(1)>,
L<nbdkit-cacheextents-filter(1)>,
L<nbdkit-xz-filter(1)>,
L<nbdkit-filter(3)>,
+L<nbdcopy(1)>,
L<qemu-img(1)>.
=head1 AUTHORS
--
2.31.1

35
0014-cache-Move-plugin-args-in-synopsis-earlier.patch Normal file
View File

@ -0,0 +1,35 @@
From 583d5308ea8d26248e521b76afb380432d2084bc Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 5 Aug 2021 18:20:37 +0100
Subject: [PATCH] cache: Move plugin-args in synopsis earlier
Makes this page consistent with nbdkit-cow-filter.
(cherry picked from commit f1ddcef468907b0321041b1c4e0a430be46920be)
---
filters/cache/nbdkit-cache-filter.pod | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/filters/cache/nbdkit-cache-filter.pod b/filters/cache/nbdkit-cache-filter.pod
index df9c1f99..d85fef09 100644
--- a/filters/cache/nbdkit-cache-filter.pod
+++ b/filters/cache/nbdkit-cache-filter.pod
@@ -4,13 +4,13 @@ nbdkit-cache-filter - nbdkit caching filter
=head1 SYNOPSIS
- nbdkit --filter=cache plugin [cache=writeback|writethrough|unsafe]
+ nbdkit --filter=cache plugin [plugin-args...]
+ [cache=writeback|writethrough|unsafe]
[cache-min-block-size=SIZE]
[cache-max-size=SIZE]
[cache-high-threshold=N]
[cache-low-threshold=N]
[cache-on-read=true|false|/PATH]
- [plugin-args...]
=head1 DESCRIPTION
--
2.31.1

53
0015-data-Improve-the-example-with-a-diagram.patch Normal file
View File

@ -0,0 +1,53 @@
From 72b87c985dc9324d896333f9ddaf317cece8a812 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sun, 8 Aug 2021 16:32:38 +0100
Subject: [PATCH] data: Improve the example with a diagram
And other improvements to readability.
(cherry picked from commit 4e3a9bda2b7a3d141234e26250c69baa6ed5194d)
---
plugins/data/nbdkit-data-plugin.pod | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/plugins/data/nbdkit-data-plugin.pod b/plugins/data/nbdkit-data-plugin.pod
index 32a450ab..b69435e9 100644
--- a/plugins/data/nbdkit-data-plugin.pod
+++ b/plugins/data/nbdkit-data-plugin.pod
@@ -172,18 +172,24 @@ compact format. It is a string containing a list of bytes which are
written into the disk image sequentially. You can move the virtual
offset where bytes are written using C<@offset>.
-For example:
-
nbdkit data '0 1 2 3 @0x1fe 0x55 0xaa'
-creates a 0x200 = 512 byte (1 sector) image containing the four bytes
-C<0 1 2 3> at the start, and the two bytes C<0x55 0xaa> at the end of
-the sector, with the remaining 506 bytes in the middle being all
-zeroes. In this example the size (512 bytes) is implied by the data.
-But you could additionally use the C<size> parameter to either
-truncate or extend (with zeroes) the disk image.
+creates:
-Whitespace between fields in the string is ignored.
+ total size 0x200 = 512 bytes (1 sector)
+┌──────┬──────┬──────┬──────┬───────── ── ── ───┬──────┬──────┐
+│ 0 │ 1 │ 2 │ 3 │ 0 0 ... 0 │ 0x55 │ 0xaa │
+└──────┴──────┴──────┴──────┴───────── ── ── ───┴──────┴──────┘
+ ↑
+ offset 0x1fe
+
+In this example the size is implied by the data. But you could also
+use the C<size> parameter to either truncate or extend (with zeroes)
+the disk image. Another way to write the same disk would be this,
+where we align the offset to the end of the sector and move back 2
+bytes to write the signature:
+
+ nbdkit data '0 1 2 3 @^0x200 @-2 le16:0xaa55'
Fields in the string can be:
--
2.31.1

45
0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch Normal file
View File

@ -0,0 +1,45 @@
From d2ed77d1b8fedcba3aedadeed883553886f4bb56 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 9 Aug 2021 14:09:31 +0100
Subject: [PATCH] cow: Add some more debugging especially for blk_read_multiple
and cow-on-read
Only activated when we use -D cow.verbose=1
(cherry picked from commit 2da1ae0ca966af955d8fcf3feffffc80d07142fd)
---
filters/cow/blk.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/filters/cow/blk.c b/filters/cow/blk.c
index 9d42b5fc..879f471a 100644
--- a/filters/cow/blk.c
+++ b/filters/cow/blk.c
@@ -258,8 +258,10 @@ blk_read_multiple (nbdkit_next *next,
if (cow_debug_verbose)
nbdkit_debug ("cow: blk_read_multiple block %" PRIu64
- " (offset %" PRIu64 ") is %s",
- blknum, (uint64_t) offset, state_to_string (state));
+ " (offset %" PRIu64 ") run of length %" PRIu64
+ " is %s",
+ blknum, (uint64_t) offset, runblocks,
+ state_to_string (state));
if (state == BLOCK_NOT_ALLOCATED) { /* Read underlying plugin. */
unsigned n, tail = 0;
@@ -285,6 +287,11 @@ blk_read_multiple (nbdkit_next *next,
* set them as allocated.
*/
if (cow_on_read) {
+ if (cow_debug_verbose)
+ nbdkit_debug ("cow: cow-on-read saving %" PRIu64 " blocks "
+ "at offset %" PRIu64 " into the cache",
+ runblocks, offset);
+
if (full_pwrite (fd, block, BLKSIZE * runblocks, offset) == -1) {
*err = errno;
nbdkit_error ("pwrite: %m");
--
2.31.1

74
0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch Normal file
View File

@ -0,0 +1,74 @@
From ad2b4d2c07def233b2192c2a7ff925d1b6b823e7 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 9 Aug 2021 14:48:15 +0100
Subject: [PATCH] tests/test-debug-flags.sh: Don't use port 10809 during test
Use a temporary Unix domain socket (-U -) instead.
Reported-by: Ming Xie
Fixes: commit 0e3a54f78f4ab0cbe4bee2b965ec0e610c399a6e
(cherry picked from commit 31f4e5c31825485e7bb39d170a7102ddbc4043c2)
---
tests/test-debug-flags.sh | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/tests/test-debug-flags.sh b/tests/test-debug-flags.sh
index 73be9904..2be799a1 100755
--- a/tests/test-debug-flags.sh
+++ b/tests/test-debug-flags.sh
@@ -46,7 +46,7 @@ rm -f $files
cleanup_fn rm -f $files
# This should work and show the "extra debugging" line in debug output.
-nbdkit -f -v -D example2.extra=1 example2 file=disk \
+nbdkit -U - -f -v -D example2.extra=1 example2 file=disk \
--run 'nbdinfo "$uri"' 2>debug-flags.out
cat debug-flags.out
if ! grep -sq 'extra debugging:' debug-flags.out ; then
@@ -82,33 +82,33 @@ check_warning ()
# This is expected to fail because we didn't set the file= parameter,
# but it should not fail because of the debug flag.
-nbdkit -f -D example2.extra=1 example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D example2.extra=1 example2 2>debug-flags.out && expected_failure
check_error "you must supply the file="
# This should fail because we didn't set the file= parameter, but it
# should also print a warning about the unknown -D flag.
-nbdkit -f -D example2.unknown=1 example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D example2.unknown=1 example2 2>debug-flags.out && expected_failure
check_error "you must supply the file="
check_warning "does not contain a global variable called example2_debug_unknown"
# This should fail because we didn't set the file= parameter, but it
# should also print a warning because the -D flag is unused.
-nbdkit -f -D example1.foo=1 example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D example1.foo=1 example2 2>debug-flags.out && expected_failure
check_error "you must supply the file="
check_warning "was not used"
# These should fail because the -D flag has a bad format.
-nbdkit -f -D = example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D = example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D . example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D . example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D =. example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D =. example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D .= example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D .= example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D .extra=1 example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D .extra=1 example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D example2.=1 example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D example2.=1 example2 2>debug-flags.out && expected_failure
check_error "must have the format"
-nbdkit -f -D example2.extra= example2 2>debug-flags.out && expected_failure
+nbdkit -U - -f -D example2.extra= example2 2>debug-flags.out && expected_failure
check_error "must have the format"
--
2.31.1

182
0018-delay-Improve-parsing-and-representation-of-delay-ti.patch Normal file
View File

@ -0,0 +1,182 @@
From 810c2449cb519100bf9ea50d743162a391eac873 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 9 Aug 2021 16:16:36 +0100
Subject: [PATCH] delay: Improve parsing and representation of delay times
We used sscanf to parse the delay settings for both NNms and NN
(seconds). In the first case we need to use sscanf otherwise the code
is awkward. In the second case using sscanf meant that bogus suffices
were not ignored, eg:
$ nbdkit null --filter=delay delay-open=10SECS --run 'nbdinfo $uri'
Use nbdkit_parse_unsigned instead. This now results in an error:
nbdkit: error: delay-open: could not parse number: "10SECS": trailing garbage
Also, previously we stored delay times as an int. This makes no sense
since they cannot be negative, so use an unsigned instead.
Reported-by: Ming Xie
(cherry picked from commit 58187831a4346b44e398f105163abac8f3dfb7f0)
---
filters/delay/delay.c | 73 ++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 42 deletions(-)
diff --git a/filters/delay/delay.c b/filters/delay/delay.c
index 5bd21321..df3729a7 100644
--- a/filters/delay/delay.c
+++ b/filters/delay/delay.c
@@ -42,25 +42,28 @@
#include <nbdkit-filter.h>
-static int delay_read_ms = 0; /* read delay (milliseconds) */
-static int delay_write_ms = 0; /* write delay (milliseconds) */
-static int delay_zero_ms = 0; /* zero delay (milliseconds) */
-static int delay_trim_ms = 0; /* trim delay (milliseconds) */
-static int delay_extents_ms = 0;/* extents delay (milliseconds) */
-static int delay_cache_ms = 0; /* cache delay (milliseconds) */
+static unsigned delay_read_ms = 0; /* read delay (milliseconds) */
+static unsigned delay_write_ms = 0; /* write delay (milliseconds) */
+static unsigned delay_zero_ms = 0; /* zero delay (milliseconds) */
+static unsigned delay_trim_ms = 0; /* trim delay (milliseconds) */
+static unsigned delay_extents_ms = 0;/* extents delay (milliseconds) */
+static unsigned delay_cache_ms = 0; /* cache delay (milliseconds) */
+static unsigned delay_open_ms = 0; /* open delay (milliseconds) */
+static unsigned delay_close_ms = 0; /* close delay (milliseconds) */
+
static int delay_fast_zero = 1; /* whether delaying zero includes fast zero */
-static int delay_open_ms = 0; /* open delay (milliseconds) */
-static int delay_close_ms = 0; /* close delay (milliseconds) */
static int
-parse_delay (const char *key, const char *value)
+parse_delay (const char *key, const char *value, unsigned *r)
{
size_t len = strlen (value);
- int r;
if (len > 2 && strcmp (&value[len-2], "ms") == 0) {
- if (sscanf (value, "%d", &r) == 1 && r >= 0)
- return r;
+ /* We have to use sscanf here instead of nbdkit_parse_unsigned
+ * because that function will reject the "ms" suffix.
+ */
+ if (sscanf (value, "%u", r) == 1)
+ return 0;
else {
nbdkit_error ("cannot parse %s in milliseconds parameter: %s",
key, value);
@@ -68,24 +71,19 @@ parse_delay (const char *key, const char *value)
}
}
else {
- if (sscanf (value, "%d", &r) == 1 && r >= 0) {
- if (r * 1000LL > INT_MAX) {
- nbdkit_error ("seconds parameter %s is too large: %s",
- key, value);
- return -1;
- }
- return r * 1000;
- }
- else {
- nbdkit_error ("cannot parse %s in seconds parameter: %s",
- key, value);
+ if (nbdkit_parse_unsigned (key, value, r) == -1)
+ return -1;
+ if (*r * 1000U > UINT_MAX) {
+ nbdkit_error ("seconds parameter %s is too large: %s", key, value);
return -1;
}
+ *r *= 1000;
+ return 0;
}
}
static int
-delay (int ms, int *err)
+delay (unsigned ms, int *err)
{
if (ms > 0 && nbdkit_nanosleep (ms / 1000, (ms % 1000) * 1000000) == -1) {
*err = errno;
@@ -150,14 +148,12 @@ delay_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
if (strcmp (key, "rdelay") == 0 ||
strcmp (key, "delay-read") == 0 ||
strcmp (key, "delay-reads") == 0) {
- delay_read_ms = parse_delay (key, value);
- if (delay_read_ms == -1)
+ if (parse_delay (key, value, &delay_read_ms) == -1)
return -1;
return 0;
}
else if (strcmp (key, "wdelay") == 0) {
- delay_write_ms = parse_delay (key, value);
- if (delay_write_ms == -1)
+ if (parse_delay (key, value, &delay_write_ms) == -1)
return -1;
/* Historically wdelay set all write-related delays. */
delay_zero_ms = delay_trim_ms = delay_write_ms;
@@ -165,15 +161,13 @@ delay_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
}
else if (strcmp (key, "delay-write") == 0 ||
strcmp (key, "delay-writes") == 0) {
- delay_write_ms = parse_delay (key, value);
- if (delay_write_ms == -1)
+ if (parse_delay (key, value, &delay_write_ms) == -1)
return -1;
return 0;
}
else if (strcmp (key, "delay-zero") == 0 ||
strcmp (key, "delay-zeroes") == 0) {
- delay_zero_ms = parse_delay (key, value);
- if (delay_zero_ms == -1)
+ if (parse_delay (key, value, &delay_zero_ms) == -1)
return -1;
return 0;
}
@@ -181,21 +175,18 @@ delay_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
strcmp (key, "delay-trims") == 0 ||
strcmp (key, "delay-discard") == 0 ||
strcmp (key, "delay-discards") == 0) {
- delay_trim_ms = parse_delay (key, value);
- if (delay_trim_ms == -1)
+ if (parse_delay (key, value, &delay_trim_ms) == -1)
return -1;
return 0;
}
else if (strcmp (key, "delay-extent") == 0 ||
strcmp (key, "delay-extents") == 0) {
- delay_extents_ms = parse_delay (key, value);
- if (delay_extents_ms == -1)
+ if (parse_delay (key, value, &delay_extents_ms) == -1)
return -1;
return 0;
}
else if (strcmp (key, "delay-cache") == 0) {
- delay_cache_ms = parse_delay (key, value);
- if (delay_cache_ms == -1)
+ if (parse_delay (key, value, &delay_cache_ms) == -1)
return -1;
return 0;
}
@@ -206,14 +197,12 @@ delay_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
return 0;
}
else if (strcmp (key, "delay-open") == 0) {
- delay_open_ms = parse_delay (key, value);
- if (delay_open_ms == -1)
+ if (parse_delay (key, value, &delay_open_ms) == -1)
return -1;
return 0;
}
else if (strcmp (key, "delay-close") == 0) {
- delay_close_ms = parse_delay (key, value);
- if (delay_close_ms == -1)
+ if (parse_delay (key, value, &delay_close_ms) == -1)
return -1;
return 0;
}
--
2.31.1

41
0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch Normal file
View File

@ -0,0 +1,41 @@
From 33318699bf1255aef0c6ee4863236d26d7b326ec Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 9 Aug 2021 20:11:41 +0100
Subject: [PATCH] server: Return from nbdkit_nanosleep early if the socket
closes
https://bugzilla.redhat.com/show_bug.cgi?id=1991652#c2
Reported-by: Ming Xie
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1991652
(cherry picked from commit 87a88f8c52a0d2fd392c35d37f8b048bcede1382)
---
server/public.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/server/public.c b/server/public.c
index 3362f1ab..4870e2d3 100644
--- a/server/public.c
+++ b/server/public.c
@@ -693,6 +693,8 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec)
* - the current connection is multi-threaded and another thread detects
* NBD_CMD_DISC or a problem with the connection
* - the input socket detects POLLRDHUP/POLLHUP/POLLERR
+ * - the input socket is invalid (POLLNVAL, probably closed by
+ * another thread)
*/
struct connection *conn = threadlocal_get_conn ();
struct pollfd fds[] = {
@@ -724,7 +726,8 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec)
*/
assert (quit ||
(conn && conn->nworkers > 0 && connection_get_status () < 1) ||
- (conn && (fds[2].revents & (POLLRDHUP | POLLHUP | POLLERR))));
+ (conn && (fds[2].revents & (POLLRDHUP | POLLHUP | POLLERR |
+ POLLNVAL))));
nbdkit_error ("aborting sleep to shut down");
errno = ESHUTDOWN;
return -1;
--
2.31.1

43
0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch Normal file
View File

@ -0,0 +1,43 @@
From 032531cd5d402119a81efbaf07d781123c5b02af Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 10 Aug 2021 08:30:43 +0100
Subject: [PATCH] server: nanosleep: Change error for early end of sleep
At the moment nbdkit_nanosleep gives an incorrect error message if it
aborts early. Even in the case when the server is not actually
shutting down it will say:
$ nbdkit --filter=delay null delay-close=3 --run 'nbdinfo --size $uri; nbdinfo --size $uri'
0
nbdkit: null[1]: error: aborting sleep to shut down
0
nbdkit: null[2]: error: aborting sleep to shut down
This commit changes the error so we only talk about shut down when the
server is actually shutting down, and use a different message in other
cases.
(cherry picked from commit cd24d9c418992e6f2c721c7deec70e564c23ab83)
---
server/public.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/server/public.c b/server/public.c
index 4870e2d3..d9ed0d9c 100644
--- a/server/public.c
+++ b/server/public.c
@@ -728,7 +728,10 @@ nbdkit_nanosleep (unsigned sec, unsigned nsec)
(conn && conn->nworkers > 0 && connection_get_status () < 1) ||
(conn && (fds[2].revents & (POLLRDHUP | POLLHUP | POLLERR |
POLLNVAL))));
- nbdkit_error ("aborting sleep to shut down");
+ if (quit)
+ nbdkit_error ("aborting sleep because of server shut down");
+ else
+ nbdkit_error ("aborting sleep because of connection close or error");
errno = ESHUTDOWN;
return -1;
--
2.31.1

142
0021-delay-Fix-delay-close.patch Normal file
View File

@ -0,0 +1,142 @@
From a8ea42d2aa3738c680a932e2c42257ce4a880a47 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 10 Aug 2021 08:39:15 +0100
Subject: [PATCH] delay: Fix delay-close
See comments in the code for how this has been fixed.
This only delays clients which use NBD_CMD_DISC (libnbd
nbd_shutdown(3)). Clients which drop the connection obviously cannot
be delayed. For example:
$ nbdkit --filter=delay null delay-close=3 \
--run 'time nbdsh -u $uri -c "h.shutdown()"
time nbdsh -u $uri -c "pass"'
real 0m3.061s # Client used shutdown, was delayed
user 0m0.028s
sys 0m0.030s
real 0m0.058s # Client disconnected, was not delayed
user 0m0.029s
sys 0m0.027s
Reported-by: Ming Xie
Fixes: commit de8dcd3a34a38b088a0f9a6f8ca754702ad1f598
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1991652
(cherry picked from commit 0cafebdb67d0d557ba1be8ea306b8acc5d9b2203)
---
filters/delay/delay.c | 42 +++++++++++++++++++--------
filters/delay/nbdkit-delay-filter.pod | 14 +++++++--
2 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/filters/delay/delay.c b/filters/delay/delay.c
index df3729a7..9252b855 100644
--- a/filters/delay/delay.c
+++ b/filters/delay/delay.c
@@ -39,6 +39,7 @@
#include <stdbool.h>
#include <errno.h>
#include <limits.h>
+#include <time.h>
#include <nbdkit-filter.h>
@@ -134,12 +135,6 @@ open_delay (int *err)
return delay (delay_open_ms, err);
}
-static int
-close_delay (int *err)
-{
- return delay (delay_close_ms, err);
-}
-
/* Called for each key=value passed on the command line. */
static int
delay_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
@@ -253,13 +248,36 @@ delay_open (nbdkit_next_open *next, nbdkit_context *nxdata,
return NBDKIT_HANDLE_NOT_NEEDED;
}
-/* Close connection. */
-static void
-delay_close (void *handle)
+/* Close connection.
+ *
+ * We cannot call nbdkit_nanosleep here because the socket may have
+ * been closed and that function will abort and return immediately.
+ * However we want to force a sleep (even if the server is shutting
+ * down) so use regular nanosleep instead.
+ *
+ * We cannot use the .close callback because that happens after the
+ * socket has closed, thus not delaying the client. By using
+ * .finalize we can delay well-behaved clients (those that use
+ * NBD_CMD_DISC). We cannot delay clients that drop the connection.
+ */
+static int
+delay_finalize (nbdkit_next *next, void *handle)
{
- int err;
+ const unsigned ms = delay_close_ms;
- close_delay (&err);
+ if (ms > 0) {
+ struct timespec ts;
+
+ ts.tv_sec = ms / 1000;
+ ts.tv_nsec = (ms % 1000) * 1000000;
+ /* If nanosleep fails we don't really want to interrupt the chain
+ * of finalize calls through the other filters, so ignore any
+ * error here.
+ */
+ nanosleep (&ts, NULL);
+ }
+
+ return next->finalize (next);
}
/* Read data. */
@@ -340,7 +358,7 @@ static struct nbdkit_filter filter = {
.config_help = delay_config_help,
.can_fast_zero = delay_can_fast_zero,
.open = delay_open,
- .close = delay_close,
+ .finalize = delay_finalize,
.pread = delay_pread,
.pwrite = delay_pwrite,
.zero = delay_zero,
diff --git a/filters/delay/nbdkit-delay-filter.pod b/filters/delay/nbdkit-delay-filter.pod
index 11ae544b..76614736 100644
--- a/filters/delay/nbdkit-delay-filter.pod
+++ b/filters/delay/nbdkit-delay-filter.pod
@@ -117,15 +117,23 @@ the plugin.
=item B<delay-open=>NNB<ms>
+(nbdkit E<ge> 1.28)
+
+Delay open (client connection) by C<SECS> seconds or C<NN>
+milliseconds.
+
=item B<delay-close=>SECS
=item B<delay-close=>NNB<ms>
(nbdkit E<ge> 1.28)
-Delay open and close operations by C<SECS> seconds or C<NN>
-milliseconds. Open corresponds to client connection. Close may not
-be visible to clients if they abruptly disconnect.
+Delay close (client disconnection) by C<SECS> seconds or C<NN>
+milliseconds. This can also cause server shutdown to be delayed if
+clients are connected at the time. This only affects clients that
+gracefully disconnect (using C<NBD_CMD_DISC> / libnbd function
+L<nbd_shutdown(3)>). Clients that abruptly disconnect from the server
+cannot be delayed.
=back
--
2.31.1

155
0022-delay-Test-delay-open-and-delay-close.patch Normal file
View File

@ -0,0 +1,155 @@
From 1f2acea1e6bd0a1120bf6b48854202ec8680f5c0 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 10 Aug 2021 09:11:43 +0100
Subject: [PATCH] delay: Test delay-open and delay-close
(cherry picked from commit 3caabaf87ec744b863b50b5bf77a9c1b93a7c3e0)
---
tests/Makefile.am | 12 +++++++--
tests/test-delay-close.sh | 54 +++++++++++++++++++++++++++++++++++++++
tests/test-delay-open.sh | 49 +++++++++++++++++++++++++++++++++++
3 files changed, 113 insertions(+), 2 deletions(-)
create mode 100755 tests/test-delay-close.sh
create mode 100755 tests/test-delay-open.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am
index edc8d66d..e61c5829 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1425,8 +1425,16 @@ EXTRA_DIST += \
$(NULL)
# delay filter tests.
-TESTS += test-delay-shutdown.sh
-EXTRA_DIST += test-delay-shutdown.sh
+TESTS += \
+ test-delay-close.sh \
+ test-delay-open.sh \
+ test-delay-shutdown.sh \
+ $(NULL)
+EXTRA_DIST += \
+ test-delay-close.sh \
+ test-delay-open.sh \
+ test-delay-shutdown.sh \
+ $(NULL)
LIBNBD_TESTS += test-delay
test_delay_SOURCES = test-delay.c
diff --git a/tests/test-delay-close.sh b/tests/test-delay-close.sh
new file mode 100755
index 00000000..1de305f5
--- /dev/null
+++ b/tests/test-delay-close.sh
@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2021 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_run
+requires_plugin null
+requires_filter delay
+requires nbdsh --version
+
+# Test delay-close with a well-behaved client.
+
+nbdkit -U - null --filter=delay delay-close=3 \
+ --run '
+start_t=$SECONDS
+nbdsh -u "$uri" -c "h.shutdown()"
+end_t=$SECONDS
+
+if [ $((end_t - start_t)) -lt 3 ]; then
+ echo "$0: delay filter failed: delay-close=3 caused delay < 3 seconds"
+ exit 1
+fi
+'
diff --git a/tests/test-delay-open.sh b/tests/test-delay-open.sh
new file mode 100755
index 00000000..2a74e44c
--- /dev/null
+++ b/tests/test-delay-open.sh
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2018-2021 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_run
+requires_plugin null
+requires_filter delay
+requires nbdinfo --version
+
+start_t=$SECONDS
+nbdkit -U - null --filter=delay delay-open=3 --run 'nbdinfo "$uri"'
+end_t=$SECONDS
+
+if [ $((end_t - start_t)) -lt 3 ]; then
+ echo "$0: delay filter failed: delay-open=3 caused delay < 3 seconds"
+ exit 1
+fi
--
2.31.1

22
nbdkit.spec
View File

@ -51,7 +51,7 @@ ExclusiveArch: x86_64
Name: nbdkit Name: nbdkit
Version: 1.26.3 Version: 1.26.3
Release: 2%{?dist} Release: 3%{?dist}
Summary: NBD server Summary: NBD server
License: BSD License: BSD
@ -87,6 +87,17 @@ Patch0008: 0008-tests-cache-Test-cache-on-read-option-really-caches.patch
Patch0009: 0009-cow-Implement-cow-on-read.patch Patch0009: 0009-cow-Implement-cow-on-read.patch
Patch0010: 0010-delay-Add-delay-open-and-delay-close.patch Patch0010: 0010-delay-Add-delay-open-and-delay-close.patch
Patch0011: 0011-python-Implement-.cleanup-method.patch Patch0011: 0011-python-Implement-.cleanup-method.patch
Patch0012: 0012-server-Make-debug-messages-atomic.patch
Patch0013: 0013-cow-General-revision-and-updates-to-the-manual.patch
Patch0014: 0014-cache-Move-plugin-args-in-synopsis-earlier.patch
Patch0015: 0015-data-Improve-the-example-with-a-diagram.patch
Patch0016: 0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch
Patch0017: 0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch
Patch0018: 0018-delay-Improve-parsing-and-representation-of-delay-ti.patch
Patch0019: 0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch
Patch0020: 0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch
Patch0021: 0021-delay-Fix-delay-close.patch
Patch0022: 0022-delay-Test-delay-open-and-delay-close.patch
BuildRequires: make BuildRequires: make
%if 0%{patches_touch_autotools} %if 0%{patches_touch_autotools}
@ -1257,6 +1268,15 @@ export LIBGUESTFS_TRACE=1
%changelog %changelog
* Tue Aug 10 2021 Richard W.M. Jones <rjones@redhat.com> - 1.26.3-3
- Fix parsing of delay-* options
resolves: rhbz#1991649
- Fix assertion failure during server shutdown
- Fix delay-close option
resolves: rhbz#1991652
- tests/test-debug-flags.sh: Don't use port 10809 during test
resolves: rhbz#1991945
* Mon Aug 09 2021 Mohan Boddu <mboddu@redhat.com> - 1.26.3-2 * Mon Aug 09 2021 Mohan Boddu <mboddu@redhat.com> - 1.26.3-2
- Rebuilt for IMA sigs, glibc 2.34, aarch64 flags - Rebuilt for IMA sigs, glibc 2.34, aarch64 flags
Related: rhbz#1991688 Related: rhbz#1991688