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

[Paolo Bonzini]

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.

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

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

Paolo