[Libguestfs] [PATCH libnbd v3] lib: Atomically update h->state when leaving the locked region.

[Eric Blake]

On 6/8/19 1:05 PM, Richard W.M. Jones wrote:
> Split h->state into:
> 
>  - h->public_state = the state on entry to the locked region
>    This is also the atomically, publicly visible state.
> 
>  - h->state = the real current state of the handle
> 
> When we leave the locked region we update h->public_state with
> h->state, so that from outside the lock the handle appears to move
> atomically from its previous state to the final state without going
> through any intermediate states.
> 
> Some calls to ‘get_state’ become calls to ‘get_next_state’ if the need
> the real state.  Others which need to see the publicly visible state
> are changed to ‘get_public_state’.
> 
> All calls to ‘set_state’ become ‘set_next_state’ because that is the
> real state that gets updated.
> 
> The purpose of this patch is to make it easier to reason about the
> state in lockless code.
> ---
>  generator/generator | 87 ++++++++++++++++++++++++++++-----------------
>  lib/connect.c       | 10 +++---
>  lib/disconnect.c    |  8 ++---
>  lib/handle.c        |  2 +-
>  lib/internal.h      | 17 +++++++--
>  lib/is-state.c      | 28 ++++++++-------
>  lib/rw.c            |  4 +--
>  7 files changed, 97 insertions(+), 59 deletions(-)

> @@ -2530,7 +2531,7 @@ let generate_lib_states_c () =
>    pr "{\n";
>    pr "  int r = 0;\n";
>    pr "\n";
> -  pr "  switch (get_state (h))\n";
> +  pr "  switch (get_next_state (h))\n";
>    pr "  {\n";
>    List.iter (
>      fun ({ parsed = { state_enum; events } }) ->


We missed a spot. With this commit, we have:

  pr "/* Returns whether in the current state read or write would be
valid.\n";
  pr " * NB: is_locked = false, may_set_error = false.\n";
  pr " */\n";
  pr "int\n";
  pr "nbd_unlocked_aio_get_direction (struct nbd_handle *h)\n";
  pr "{\n";
  pr "  int r = 0;\n";
  pr "\n";
  pr "  switch (get_next_state (h))\n";

which means the lockless public function nbd_aio_get_direction is
non-deterministic while another thread holds the lock.  I'm applying the
obvious patch to split out nbd_internal_aio_get_direction(state), so
that the public version remains tied to the prior state, while
nbd_unlocked_poll() still tracks the internal state.

-- 
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/20190620/4d4698ea/attachment.sig>