Re: [Libguestfs] [nbdkit PATCH] readahead: Fix multi-conn inconsistency
On 2/19/21 3:18 PM, Eric Blake wrote:
On 2/19/21 3:03 PM, Eric Blake wrote:
> The readahead filter was blindly passing through the plugin's
> .can_multi_conn, but does not kill the readahead buffer on a flush.
> This is a bug in the following scenario, when the plugin advertised
> multi-conn:
>
> In order to preserve the plugin's multi-conn status, we MUST drop our
> cache any time we flush, so that our next read picks up whatever got
> flushed from another connection.
>
> But when the server lacks multi-conn, we don't have to kill our
> readahead buffer on flush, because the client already has no guarantee
> that a flush from one connection will affect the read seen by another
> (killing on write was already good enough to maintain semantics).
> ---
> +static int
> +readahead_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
> + void *handle, uint32_t flags, int *err)
> +{
> + if (next_ops->can_multi_conn(nxdata) != 1)
That should be ==, to match my commit message.
> + kill_readahead ();
> + return next_ops->flush (nxdata, flags, err);
> +}
> +
In fact, the more I think about it, the more I want it to be
unconditional, killing readahead even when not multi-conn. Why?
Because when multi-conn is not advertised, it seems odd that there is no
way at all to force the NBD server to flush its caches; it seems like
clients should be able to expect a flush on connA (to write pending data
to cache), then a flush on connB (to drop any readahead data), prior to
a read on connB, should let connB see what connA wrote. All that
multi-conn buys you is that you can skip one of the two flush calls.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org