Richard W.M. Jones
2021-Aug-10 08:19 UTC
[Libguestfs] [PATCH nbdkit 1/3] 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.
---
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.32.0
Richard W.M. Jones
2021-Aug-10 08:19 UTC
[Libguestfs] [PATCH nbdkit 2/3] 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
---
filters/delay/nbdkit-delay-filter.pod | 13 +++++++--
filters/delay/delay.c | 42 +++++++++++++++++++--------
2 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/filters/delay/nbdkit-delay-filter.pod
b/filters/delay/nbdkit-delay-filter.pod
index 11ae544b..bfc90dae 100644
--- a/filters/delay/nbdkit-delay-filter.pod
+++ b/filters/delay/nbdkit-delay-filter.pod
@@ -117,15 +117,22 @@ 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 of C<NN>
+milliseconds. This will also cause server shutdown to be delayed.
+This only affects clients that gracefully disconnect (using
+C<NBD_CMD_DISC> / libnbd function L<nbd_shutdown(3)>). Clients
that
+abruptly close the socket cannot be delayed.
=back
diff --git a/filters/delay/delay.c b/filters/delay/delay.c
index df3729a7..e0b32f46 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 for 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,
--
2.32.0
Richard W.M. Jones
2021-Aug-10 08:19 UTC
[Libguestfs] [PATCH nbdkit 3/3] delay: Test delay-open and delay-close
--- tests/Makefile.am | 12 +++++++-- tests/test-delay-close.sh | 54 +++++++++++++++++++++++++++++++++++++++ tests/test-delay-open.sh | 49 +++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 04858849..344bb697 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1427,8 +1427,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.32.0