[Libguestfs] [nbdkit PATCH v3 12/16] filters: New API for plugin context management

[Eric Blake]

On 3/5/21 5:31 PM, Eric Blake wrote:
> As promised in an earlier patch, get rid of the temporary
> nbdkit_backend_reopen, and replace it with the finer-grained
> combination of nbdkit_context_get_backend, nbdkit_context_next_open,
> nbdkit_context_next_close, and nbdkit_context_set_next.  Update the
> retry filter to use the new API.  Change the (temporary) preprocessor
> guard from NBDKIT_RETRY_FILTER to NBDKIT_TYPESAFE (will be dropped
> later once more filters are converted).  Drop the now-unused
> backend_reopen() function.  Expose .prepare and .finalize callbacks
> into struct nbdkit_next_ops for use during manual context
> manipulation.

Insufficient testing on my part:

> +++ b/docs/nbdkit-filter.pod
> @@ -132,25 +132,33 @@ C<nbdkit_next_after_fork>, C<nbdkit_next_preconnect>,
>  C<nbdkit_next_list_exports>, C<nbdkit_next_default_export>,
>  C<nbdkit_next_open>) and a structure called C<struct nbdkit_next_ops>.
>  These abstract the next plugin or filter in the chain.  There is also

> +an opaque pointer C<backend>, C<context> or C<nxdata> which must be
> +passed along when calling these functions.  The value of C<backend> is
> +stable between C<.after_fork>, C<.preconnect>, C<.list_exports>, and
> +C<.default_export>, and can also be obtained by using
> +C<nbdkit_context_get_backend> on the C<context> parameter to C<.open>.

This promise makes sense,

> @@ -468,31 +563,21 @@ C<nbdkit_is_tls>.
>  The filter should generally call C<next> as its first step, to
>  allocate from the plugin outwards, so that C<.close> running from the
>  outer filter to the plugin will be in reverse.  Skipping a call to
> -C<next> is acceptable if the filter will not access C<next_ops>
> -during any of the remaining callbacks reached on the same connection.
> +C<next> is acceptable if the filter will not access C<next_ops> during
> +any of the remaining callbacks reached on the same connection.  The
> +C<next> function is provided for convenience; the same functionality
> +can be obtained manually (other than error checking) by using the
> +following:
> +
> + nbdkit_context_set_next (context, nbdkit_next_context_open
> +    (nbdkit_context_get_backend (context), readonly, exportname));

as does this documentation,

> +++ b/server/filters.c
> @@ -263,12 +263,14 @@ filter_default_export (struct backend *b, int readonly, int is_tls)
>  static int
>  next_open (struct context *c, int readonly, const char *exportname)
>  {
> -  struct backend *b = c->b;
> -  struct context *c_next = backend_open (b->next, readonly, exportname);
> +  struct backend *b = nbdkit_context_get_backend (c);
> +  struct context *c_next = nbdkit_next_context_open (b, readonly, exportname);

and this even tries to mirror the documentation,

> +  struct context *old;
> 
>    if (c_next == NULL)
>      return -1;
> -  c->c_next = c_next;
> +  old = nbdkit_context_set_next (c, c_next);
> +  assert (old == NULL);
>    return 0;
>  }
> 
> @@ -694,12 +696,35 @@ filter_register (struct backend *next, size_t index, const char *filename,
>    return (struct backend *) f;
>  }
> 
> -int
> -nbdkit_backend_reopen (struct context *c, int readonly,
> -                       const char *exportname, struct context **nxdata)
> +struct backend *
> +nbdkit_context_get_backend (struct context *c)
>  {
> -  int r = backend_reopen (c, readonly, exportname);
> +  assert (c);
> +  return c->b;

but this is returning the wrong context (that of the current filter,
rather than of the backend)

> +}
> +
> +struct context *
> +nbdkit_next_context_open (struct backend *b,
> +                          int readonly, const char *exportname)
> +{
> +  assert (b);
> +  return backend_open (b->next, readonly, exportname);

and this is compensating by calling backend_open() on the wrong context
(it should be calling it on b, rather than on b->next).

I'm fixing the bug and enhancing test-layers-filter.c to catch the problem.

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