[Libguestfs] [PATCH libnbd v2] examples: Include an example of integrating with the glib main loop.

[Eric Blake]

On 7/17/19 8:00 AM, Richard W.M. Jones wrote:
> ---
>  .gitignore                |   1 +
>  README                    |   2 +
>  configure.ac              |   9 +
>  examples/Makefile.am      |  22 ++
>  examples/glib-main-loop.c | 511 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 545 insertions(+)

Looks good.

> 
> +  revents = g_source_query_unix_fd ((GSource *) source, source->tag);
> +
> +  DEBUG (source, "dispatch: revents = 0x%x%s%s",
> +         revents,
> +         revents & G_IO_IN ? " G_IO_IN" : "",
> +         revents & G_IO_OUT ? " G_IO_OUT" : "");
> +
> +  r = 0;
> +  if ((revents & G_IO_IN) != 0)
> +    r = nbd_aio_notify_read (source->nbd);
> +  else if ((revents & G_IO_OUT) != 0)
> +    r = nbd_aio_notify_write (source->nbd);
> +

If revents ever contains G_IO_ERR/G_IO_HUP, and if we ever add
nbd_aio_notify_error(), we should inform the state machine about that fd
problem.  I've mentioned that idea before; I guess it is more of an
optimization - we will probably notice the problem eventually if the
client ever tries to issue another command or detects EOF while reading
a response; but tying up the resource until the next client command vs.
identifying the problem immediately may matter to some clients.

Also, there may still be a possible race:

> +/* This idle callback reads data from the source nbdkit until the ring
> + * is full.
> + */
> +static gboolean
> +read_data (gpointer user_data)
> +{
> +  static uint64_t posn = 0;

> +  buffers[i].rcookie =
> +    nbd_aio_pread_callback (gssrc->nbd, buffers[i].data,
> +                            BUFFER_SIZE, buffers[i].offset,
> +                            finished_read, NULL, 0);

It may be possible in rare situations that the libnbd state machine can
send() the command AND see data ready to recv() (perhaps from a previous
command still in flight), where it ends up read()ing until blocking and
happens to get the server's reply to this command and fire off the
callback, all prior to the nbd_aio_pread_callback() returning.  If that
ever happens, then buffers[i].rcookie will still be unset at the time
the callback fires...

> +/* This callback is called from libnbd when any read command finishes. */
> +static int
> +finished_read (void *vp, int64_t rcookie, int *error)
> +{
> +  size_t i;
> +
> +  if (gssrc == NULL)
> +    return 0;
> +
> +  DEBUG (gssrc, "finished_read: read completed");
> +
> +  /* Find the corresponding buffer and mark it as completed. */
> +  for (i = 0; i < nr_buffers; ++i) {
> +    if (buffers[i].rcookie == rcookie)
> +      goto found;
> +  }
> +  /* This should never happen. */
> +  abort ();

...such that we can hit this abort().  (Same for the write callback).
But I don't know how likely that scenario is in practice, and don't see
it as a reason to defer committing this patch as-is.  I don't know if
there is any way to rework the state machine to guarantee that a
completion callback will not fire during the same aio_FOO_callback that
registered the callback (that is, to guarantee that h->lock will be
dropped and must be re-obtained prior to firing off a callback), but
it's worth thinking about.

-- 
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/20190717/6bec7710/attachment.sig>