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