Re: [Qemu-devel] [PATCHv2 02/11] iscsi: read unmap info from block limits vpd page

[ronnie sahlberg]

max_unmap :

If the target does not return an explicit limit for max_unmap it will
return 0xffffffff which means "no limit".

I think you should add a check for max_unmap and clamp it down to
something sane.
Maybe a maximum of 128M ?

Same for bdc, that should also be checked and clamped down to sane values.


On Thu, Jun 27, 2013 at 11:11 PM, Peter Lieven <address@hidden> wrote:
> Signed-off-by: Peter Lieven <address@hidden>
> ---
>  block/iscsi.c |   80 
> ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a38a1bf..2e2455d 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -54,6 +54,8 @@ typedef struct IscsiLun {
>      uint8_t lbpu;
>      uint8_t lbpws;
>      uint8_t lbpws10;
> +    uint32_t max_unmap;
> +    uint32_t max_unmap_bdc;
>  } IscsiLun;
>
>  typedef struct IscsiAIOCB {
> @@ -1007,6 +1009,37 @@ static QemuOptsList runtime_opts = {
>      },
>  };
>
> +static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi,
> +                                          int lun, int evpd, int pc) {
> +        int full_size;
> +        struct scsi_task *task = NULL;
> +        task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64);
> +        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> +            goto fail;
> +        }
> +        full_size = scsi_datain_getfullsize(task);
> +        if (full_size > task->datain.size) {
> +            scsi_free_scsi_task(task);
> +
> +            /* we need more data for the full list */
> +            task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size);
> +            if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> +                goto fail;
> +            }
> +        }
> +
> +        return task;
> +
> +fail:
> +        error_report("iSCSI: Inquiry command failed : %s",
> +                     iscsi_get_error(iscsi));
> +        if (task) {
> +            scsi_free_scsi_task(task);
> +            return NULL;
> +        }
> +        return NULL;
> +}
> +
>  /*
>   * We support iscsi url's on the form
>   * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> @@ -1139,33 +1172,12 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags)
>
>      if (iscsilun->lbpme) {
>          struct scsi_inquiry_logical_block_provisioning *inq_lbp;
> -        int full_size;
> -
> -        task = iscsi_inquiry_sync(iscsi, iscsilun->lun, 1,
> -                                  
> SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING,
> -                                  64);
> -        if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> -            error_report("iSCSI: Inquiry command failed : %s",
> -                   iscsi_get_error(iscsilun->iscsi));
> +        task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1,
> +                                
> SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING);
> +        if (task == NULL) {
>              ret = -EINVAL;
>              goto out;
>          }
> -        full_size = scsi_datain_getfullsize(task);
> -        if (full_size > task->datain.size) {
> -            scsi_free_scsi_task(task);
> -
> -            /* we need more data for the full list */
> -            task = iscsi_inquiry_sync(iscsi, iscsilun->lun, 1,
> -                                      
> SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING,
> -                                      full_size);
> -            if (task == NULL || task->status != SCSI_STATUS_GOOD) {
> -                error_report("iSCSI: Inquiry command failed : %s",
> -                             iscsi_get_error(iscsilun->iscsi));
> -                ret = -EINVAL;
> -                goto out;
> -            }
> -        }
> -
>          inq_lbp = scsi_datain_unmarshall(task);
>          if (inq_lbp == NULL) {
>              error_report("iSCSI: failed to unmarshall inquiry datain blob");
> @@ -1175,6 +1187,26 @@ static int iscsi_open(BlockDriverState *bs, QDict 
> *options, int flags)
>          iscsilun->lbpu = inq_lbp->lbpu;
>          iscsilun->lbpws = inq_lbp->lbpws;
>          iscsilun->lbpws10 = inq_lbp->lbpws10;
> +        scsi_free_scsi_task(task);
> +        task = NULL;
> +    }
> +
> +    if (iscsilun->lbpu) {
> +        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;
> +        }
> +        iscsilun->max_unmap = inq_bl->max_unmap;
> +        iscsilun->max_unmap_bdc = inq_bl->max_unmap_bdc;
>      }
>
>  #if defined(LIBISCSI_FEATURE_NOP_COUNTER)
> --
> 1.7.9.5
>