From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43748)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Whyla-0006fd-UR
	for qemu-devel@nongnu.org; Wed, 07 May 2014 06:08:08 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1WhylR-000754-OL
	for qemu-devel@nongnu.org; Wed, 07 May 2014 06:07:58 -0400
Received: from mail-we0-x230.google.com ([2a00:1450:400c:c03::230]:52025)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1WhylR-00074p-IL
	for qemu-devel@nongnu.org; Wed, 07 May 2014 06:07:49 -0400
Received: by mail-we0-f176.google.com with SMTP id q59so733271wes.7
	for <qemu-devel@nongnu.org>; Wed, 07 May 2014 03:07:48 -0700 (PDT)
Date: Wed, 7 May 2014 12:07:45 +0200
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20140507100745.GD1771@stefanha-thinkpad.muc.redhat.com>
References: <1398956086-20171-1-git-send-email-stefanha@redhat.com>
	<1398956086-20171-9-git-send-email-stefanha@redhat.com>
	<B500D1F1-BB19-4C51-B3E6-517CD4063732@kamp.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <B500D1F1-BB19-4C51-B3E6-517CD4063732@kamp.de>
Subject: Re: [Qemu-devel] [PATCH 08/22] iscsi: implement
 .bdrv_detach/attach_aio_context()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Lieven <pl@kamp.de>
Cc: Kevin Wolf <kwolf@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, "Shergill, Gurinder" <gurinder.shergill@hp.com>, qemu-devel@nongnu.org, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, "Vinod, Chegu" <chegu_vinod@hp.com>

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?

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?

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.

Stefan