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

Re: [Libguestfs] [PATCH libnbd] lib: Remove cookie parameter from completion callbacks.



On 7/30/19 11:28 AM, Richard W.M. Jones wrote:
> On Tue, Jul 30, 2019 at 11:21:25AM -0500, Eric Blake wrote:
>> On 7/30/19 10:36 AM, Richard W.M. Jones wrote:
>>> +When the command completes, C<callback>
>>>  will be invoked as described in L<libnbd(3)/Completion callbacks>.
>>> +
>>
>> Do we need wording anywhere (probably in the .pod, so we only state it
>> once) that mentions that the callback routine is not invoked if
>> nbd_aio_FOO_callback returns -1?
> 
> I guess it's implicit from the fact that returning -1 indicates an
> error.
> 
> In fact the relationship ought to be stronger than that - the command
> should run if and only if nbd_aio_FOO_callback returns >= 0.  (That's
> not actually true at the moment because an error can happen after
> we've enqueued the command.)

Eww, you're right. We return -1 if nbd_internal_run queued the command
but encountered an error moving the state machine along; but that is
much trickier for the user to recover from (if they aren't using the
_callback form, then they NEED to know the cookie id to eventually
retire the command during nbd_aio_command_completed). Maybe we should
guarantee that we return the cookie in that case, even though a failure
was detected?  I don't think it is a good idea to change the API from
'int64 nbd_aio_pread()' to 'int nbd_aio_pread(int64 *cookie)' where
*cookie gets assigned on successful queue whether or not the overall
command returns 0 or -1.

In the long term, if nbd_internal_run() fails, then the next nbd_* call
that tries to manipulate the state machine will see that the state
machine is in the DEAD state; learning that the state machine failed
during nbd_aio_pread probably doesn't buy us much benefit.  So I'm
leaning towards just ignoring nbd_internal_run failure, and guaranteeing
that we return > 0 if we queue the command, no matter whether we then
encounter a state machine error after that point.

-- 
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]