Re: [Libguestfs] [libnbd PATCH 2/6] generator: Allow DEAD state actions to run
On 6/29/19 12:17 PM, Richard W.M. Jones wrote:
This wasn't exactly how I imagined it - I thought we'd change the
generator so that ‘return -1’ wouldn't stop the state machine, but
would save an error indication, keep running the machine until it
blocks, then return an error.
Maybe this would have worked, but I didn't closely check whether the
other sites that return -1, such as CONNECT_TCP.START jumping back to
%^START after getaddrinfo() fails, would break if the state machine kept
running further rather than just stopping right away.
However this is fine, so ACK.
Okay, I'll go ahead and push this as-is; we can always do a followup
patch to do things differently if desired.
If I was going to improve this patch in some way (and didn't implement
the idea above) then: I'd add a new macro for entering the DEAD state,
setting the error, and returning the right code all in one statement.
On the basis that it removes the scope for programmer error. This
would require a small change to the generator so the new macro is
recognized as working like SET_NEXT_STATE; and probably a second
change to the generator to prevent ordinary SET_NEXT_STATE from
jumping to the DEAD state, to force everyone to use the new macro in
this case.
Not all of the callers set the error at the same point they call
SET_NEXT_STATE (for example, recv_into_rbuf() sets the error before
returning -1, but is in prologue code so it can't call SET_NEXT_STATE;
instead, the caller, on seeing a return of -1, uses SET_NEXT_STATE but
doesn't have to set an error).
--
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)