From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42161) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFSKZ-0002rV-Fl for qemu-devel@nongnu.org; Mon, 17 Feb 2014 12:50:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFSKU-00072x-Ix for qemu-devel@nongnu.org; Mon, 17 Feb 2014 12:50:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10613) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFSKU-00070u-BQ for qemu-devel@nongnu.org; Mon, 17 Feb 2014 12:50:06 -0500 Message-ID: <53024BC1.9050601@redhat.com> Date: Mon, 17 Feb 2014 18:49:53 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1392658448-6650-1-git-send-email-pl@kamp.de> In-Reply-To: <1392658448-6650-1-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RESEND] block/iscsi: query for supported VPD pages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven , qemu-devel@nongnu.org Cc: kwolf@redhat.com, ronniesahlberg@gmail.com, stefanha@redhat.com Il 17/02/2014 18:34, Peter Lieven ha scritto: > this patch ensures that we only query for block provisioning and > block limits vpd pages if they are advertised. It also cleans > up the inquiry code and eliminates some redundant code. > > Signed-off-by: Peter Lieven Thanks, I've queued it. It conflicts with my bdrv_open error series, but I'll handle the rebase. Paolo > --- > block/iscsi.c | 107 +++++++++++++++++++++++++++++---------------------------- > 1 file changed, 54 insertions(+), 53 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 6f4af72..38b07f8 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1066,7 +1066,8 @@ static QemuOptsList runtime_opts = { > }; > > static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, > - int lun, int evpd, int pc) { > + int lun, int evpd, int pc, > + void **inq) { > int full_size; > struct scsi_task *task = NULL; > task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64); > @@ -1084,14 +1085,18 @@ static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, > } > } > > + *inq = scsi_datain_unmarshall(task); > + if (*inq == NULL) { > + goto fail; > + } > + > return task; > > fail: > error_report("iSCSI: Inquiry command failed : %s", > iscsi_get_error(iscsi)); > - if (task) { > + if (task != NULL) { > scsi_free_scsi_task(task); > - return NULL; > } > return NULL; > } > @@ -1108,11 +1113,12 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > struct iscsi_url *iscsi_url = NULL; > struct scsi_task *task = NULL; > struct scsi_inquiry_standard *inq = NULL; > + struct scsi_inquiry_supported_pages *inq_vpd; > char *initiator_name = NULL; > QemuOpts *opts; > Error *local_err = NULL; > const char *filename; > - int ret; > + int i, ret; > > if ((BDRV_SECTOR_SIZE % 512) != 0) { > error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. " > @@ -1132,7 +1138,6 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > > filename = qemu_opt_get(opts, "filename"); > > - > iscsi_url = iscsi_parse_full_url(iscsi, filename); > if (iscsi_url == NULL) { > error_report("Failed to parse URL : %s", filename); > @@ -1194,24 +1199,17 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > > iscsilun->iscsi = iscsi; > iscsilun->lun = iscsi_url->lun; > + iscsilun->has_write_same = true; > > - task = iscsi_inquiry_sync(iscsi, iscsilun->lun, 0, 0, 36); > - > - if (task == NULL || task->status != SCSI_STATUS_GOOD) { > - error_report("iSCSI: failed to send inquiry command."); > - ret = -EINVAL; > - goto out; > - } > - > - inq = scsi_datain_unmarshall(task); > - if (inq == NULL) { > - error_report("iSCSI: Failed to unmarshall inquiry data."); > + task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 0, 0, > + (void **) &inq); > + if (task == NULL) { > ret = -EINVAL; > goto out; > } > - > iscsilun->type = inq->periperal_device_type; > - iscsilun->has_write_same = true; > + scsi_free_scsi_task(task); > + task = NULL; > > if ((ret = iscsi_readcapacity_sync(iscsilun)) != 0) { > goto out; > @@ -1228,45 +1226,48 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, > bs->sg = 1; > } > > - if (iscsilun->lbpme) { > - struct scsi_inquiry_logical_block_provisioning *inq_lbp; > - task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, > - SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING); > - if (task == NULL) { > - ret = -EINVAL; > - goto out; > - } > - inq_lbp = scsi_datain_unmarshall(task); > - if (inq_lbp == NULL) { > - error_report("iSCSI: failed to unmarshall inquiry datain blob"); > - ret = -EINVAL; > - goto out; > - } > - memcpy(&iscsilun->lbp, inq_lbp, > - sizeof(struct scsi_inquiry_logical_block_provisioning)); > - scsi_free_scsi_task(task); > - task = NULL; > + task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, > + SCSI_INQUIRY_PAGECODE_SUPPORTED_VPD_PAGES, > + (void **) &inq_vpd); > + if (task == NULL) { > + ret = -EINVAL; > + goto out; > } > - > - if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { > + for (i = 0; i < inq_vpd->num_pages; i++) { > + struct scsi_task *inq_task; > + struct scsi_inquiry_logical_block_provisioning *inq_lbp; > struct scsi_inquiry_block_limits *inq_bl; > - task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, > - SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS); > - if (task == NULL) { > - ret = -EINVAL; > - goto out; > - } > - inq_bl = scsi_datain_unmarshall(task); > - if (inq_bl == NULL) { > - error_report("iSCSI: failed to unmarshall inquiry datain blob"); > - ret = -EINVAL; > - goto out; > + switch (inq_vpd->pages[i]) { > + case SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING: > + inq_task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, > + SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING, > + (void **) &inq_lbp); > + if (inq_task == NULL) { > + ret = -EINVAL; > + goto out; > + } > + memcpy(&iscsilun->lbp, inq_lbp, > + sizeof(struct scsi_inquiry_logical_block_provisioning)); > + scsi_free_scsi_task(inq_task); > + break; > + case SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS: > + inq_task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, > + SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS, > + (void **) &inq_bl); > + if (inq_task == NULL) { > + ret = -EINVAL; > + goto out; > + } > + memcpy(&iscsilun->bl, inq_bl, > + sizeof(struct scsi_inquiry_block_limits)); > + scsi_free_scsi_task(inq_task); > + break; > + default: > + break; > } > - memcpy(&iscsilun->bl, inq_bl, > - sizeof(struct scsi_inquiry_block_limits)); > - scsi_free_scsi_task(task); > - task = NULL; > } > + scsi_free_scsi_task(task); > + task = NULL; > > #if defined(LIBISCSI_FEATURE_NOP_COUNTER) > /* Set up a timer for sending out iSCSI NOPs */ >