Re: [Qemu-devel] [PATCH 08/22] iscsi: implement .bdrv_detach/attach_aio_context()

[Peter Lieven <pl@kamp.de>]

On 07.05.2014 12:29, Paolo Bonzini wrote:
> Il 07/05/2014 12:07, Stefan Hajnoczi ha scritto:
>> On Fri, May 02, 2014 at 12:39:06AM +0200, Peter Lieven wrote:
>>>> +static void iscsi_attach_aio_context(BlockDriverState *bs,
>>>> +                                     AioContext *new_context)
>>>> +{
>>>> +    IscsiLun *iscsilun = bs->opaque;
>>>> +
>>>> +    iscsilun->aio_context = new_context;
>>>> +    iscsi_set_events(iscsilun);
>>>> +
>>>> +#if defined(LIBISCSI_FEATURE_NOP_COUNTER)
>>>> +    /* Set up a timer for sending out iSCSI NOPs */
>>>> +    iscsilun->nop_timer = aio_timer_new(iscsilun->aio_context,
>>>> + QEMU_CLOCK_REALTIME, SCALE_MS,
>>>> + iscsi_nop_timed_event, iscsilun);
>>>> +    timer_mod(iscsilun->nop_timer,
>>>> +              qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
>>>> +#endif
>>>> +}
>>>
>>> Is it still guaranteed that iscsi_nop_timed_event for a target is not invoked
>>> while we are in another function/callback of the iscsi driver for the same target?
>
> Yes, since the timer is in the same AioContext as the iscsi driver callbacks.


Ok. Stefan: What MUST NOT happen is that the timer gets fired while we are in iscsi_service.
As Paolo outlined, this cannot happen, right?

>
>> This is a good point.
>>
>> Previously, the nop timer was deferred until the qemu_aio_wait() loop
>> terminates.
>>
>> With this patch the nop timer fires during aio_poll() loops for any
>> synchronous emulation that QEMU does (including iscsi_aio_cancel() and
>> .bdrv_ioctl() in block/iscsi.c).
>>
>> I don't know libiscsi well enough to understand the implications.  I can
>> see that iscsi_reconnect() resends in-flight commands.  So what's the
>> upshot of all this?
>
> I think it's fine.  The target will process NOPs asynchronously, so iscsi_nop_timed_event will see no NOP in flight if the target is working properly.

Yes, or at most one in flight NOP.

>
>> BTW, is iscsi_reconnect() the right libiscsi interface to use since it
>> is synchronous?  It seems like this would block QEMU until the socket
>> has connected!  The guest would be frozen.
>
> There is no asynchronous interface yet for reconnection, unfortunately.

We initiate the reconnect after we miss a few NOP replies. So the target is already down for approx. 30 seconds.
Every process inside the guest is already haging or has timed out.

If I understand correctly with the new patches only the communication with this target is hanging or isn't it?
So what benefit would an asyncronous reconnect have?

Peter