From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi2Xh-0004DT-0Z for qemu-devel@nongnu.org; Wed, 07 May 2014 10:10:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wi2XX-0004BE-Ug for qemu-devel@nongnu.org; Wed, 07 May 2014 10:09:52 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:54981 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi2XX-0004B2-Kh for qemu-devel@nongnu.org; Wed, 07 May 2014 10:09:43 -0400 Message-ID: <536A3E97.3000503@kamp.de> Date: Wed, 07 May 2014 16:09:27 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1398956086-20171-1-git-send-email-stefanha@redhat.com> <1398956086-20171-9-git-send-email-stefanha@redhat.com> <20140507100745.GD1771@stefanha-thinkpad.muc.redhat.com> <536A0AF8.7030600@redhat.com> In-Reply-To: <536A0AF8.7030600@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 08/22] iscsi: implement .bdrv_detach/attach_aio_context() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Stefan Hajnoczi Cc: Kevin Wolf , Stefan Hajnoczi , "Shergill, Gurinder" , qemu-devel@nongnu.org, Ronnie Sahlberg , "Vinod, Chegu" 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