From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Upxuo-00014A-Ph for qemu-devel@nongnu.org; Fri, 21 Jun 2013 05:46:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Upxuk-0006YQ-SV for qemu-devel@nongnu.org; Fri, 21 Jun 2013 05:45:58 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:39001 helo=mx01.kamp.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1Upxuk-0006Y9-Id for qemu-devel@nongnu.org; Fri, 21 Jun 2013 05:45:54 -0400 Message-ID: <51C420C4.6040503@kamp.de> Date: Fri, 21 Jun 2013 11:45:40 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1371752409-16313-1-git-send-email-pl@kamp.de> <1371752409-16313-2-git-send-email-pl@kamp.de> <20130621091842.GB2986@dhcp-200-207.str.redhat.com> In-Reply-To: <20130621091842.GB2986@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pbonzini@redhat.com, ronniesahlberg@gmail.com, qemu-devel@nongnu.org, Stefan Hajnoczi Am 21.06.2013 11:18, schrieb Kevin Wolf: > Am 20.06.2013 um 20:20 hat Peter Lieven geschrieben: >> Signed-off-by: Peter Lieven >> --- >> block/iscsi.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 0bbf0b1..e6b966d 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -49,6 +49,7 @@ typedef struct IscsiLun { >> uint64_t num_blocks; >> int events; >> QEMUTimer *nop_timer; >> + uint8_t lbpme; >> } IscsiLun; >> >> typedef struct IscsiAIOCB { >> @@ -800,6 +801,60 @@ iscsi_getlength(BlockDriverState *bs) >> return len; >> } >> >> +static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs, >> + int64_t sector_num, >> + int nb_sectors, int *pnum) >> +{ >> + IscsiLun *iscsilun = bs->opaque; >> + struct scsi_task *task = NULL; >> + struct scsi_get_lba_status *lbas = NULL; >> + struct scsi_lba_status_descriptor *lbasd = NULL; >> + int ret; >> + >> + *pnum = nb_sectors; >> + >> + if (iscsilun->lbpme == 0) { >> + return 1; >> + } >> + >> + /* in-flight requests could invalidate the lba status result */ >> + while (iscsi_process_flush(iscsilun)) { >> + qemu_aio_wait(); >> + } > Note that you're blocking here. The preferred way would be something > involving a yield from the coroutine and a reenter as soon as all > requests are done. Maybe a CoRwLock does what you need? Is there a document how to use it? Or can you help here? > >> + >> + task = iscsi_get_lba_status_sync(iscsilun->iscsi, iscsilun->lun, >> + sector_qemu2lun(sector_num, iscsilun), >> + 8+16); > Spacing around operators (8 + 16) Thanks, the checkpatch script didn't catch this. > >> + >> + if (task == NULL || task->status != SCSI_STATUS_GOOD) { >> + scsi_free_scsi_task(task); >> + return 1; > Error cases should set *pnum = 0 and return 0. In this special case, the target might not implement get_lba_status or it might return SCSI_STATUS_BUSY. We shouldn't generate an error or should we? Peter