[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]