[Libguestfs] [PATCH 2/9] Refactor plugin_* functions into a backend struct.

[Eric Blake]

On 01/17/2018 02:53 PM, Richard W.M. Jones wrote:
> Introduce the concept of a backend.  Currently the only type of
> backend is a plugin, and there can only be one of them.  Instead of
> calling functions like ‘plugin_pwrite’ you call the backend method
> ‘backend->pwrite (backend, ...)’.
> 
> The change is largely mechanical.  I was able to remove ‘assert (dl)’
> statements throughout since we can now prove they will never be
> called.
> 
> Note this does not lift the restriction of one plugin per server, and
> it can *never* do that because plugins can use global variables.
> ---
>  src/connections.c |  40 +++--
>  src/internal.h    |  55 +++---
>  src/locks.c       |   8 +-
>  src/main.c        |  31 ++--
>  src/plugins.c     | 529 ++++++++++++++++++++++++++++++------------------------
>  5 files changed, 368 insertions(+), 295 deletions(-)

Hmmm.  I'm wondering if we want a src/backend.c,...

> -  if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers == 1)
> +  if (backend->thread_model (backend) < NBDKIT_THREAD_MODEL_PARALLEL ||
> +      nworkers == 1)

...where we'd call backend_thread_model (backend), and backend.c has:

int backend_thread_model (struct backend *b)
{
  return b->thread_model (b);
}

The real reason I ask is because of:

> +++ b/src/plugins.c

> +static int
> +plugin_zero (struct backend *b, struct connection *conn,
>               uint32_t count, uint64_t offset, int may_trim)
>  {
> -  assert (dl);
> -  assert (connection_get_handle (conn));
> +  struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
>    char *buf;
>    uint32_t limit;
>    int result;
>    int err = 0;
>  
> +  assert (connection_get_handle (conn));
> +
>    debug ("zero count=%" PRIu32 " offset=%" PRIu64 " may_trim=%d",
>           count, offset, may_trim);
>  
>    if (!count)
>      return 0;
> -  if (plugin.zero) {
> +  if (p->plugin.zero) {
>      errno = 0;
> -    result = plugin.zero (connection_get_handle (conn), count, offset, may_trim);
> +    result = p->plugin.zero (connection_get_handle (conn),
> +                             count, offset, may_trim);
>      if (result == -1) {
>        err = threadlocal_get_error ();
> -      if (!err && plugin_errno_is_preserved ())
> +      if (!err && plugin_errno_is_preserved (b))
>          err = errno;
>      }
>      if (result == 0 || err != EOPNOTSUPP)
>        return result;
>    }
>  
> -  assert (plugin.pwrite);
> +  assert (p->plugin.pwrite);
>    threadlocal_set_error (0);
>    limit = count < MAX_REQUEST_SIZE ? count : MAX_REQUEST_SIZE;
>    buf = calloc (limit, 1);
> @@ -525,7 +446,8 @@ plugin_zero (struct connection *conn,
>    }
>  
>    while (count) {
> -    result = plugin.pwrite (connection_get_handle (conn), buf, limit, offset);
> +    result = p->plugin.pwrite (connection_get_handle (conn),
> +                               buf, limit, offset);

this fallback.  If I leave the EOPNOTSUPP check in plugins.c, then every
filter has to duplicate the logic.  But with a backend.c, I could do
something along these lines (psuedocode, not actually tested):

int backend_zero (struct backend *b, struct connection *conn,
                  uint32_t count, uint64_t offset, int may_trim)
{
  try b->zero (b, conn, count, offset, may_trim)
  if EOPNOTSUPP
    loop on b->pwrite (b, conn, buf, limit, offset)
}

coupled with:

static int
plugin_zero (struct backend *b, struct connection *conn,
             uint32_t count, uint64_t offset, int may_trim)
{
  if (!p->plugin.zero) {
    errno = EOPNOTSUPP;
    return -1;
  }
  return p->plugin.zero (connection_get_handle (conn), count, offset,
may_trim);
}

basically, splitting the fallback code that needs to be reusable from
the plugin code that just has to call into the actual plugin.so
function; this way, filters can also implement .zero to fail with
EOPNOTSUPP and trigger the fallback code without having to duplicate things.

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

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