[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH nbdkit v3 1/3] filters: Implement next_ops .reopen call.



On 9/19/19 10:26 AM, Richard W.M. Jones wrote:
> This is intended for use by the forthcoming retry filter to close and
> reopen the backend chain.  It is handled entirely by server/backend.c
> as no cooperation is needed with the plugin.
> 
> Note the explicit readonly parameter: An alternative would be to store
> the previous readonly setting in the b_conn_handle struct.  However
> passing it explicitly allows the retry filter to retry as readonly,
> which might be useful.  This design does however require any filter
> which might call .reopen to save the original readonly parameter from
> the .open call.
> ---
>  include/nbdkit-filter.h |  6 ++++++
>  server/backend.c        | 12 ++++++++++++
>  server/filters.c        |  8 ++++++++
>  server/internal.h       |  4 ++++
>  4 files changed, 30 insertions(+)
> 

I made comments on v2 that still apply here (crossed emails again).
Another one I just thought of:


> +int
> +backend_reopen (struct backend *b, struct connection *conn, int readonly)
> +{
> +  struct b_conn_handle *h = &conn->handles[b->i];
> +
> +  debug ("%s: reopen", b->name);
> +
> +  if (h->handle != NULL)
> +    backend_close (b, conn);
> +  return backend_open (b, conn, readonly);

Do we want to grab a resource lock at this point?  Should we be trying
hard to prevent further client calls from going to the retry filter
while the retry filter is reopening the real plugin?  Or do those locks
belong in the retry filter itself?  If nothing else, we probably want
some sort of locking to ensure that two threads are not trying reopen at
the same time.

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]