[Libguestfs] [PATCH nbdkit 2/3] delay: Fix delay-close

[Eric Blake]

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 shutting

s/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