Re: [Qemu-devel] [PATCH RESEND] block/iscsi: query for supported VPD pages

[Paolo Bonzini <pbonzini@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 <pl@kamp.de>

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 */
>