Re: [Libguestfs] [PATCH libnbd v3] lib: Atomically update h->state when leaving the locked region.
On 6/8/19 1:05 PM, Richard W.M. Jones wrote:
v1 was here:
https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00055
v2 was here:
https://www.redhat.com/archives/libguestfs/2019-June/thread.html#00067
v3:
- Fix atomicly -> atomically in commit message.
- Fix a comment.
- Fix TOCTTOU: There is now an inline function generated called
<name>_is_permitted_state, and this is called twice, first outside
the lock for the quick rejection check, and again inside the lock
in case the state changed.
I think an improvement would be to _drop_ the first check (outside the
lock). This is an error check, so not performance critical since we
are definitely going to acquire the lock on the hot path.
Indeed - most callers aren't going to trigger the early exit path, while
the case of the state changing (most likely changing to DEAD for
whatever reason) is something we want to be robust once we do get the
lock, even if it meant blocking until we got lock.
Patch looks good to me now, whether or not you drop the early exit path
outside the lock.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)