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

[Peter Lieven <pl@kamp.de>]

Am 17.02.2014 18:26, schrieb Paolo Bonzini:
> Il 17/02/2014 18:09, Peter Lieven ha scritto:
>> ping ;-)
>
> Looks good, but I cannot find the original post.
strange, sent it to qemu-devel on Jan/29.

Peter
>
> Paolo
>
>> Am 29.01.2014 12:56, schrieb Peter Lieven:
>>> 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>
>>> ---
>>>  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 */
>>
>>
>>
>