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

Re: [Libguestfs] [libnbd PATCH 1/6] api: Add nbd_aio_in_flight



On 6/29/19 8:28 AM, Eric Blake wrote:
> Some clients need to know when it is safe to issue NBD_CMD_DISC, or to
> decide whether calling poll(POLLIN) will block indefinitely because
> the server isn't expected to respond.  Make this easier to learn by
> tracking the count of commands we have queued up to send, as well as
> the count of commands where we are waiting on the server's response.

This documents one thing...

> 
> Update tests/aio-parallel* and examples/batched-read-write to use
> nbd's own in-flight counter instead of reimplementing it ourselves.
> 
> Note that h->in_flight is only ever updated while the lock is held;
> but we may want to consider also making it atomic and therefore
> readable as a lock-less function.
> ---


> @@ -2012,6 +2018,22 @@ C<nbd_aio_command_completed> to actually retire the command and learn
>  whether the command was successful.";
>    };
> 
> +  "aio_in_flight", {
> +    default_call with
> +    args = []; ret = RInt;
> +    permitted_states = [ Connected; Closed; Dead ];
> +    (* XXX is_locked = false ? *)
> +    shortdesc = "check how many aio commands are still in flight";
> +    longdesc = "\
> +Return the number of in-flight aio commands that are still awaiting a
> +response from the server before they can be retired.  If this returns
> +a non-zero value when requesting a disconnect from the server (see
> +C<nbd_aio_disconnect> and C<nbd_shutdown>), libnbd does not try to
> +wait for those commands to complete gracefully; if the server strands
> +commands while shutting down, C<nbd_aio_command_completed> will not
> +be able to report status on those commands.";

and this concurs with the intent...


> +++ b/lib/aio.c
> @@ -23,6 +23,7 @@
>  #include <stdbool.h>
>  #include <errno.h>
>  #include <inttypes.h>
> +#include <assert.h>
> 
>  #include "internal.h"
> 
> @@ -84,6 +85,8 @@ nbd_unlocked_aio_command_completed (struct nbd_handle *h,
>      prev_cmd->next = cmd->next;
>    else
>      h->cmds_done = cmd->next;
> +  h->in_flight--;
> +  assert (h->in_flight >= 0);

...but this implementation is wrong.  It counts commands that have a
response but are not retired as being in-flight.  Rather, we should be
decrementing in_flight at the point a command moves into h->cmds_done.
I'll post a fix.

-- 
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]