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
On Tue, Aug 10, 2021 at 09:19:14AM +0100, Richard W.M. Jones wrote:> 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>or> +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 >> +/* 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 shuttings/for /to /> + * 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) > {With those typos fixed, series looks good to me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org