From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37294) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V0784-0007aN-ND for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:37:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V077x-00048q-KQ for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:37:36 -0400 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:45224 helo=mx01.kamp.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1V077x-00047q-5a for qemu-devel@nongnu.org; Fri, 19 Jul 2013 05:37:29 -0400 Message-ID: <51E908DD.2030404@kamp.de> Date: Fri, 19 Jul 2013 11:37:33 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1374218381-18597-1-git-send-email-pl@kamp.de> <1374218381-18597-4-git-send-email-pl@kamp.de> <51E9044C.80001@redhat.com> In-Reply-To: <51E9044C.80001@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv5 3/3] iscsi: add .bdrv_co_discard List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, ronniesahlberg@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com On 19.07.2013 11:18, Paolo Bonzini wrote: > Il 19/07/2013 09:19, Peter Lieven ha scritto: >> Signed-off-by: Peter Lieven >> --- >> block/iscsi.c | 156 +++++++++++++++++++++++++++------------------------------ >> 1 file changed, 73 insertions(+), 83 deletions(-) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 19afd6c..cb44fb1 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -85,6 +85,7 @@ typedef struct IscsiAIOCB { >> #define NOP_INTERVAL 5000 >> #define MAX_NOP_FAILURES 3 >> #define ISCSI_CMD_RETRIES 5 >> +#define ISCSI_MAX_UNMAP 131072 >> >> static void >> iscsi_bh_cb(void *p) >> @@ -622,88 +623,6 @@ iscsi_aio_flush(BlockDriverState *bs, >> return &acb->common; >> } >> >> -static int iscsi_aio_discard_acb(IscsiAIOCB *acb); >> - >> -static void >> -iscsi_unmap_cb(struct iscsi_context *iscsi, int status, >> - void *command_data, void *opaque) >> -{ >> - IscsiAIOCB *acb = opaque; >> - >> - if (acb->canceled != 0) { >> - return; >> - } >> - >> - acb->status = 0; >> - if (status != 0) { >> - if (status == SCSI_STATUS_CHECK_CONDITION >> - && acb->task->sense.key == SCSI_SENSE_UNIT_ATTENTION >> - && acb->retries-- > 0) { >> - scsi_free_scsi_task(acb->task); >> - acb->task = NULL; >> - if (iscsi_aio_discard_acb(acb) == 0) { >> - iscsi_set_events(acb->iscsilun); >> - return; >> - } >> - } >> - error_report("Failed to unmap data on iSCSI lun. %s", >> - iscsi_get_error(iscsi)); >> - acb->status = -EIO; >> - } >> - >> - iscsi_schedule_bh(acb); >> -} >> - >> -static int iscsi_aio_discard_acb(IscsiAIOCB *acb) { >> - struct iscsi_context *iscsi = acb->iscsilun->iscsi; >> - struct unmap_list list[1]; >> - >> - acb->canceled = 0; >> - acb->bh = NULL; >> - acb->status = -EINPROGRESS; >> - acb->buf = NULL; >> - >> - list[0].lba = sector_qemu2lun(acb->sector_num, acb->iscsilun); >> - list[0].num = acb->nb_sectors * BDRV_SECTOR_SIZE / acb->iscsilun->block_size; >> - >> - acb->task = iscsi_unmap_task(iscsi, acb->iscsilun->lun, >> - 0, 0, &list[0], 1, >> - iscsi_unmap_cb, >> - acb); >> - if (acb->task == NULL) { >> - error_report("iSCSI: Failed to send unmap command. %s", >> - iscsi_get_error(iscsi)); >> - return -1; >> - } >> - >> - return 0; >> -} >> - >> -static BlockDriverAIOCB * >> -iscsi_aio_discard(BlockDriverState *bs, >> - int64_t sector_num, int nb_sectors, >> - BlockDriverCompletionFunc *cb, void *opaque) >> -{ >> - IscsiLun *iscsilun = bs->opaque; >> - IscsiAIOCB *acb; >> - >> - acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque); >> - >> - acb->iscsilun = iscsilun; >> - acb->nb_sectors = nb_sectors; >> - acb->sector_num = sector_num; >> - acb->retries = ISCSI_CMD_RETRIES; >> - >> - if (iscsi_aio_discard_acb(acb) != 0) { >> - qemu_aio_release(acb); >> - return NULL; >> - } >> - >> - iscsi_set_events(iscsilun); >> - >> - return &acb->common; >> -} >> - >> #ifdef __linux__ >> static void >> iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, >> @@ -985,6 +904,77 @@ out: >> return ret; >> } >> >> +static int >> +coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num, >> + int nb_sectors) >> +{ >> + IscsiLun *iscsilun = bs->opaque; >> + struct IscsiTask iTask; >> + struct unmap_list list; >> + uint32_t nb_blocks; >> + uint32_t max_unmap; >> + >> + if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { >> + return -EINVAL; >> + } >> + >> + if (!iscsilun->lbp.lbpu) { >> + /* UNMAP is not supported by the target */ >> + return 0; >> + } > I guess you'll later add support for lbpws. It depends: I think almost all targets will support both. I would use unmap for discard and write_same for write_zeroes. Peter > > Anyway, series looks fine. Thanks. Feel free to pull it once the get_lba_status thing is upstream. Peter