[Libguestfs] [PATCH nbdkit v2 02/11] blocksize: Implement filtering of .can_multi_conn (forcing it to false).

[Eric Blake]

On 1/5/19 8:50 AM, Richard W.M. Jones wrote:
> I examined each filter to see which ones implement a cache and do not
> properly consider consistency across clients for flush requests.  For
> these filters we should force .can_multi_conn to return false.
> 
> I believe only one filter (blocksize) needs to be updated and all the
> other ones are safe.
> ---
>  filters/blocksize/blocksize.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
> index 34f58c9..9e0e713 100644
> --- a/filters/blocksize/blocksize.c
> +++ b/filters/blocksize/blocksize.c
> @@ -164,6 +164,17 @@ blocksize_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
>    return size == -1 ? size : size & ~(minblock - 1);
>  }
>  
> +static int
> +blocksize_can_multi_conn (struct nbdkit_next_ops *next_ops, void *nxdata,
> +                          void *handle)
> +{
> +  /* Although we are serializing all requests anyway so this likely
> +   * doesn't make a difference, return false because the bounce buffer
> +   * is not consistent for flush.
> +   */
> +  return 0;
> +}

I think we could get away with returning 1, _because_ we are serializing
all requests.  Even though we make multiple calls into the plugin
(worst-case being read-modify-write-flush on a client write-with-FUA),
no other operation can compete with a client flush.  But I also agree
that if we were to fix the bounce buffer to be thread-safe and change
the filter to be more parallel, then you are also right that unless we
add a .flush callback and add locking so that all calls into the plugin
are guarded within the same lock, then returning 0 is appropriate, even
if the plugin itself returns 1.  So I agree with adding this callback,
and could go either way with you returning 0 or 1 for now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190105/4c6ca2f1/attachment.sig>