From 704d2b18ae3db2e246f27e978254afe448c32a72 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 10 Aug 2021 16:09:59 +0100 Subject: [PATCH] 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 --- 0012-server-Make-debug-messages-atomic.patch | 157 +++++++++++++++ ...l-revision-and-updates-to-the-manual.patch | 150 +++++++++++++++ ...Move-plugin-args-in-synopsis-earlier.patch | 35 ++++ ...a-Improve-the-example-with-a-diagram.patch | 53 +++++ ...e-debugging-especially-for-blk_read_.patch | 45 +++++ ...-flags.sh-Don-t-use-port-10809-durin.patch | 74 +++++++ ...rsing-and-representation-of-delay-ti.patch | 182 ++++++++++++++++++ ...om-nbdkit_nanosleep-early-if-the-soc.patch | 41 ++++ ...-Change-error-for-early-end-of-sleep.patch | 43 +++++ 0021-delay-Fix-delay-close.patch | 142 ++++++++++++++ ...elay-Test-delay-open-and-delay-close.patch | 155 +++++++++++++++ nbdkit.spec | 22 ++- 12 files changed, 1098 insertions(+), 1 deletion(-) create mode 100644 0012-server-Make-debug-messages-atomic.patch create mode 100644 0013-cow-General-revision-and-updates-to-the-manual.patch create mode 100644 0014-cache-Move-plugin-args-in-synopsis-earlier.patch create mode 100644 0015-data-Improve-the-example-with-a-diagram.patch create mode 100644 0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch create mode 100644 0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch create mode 100644 0018-delay-Improve-parsing-and-representation-of-delay-ti.patch create mode 100644 0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch create mode 100644 0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch create mode 100644 0021-delay-Fix-delay-close.patch create mode 100644 0022-delay-Test-delay-open-and-delay-close.patch diff --git a/0012-server-Make-debug-messages-atomic.patch b/0012-server-Make-debug-messages-atomic.patch new file mode 100644 index 0000000..0e97b71 --- /dev/null +++ b/0012-server-Make-debug-messages-atomic.patch @@ -0,0 +1,157 @@ +From 453a7611b625fc6f306a47ccc61bf3b83d75e522 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0013-cow-General-revision-and-updates-to-the-manual.patch b/0013-cow-General-revision-and-updates-to-the-manual.patch new file mode 100644 index 0000000..1ae7c56 --- /dev/null +++ b/0013-cow-General-revision-and-updates-to-the-manual.patch @@ -0,0 +1,150 @@ +From d15e320d20260c14973ef84172ae8cbe337a2b48 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 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 +- +-=item * +- +-All connections to the nbdkit instance see the same view of the disk. +- +-This is different from L 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 pass the I<-r> flag to nbdkit. +- +-=back ++B ++If you want to save changes, either copy out the whole disk using a ++tool like L, or use the method described in L ++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 +- +-When the client issues a cache (prefetch) request, preemptively save +-the data from the plugin into the overlay. +- + =item B + + 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 ++=item B + +-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 + + Do not save data from read requests in the overlay. This leaves the + overlay as small as possible. This is the default. + ++=item B ++ ++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 + + When F (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, 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 only supports read access, but you can provide +-temporary write access by doing (although this does B save +-changes to the file): ++temporary write access by using the command above. Because xz ++decompression is slow, using C 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. +@@ -118,6 +113,14 @@ F now contains the differences between the base + (F) and the changes stored in nbdkit-cow-filter. C + 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 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, + L, + L, + L, ++L, + L. + + =head1 AUTHORS +-- +2.31.1 + diff --git a/0014-cache-Move-plugin-args-in-synopsis-earlier.patch b/0014-cache-Move-plugin-args-in-synopsis-earlier.patch new file mode 100644 index 0000000..b30531d --- /dev/null +++ b/0014-cache-Move-plugin-args-in-synopsis-earlier.patch @@ -0,0 +1,35 @@ +From 583d5308ea8d26248e521b76afb380432d2084bc Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0015-data-Improve-the-example-with-a-diagram.patch b/0015-data-Improve-the-example-with-a-diagram.patch new file mode 100644 index 0000000..075cdca --- /dev/null +++ b/0015-data-Improve-the-example-with-a-diagram.patch @@ -0,0 +1,53 @@ +From 72b87c985dc9324d896333f9ddaf317cece8a812 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 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 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 + diff --git a/0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch b/0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch new file mode 100644 index 0000000..68175ca --- /dev/null +++ b/0016-cow-Add-some-more-debugging-especially-for-blk_read_.patch @@ -0,0 +1,45 @@ +From d2ed77d1b8fedcba3aedadeed883553886f4bb56 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch b/0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch new file mode 100644 index 0000000..1462493 --- /dev/null +++ b/0017-tests-test-debug-flags.sh-Don-t-use-port-10809-durin.patch @@ -0,0 +1,74 @@ +From ad2b4d2c07def233b2192c2a7ff925d1b6b823e7 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0018-delay-Improve-parsing-and-representation-of-delay-ti.patch b/0018-delay-Improve-parsing-and-representation-of-delay-ti.patch new file mode 100644 index 0000000..09d2141 --- /dev/null +++ b/0018-delay-Improve-parsing-and-representation-of-delay-ti.patch @@ -0,0 +1,182 @@ +From 810c2449cb519100bf9ea50d743162a391eac873 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + +-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 + diff --git a/0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch b/0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch new file mode 100644 index 0000000..724feb6 --- /dev/null +++ b/0019-server-Return-from-nbdkit_nanosleep-early-if-the-soc.patch @@ -0,0 +1,41 @@ +From 33318699bf1255aef0c6ee4863236d26d7b326ec Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch b/0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch new file mode 100644 index 0000000..edc212e --- /dev/null +++ b/0020-server-nanosleep-Change-error-for-early-end-of-sleep.patch @@ -0,0 +1,43 @@ +From 032531cd5d402119a81efbaf07d781123c5b02af Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/0021-delay-Fix-delay-close.patch b/0021-delay-Fix-delay-close.patch new file mode 100644 index 0000000..d6d2091 --- /dev/null +++ b/0021-delay-Fix-delay-close.patch @@ -0,0 +1,142 @@ +From a8ea42d2aa3738c680a932e2c42257ce4a880a47 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + #include + #include ++#include + + #include + +@@ -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 BNNB + ++(nbdkit E 1.28) ++ ++Delay open (client connection) by C seconds or C ++milliseconds. ++ + =item BSECS + + =item BNNB + + (nbdkit E 1.28) + +-Delay open and close operations by C seconds or C +-milliseconds. Open corresponds to client connection. Close may not +-be visible to clients if they abruptly disconnect. ++Delay close (client disconnection) by C seconds or C ++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 / libnbd function ++L). Clients that abruptly disconnect from the server ++cannot be delayed. + + =back + +-- +2.31.1 + diff --git a/0022-delay-Test-delay-open-and-delay-close.patch b/0022-delay-Test-delay-open-and-delay-close.patch new file mode 100644 index 0000000..fe3936e --- /dev/null +++ b/0022-delay-Test-delay-open-and-delay-close.patch @@ -0,0 +1,155 @@ +From 1f2acea1e6bd0a1120bf6b48854202ec8680f5c0 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +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 + diff --git a/nbdkit.spec b/nbdkit.spec index 5faa48f..3681299 100644 --- a/nbdkit.spec +++ b/nbdkit.spec @@ -51,7 +51,7 @@ ExclusiveArch: x86_64 Name: nbdkit Version: 1.26.3 -Release: 2%{?dist} +Release: 3%{?dist} Summary: NBD server 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 Patch0010: 0010-delay-Add-delay-open-and-delay-close.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 %if 0%{patches_touch_autotools} @@ -1257,6 +1268,15 @@ export LIBGUESTFS_TRACE=1 %changelog +* Tue Aug 10 2021 Richard W.M. Jones - 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 - 1.26.3-2 - Rebuilt for IMA sigs, glibc 2.34, aarch64 flags Related: rhbz#1991688