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

[Richard W.M. Jones]

On Wed, Jun 05, 2019 at 08:24:16AM -0500, Eric Blake wrote:
> On 6/5/19 8:03 AM, Richard W.M. Jones wrote:
> > On Wed, Jun 05, 2019 at 01:35:32PM +0100, Richard W.M. Jones wrote:
> >> On Wed, Jun 05, 2019 at 12:15:37PM +0100, Richard W.M. Jones wrote:
> >>> -#define set_state(h,next_state) ((h)->state) = (next_state)
> >>> +#define set_next_state(h,_next_state) ((h)->next_state) = (_next_state)
> >>> +#define get_next_state(h) ((h)->next_state)
> >>>  #define get_state(h) ((h)->state)
> >>
> >> So I wonder if it's better to rename get_state as get_last_state or
> >> get_visible_state?
> > 
> > Or even get_public_state?
> 
> get_public_state sounds nice (the state that nbd_connection_state will
> return).
> 
> > 
> >> And/or rename get_next_state/set_next_state to
> >> get_state/set_state?
> 
> If we rename the public state (which implies that it is frozen while the
> lock is held), using 'set_state' to adjust the next state is reasonable.
> 
> >>
> >> Ideas welcome to make the code clearer.
> 
> Do we have to use h->next_state everywhere, or can we rely on a
> stack-allocated variable passed through all the functions? Then again,
> we're already passing h through to all internal functions, which means
> h->next_state is already accessible without adding a parameter; but a
> stack-allocated variable may be harder to pass without lots of churn.

Yes we could allocate such a variable in the generated wrapper (but
only for functions which are !is_locked) and pass a pointer to it
throughout the code.  It's likely quite a bit of churn.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html