From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40514) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwvnD-00057j-3I for qemu-devel@nongnu.org; Wed, 10 Jul 2013 10:54:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UwvnB-0002Mq-6C for qemu-devel@nongnu.org; Wed, 10 Jul 2013 10:54:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64595) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwvnA-0002Mf-UZ for qemu-devel@nongnu.org; Wed, 10 Jul 2013 10:54:53 -0400 Date: Wed, 10 Jul 2013 16:54:45 +0200 From: Kevin Wolf Message-ID: <20130710145445.GW3898@dhcp-200-207.str.redhat.com> References: <1372338695-411-1-git-send-email-pl@kamp.de> <1372338695-411-4-git-send-email-pl@kamp.de> <20130710094113.GE3898@dhcp-200-207.str.redhat.com> <51DD6685.6060406@kamp.de> <20130710144309.GT3898@dhcp-200-207.str.redhat.com> <51DD7495.9040709@kamp.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51DD7495.9040709@kamp.de> Subject: Re: [Qemu-devel] [PATCHv2 03/11] iscsi: add bdrv_co_is_allocated List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: pbonzini@redhat.com, ronniesahlberg@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 10.07.2013 um 16:49 hat Peter Lieven geschrieben: > Am 10.07.2013 16:43, schrieb Kevin Wolf: > > Am 10.07.2013 um 15:49 hat Peter Lieven geschrieben: > >> Am 10.07.2013 11:41, schrieb Kevin Wolf: > >>> Am 27.06.2013 um 15:11 hat Peter Lieven geschrieben: > >>>> this patch adds a coroutine for .bdrv_co_is_allocated as well as > >>>> a generic framework that can be used to build coroutines in block/iscsi. > >>>> > >>>> Signed-off-by: Peter Lieven > >>>> --- > >>>> block/iscsi.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> 1 file changed, 123 insertions(+) > >>>> > >>>> diff --git a/block/iscsi.c b/block/iscsi.c > >>>> index 2e2455d..d31ee95 100644 > >>>> --- a/block/iscsi.c > >>>> +++ b/block/iscsi.c > >>>> @@ -58,6 +58,15 @@ typedef struct IscsiLun { > >>>> uint32_t max_unmap_bdc; > >>>> } IscsiLun; > >>>> > >>>> +typedef struct IscsiTask { > >>>> + int status; > >>>> + int complete; > >>>> + int retries; > >>>> + int do_retry; > >>>> + struct scsi_task *task; > >>>> + Coroutine *co; > >>>> +} IscsiTask; > >>>> + > >>>> typedef struct IscsiAIOCB { > >>>> BlockDriverAIOCB common; > >>>> QEMUIOVector *qiov; > >>>> @@ -113,6 +122,45 @@ iscsi_schedule_bh(IscsiAIOCB *acb) > >>>> qemu_bh_schedule(acb->bh); > >>>> } > >>>> > >>>> +static void > >>>> +iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, > >>>> + void *command_data, void *opaque) > >>>> +{ > >>>> + struct IscsiTask *iTask = opaque; > >>>> + struct scsi_task *task = command_data; > >>>> + > >>>> + iTask->complete = 1; > >>>> + iTask->status = status; > >>>> + iTask->do_retry = 0; > >>>> + iTask->task = task; > >>>> + > >>>> + if (iTask->retries-- > 0 && status == SCSI_STATUS_CHECK_CONDITION > >>>> + && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) { > >>>> + iTask->do_retry = 1; > >>>> + goto out; > >>>> + } > >>>> + > >>>> + if (status != SCSI_STATUS_GOOD) { > >>>> + error_report("iSCSI: Failure. %s", iscsi_get_error(iscsi)); > >>>> + goto out; > >>>> + } > >>>> + > >>>> +out: > >>>> + if (iTask->status != SCSI_STATUS_GOOD) { > >>>> + scsi_free_scsi_task(iTask->task); > >>>> + iTask->task = NULL; > >>>> + } > >>>> + if (iTask->co) { > >>>> + qemu_coroutine_enter(iTask->co, NULL); > >>>> + } > >>>> +} > >>> In which context does this callback run? If this is from a separate > >>> thread, you may not do all of this (without even holding the QBL). You > >>> should probably use a BH to switch to the I/O thread. > >> good question. the callbacks can only be fired when iscsi_service() is invoked. > >> this is currently done by registereing iscsi_process_read/write/flush via qemu_aio_set_fd_handler(). > >> > >> so the callbacks are invoked in the aio context it seems. can this be right? > > Yes, I took another look and I think it's fine. > Would this still be true if all aio routines are replaced by coroutines? Yes, AIO callbacks and coroutines are mostly equivalent from the point of view of anything external to the block driver itself. Kevin