[stgt] [PATCHes] Updated patches for thin-provisioning support

ronnie sahlberg ronniesahlberg at gmail.com
Tue Apr 17 09:36:32 CEST 2012


On Tue, Apr 17, 2012 at 1:55 PM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> On Sun, 15 Apr 2012 13:48:12 +1000
> ronnie sahlberg <ronniesahlberg at gmail.com> wrote:
>
>> From e58b5bf485c3eee24e04970b05aaba4472d0fa35 Mon Sep 17 00:00:00 2001
>> From: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> Date: Sun, 15 Apr 2012 12:07:32 +1000
>> Subject: [PATCH 1/2] SBC UNMAP: Add support for thin-provisioning and the UNMAP command.
>>
>> The UNMAP command is implemented using FALLOC_FL_PUNCH_HOLE and will
>> release UNMAPPED blocks back to the underlying filesystem.
>>
>> FALLOC_FL_PUNCH_HOLE is fairly new addition to Linux but works on
>> ext4 and XFS filesystems currently.
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> ---
>>  doc/tgtadm.8.xml |   25 ++++++++++++++++++
>>  usr/bs_rdwr.c    |   54 ++++++++++++++++++++++++++++++++++++++
>>  usr/sbc.c        |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  usr/scsi.h       |    1 +
>>  usr/spc.c        |   43 ++++++++++++++++++++++++++++---
>>  usr/target.c     |    2 +
>>  usr/tgtd.h       |    2 +
>>  7 files changed, 197 insertions(+), 5 deletions(-)
>>
>> diff --git a/doc/tgtadm.8.xml b/doc/tgtadm.8.xml
>> index 668e184..a40f659 100644
>> --- a/doc/tgtadm.8.xml
>> +++ b/doc/tgtadm.8.xml
>> @@ -352,6 +352,31 @@ tgtadm --lld iscsi --mode logicalunit --op update --tid 1 --lun 1 \
>>           --params readonly=1
>>        </screen>
>>
>> +      <varlistentry><term><option>thin_provisioning=<0|1></option></term>
>> +        <listitem>
>> +          <para>
>> +         This controls the provisioning for the LUN. A thin-provisioned
>> +         LUN is represented as a sparse file.
>> +         TGTD supports provisioning type 2 for sparse files.
>> +         When initiators use the SCSI UNMAP command TGTD will release
>> +         the affected areas back to the filesystem using
>> +         FALLOC_FL_PUNCH_HOLE.
>> +          </para>
>> +          <para>
>> +         This parameter only applies to DISK devices.
>> +          </para>
>> +          <para>
>> +         Thin-provisioning only works for LUNs stored on filesystems
>> +         that support FALLOC_FL_PUNCH_HOLE.
>> +          </para>
>> +        </listitem>
>> +      </varlistentry>
>> +
>> +      <screen format="linespecific">
>> +tgtadm --lld iscsi --mode logicalunit --op update --tid 1 --lun 1 \
>> +         --params thin_provisioning=1
>> +      </screen>
>> +
>>      </variablelist>
>>    </refsect1>
>>
>> diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
>> index 84ed278..9e15603 100644
>> --- a/usr/bs_rdwr.c
>> +++ b/usr/bs_rdwr.c
>> @@ -27,6 +27,8 @@
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>>  #include <unistd.h>
>>
>>  #include <linux/fs.h>
>> @@ -160,6 +162,58 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
>>
>>               free(tmpbuf);
>>               break;
>> +     case UNMAP:
>> +             if (!cmd->dev->attrs.thinprovisioning) {
>> +                     result = SAM_STAT_CHECK_CONDITION;
>> +                     key = ILLEGAL_REQUEST;
>> +                     asc = ASC_INVALID_FIELD_IN_CDB;
>> +                     break;
>> +             }
>> +
>> +             length = scsi_get_out_length(cmd);
>> +             tmpbuf = scsi_get_out_buffer(cmd);
>> +
>> +             if (length < 8)
>> +                     break;
>> +
>> +             length -= 8;
>> +             tmpbuf += 8;
>> +
>> +             while (length >= 16) {
>> +                     uint64_t offset;
>> +                     uint64_t len;
>> +
>> +                     offset = get_unaligned_be64(&tmpbuf[0]);
>> +                     offset = offset << cmd->dev->blk_shift;
>> +
>> +                     len = get_unaligned_be32(&tmpbuf[8]);
>> +                     len = len << cmd->dev->blk_shift;
>> +
>> +                     if (offset + len > cmd->dev->size) {
>> +                             eprintf("UNMAP beyond EOF\n");
>> +                             result = SAM_STAT_CHECK_CONDITION;
>> +                             key = ILLEGAL_REQUEST;
>> +                             asc = ASC_LBA_OUT_OF_RANGE;
>> +                             break;
>> +                     }
>> +
>> +                     if (len > 0) {
>
> We should check len right after getting it and return sense if it's
> invalid?

I think this is ok.

SBC 5.29.2  :
...
If the NUMBER OF LOGICAL BLOCKS is set to zero, then no LBAs shall be
unmapped for this UNMAP block
descriptor. This condition shall not be considered an error.
...


>
>
>> +                             if (unmap_file_region(fd, offset, len) != 0) {
>> +                                     eprintf("Failed to punch hole for"
>> +                                             " UNMAP at offset:%" PRIu64
>> +                                             " length:%" PRIu64 "\n",
>> +                                             offset, len);
>> +                                     result = SAM_STAT_CHECK_CONDITION;
>> +                                     key = HARDWARE_ERROR;
>> +                                     asc = ASC_INTERNAL_TGT_FAILURE;
>> +                                     break;
>> +                             }
>> +                     }
>> +
>> +                     length -= 16;
>> +                     tmpbuf += 16;
>> +             }
>> +             break;
>>       default:
>>               break;
>>       }
>> diff --git a/usr/sbc.c b/usr/sbc.c
>> index 0c3681e..55145cf 100644
>> --- a/usr/sbc.c
>> +++ b/usr/sbc.c
>> @@ -141,6 +141,60 @@ sense:
>>       return SAM_STAT_CHECK_CONDITION;
>>  }
>>
>> +static int sbc_unmap(int host_no, struct scsi_cmd *cmd)
>> +{
>> +     int ret;
>> +     unsigned char key = ILLEGAL_REQUEST;
>> +     uint16_t asc = ASC_LUN_NOT_SUPPORTED;
>> +     struct scsi_lu *lu = cmd->dev;
>> +     int anchor;
>> +
>> +     ret = device_reserved(cmd);
>> +     if (ret)
>> +             return SAM_STAT_RESERVATION_CONFLICT;
>> +
>> +     /* We dont support anchored blocks */
>> +     anchor = cmd->scb[1] & 0x01;
>> +     if (anchor) {
>> +             key = ILLEGAL_REQUEST;
>> +             asc = ASC_INVALID_FIELD_IN_CDB;
>> +             goto sense;
>> +     }
>> +
>> +     if (!lu->attrs.thinprovisioning) {
>> +             key = ILLEGAL_REQUEST;
>> +             asc = ASC_INVALID_OP_CODE;
>> +             goto sense;
>> +     }
>> +
>> +     if (lu->attrs.removable && !lu->attrs.online) {
>> +             key = NOT_READY;
>> +             asc = ASC_MEDIUM_NOT_PRESENT;
>> +             goto sense;
>> +     }
>> +
>> +     if (lu->attrs.readonly) {
>> +             key = DATA_PROTECT;
>> +             asc = ASC_WRITE_PROTECT;
>> +             goto sense;
>> +     }
>> +
>> +     ret = cmd->dev->bst->bs_cmd_submit(cmd);
>> +     if (ret) {
>> +             key = HARDWARE_ERROR;
>> +             asc = ASC_INTERNAL_TGT_FAILURE;
>> +             goto sense;
>> +     }
>> +
>> +sense:
>> +     cmd->offset = 0;
>> +     scsi_set_in_resid_by_actual(cmd, 0);
>> +     scsi_set_out_resid_by_actual(cmd, 0);
>> +
>> +     sense_data_build(cmd, key, asc);
>> +     return SAM_STAT_CHECK_CONDITION;
>> +}
>> +
>>  static int sbc_rw(int host_no, struct scsi_cmd *cmd)
>>  {
>>       int ret;
>> @@ -370,6 +424,8 @@ static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
>>       data[2] = __cpu_to_be32(1UL << bshift);
>>
>>       val = (cmd->dev->attrs.lbppbe << 16) | cmd->dev->attrs.la_lba;
>> +     if (cmd->dev->attrs.thinprovisioning)
>> +             val |= (3 << 14); /* set LBPME and LBPRZ */
>>       data[3] = __cpu_to_be32(val);
>>
>>  overflow:
>> @@ -549,7 +605,24 @@ static struct device_type_template sbc_template = {
>>               {spc_illegal_op,},
>>               {spc_illegal_op,},
>>
>> -             [0x40 ... 0x4f] = {spc_illegal_op,},
>> +             /* 0x40 */
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {sbc_unmap,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>> +             {spc_illegal_op,},
>>
>>               /* 0x50 */
>>               {spc_illegal_op,},
>> diff --git a/usr/scsi.h b/usr/scsi.h
>> index ca1109a..1508cc6 100644
>> --- a/usr/scsi.h
>> +++ b/usr/scsi.h
>> @@ -60,6 +60,7 @@
>>  #define WRITE_LONG            0x3f
>>  #define CHANGE_DEFINITION     0x40
>>  #define WRITE_SAME            0x41
>> +#define UNMAP                      0x42
>>  #define READ_TOC              0x43
>>  #define LOG_SELECT            0x4c
>>  #define LOG_SENSE             0x4d
>> diff --git a/usr/spc.c b/usr/spc.c
>> index 8d40139..459993c 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -139,6 +139,24 @@ static void update_vpd_83(struct scsi_lu *lu, void *id)
>>       strncpy((char *)data + 4, id, SCSI_ID_LEN);
>>  }
>>
>> +static void update_vpd_b2(struct scsi_lu *lu, void *id)
>> +{
>> +     struct vpd *vpd_pg = lu->attrs.lu_vpd[PCODE_OFFSET(0xb2)];
>> +     uint8_t *data = vpd_pg->data;
>> +
>> +     if (lu->attrs.thinprovisioning) {
>> +             data[0] = 0;            /* threshold exponent */
>
> Should be non-zero? What version of sbc spec you read? Mine might be
> old.

Treshold exponent set ot 0 just means we dont support tresholds.

SBC 6.5.4 :
...
The THRESHOLD EXPONENT field indicates the threshold set size in LBAs
as a power of 2 (i.e., the threshold set
size is equal to 2(threshold exponent)). A THRESHOLD EXPONENT field
set to zero indicates that the logical unit does
not support logical block provisioning thresholds.
...


>
>> +             data[1] = 0x84;         /* LBPU LBPRZ */
>> +             data[2] = 0x02;         /* provisioning type */
>> +             data[3] = 0;
>> +     } else {
>> +             data[0] = 0;
>> +             data[1] = 0;
>> +             data[2] = 0;
>> +             data[3] = 0;
>> +     }
>> +}
>> +
>>  static void update_b0_opt_xfer_gran(struct scsi_lu *lu, int opt_xfer_gran)
>>  {
>>       struct vpd *vpd_pg = lu->attrs.lu_vpd[PCODE_OFFSET(0xb0)];
>> @@ -1641,7 +1659,7 @@ enum {
>>       Opt_removable, Opt_readonly, Opt_online,
>>       Opt_mode_page,
>>       Opt_path,
>> -     Opt_bsoflags,
>> +     Opt_bsoflags, Opt_thinprovisioning,
>>       Opt_err,
>>  };
>>
>> @@ -1662,6 +1680,7 @@ static match_table_t tokens = {
>>       {Opt_mode_page, "mode_page=%s"},
>>       {Opt_path, "path=%s"},
>>       {Opt_bsoflags, "bsoflags=%s"},
>> +     {Opt_thinprovisioning, "thin_provisioning=%s"},
>>       {Opt_err, NULL},
>>  };
>>
>> @@ -1752,6 +1771,12 @@ tgtadm_err lu_config(struct scsi_lu *lu, char *params, match_fn_t *fn)
>>                       match_strncpy(buf, &args[0], sizeof(buf));
>>                       attrs->readonly = atoi(buf);
>>                       break;
>> +             case Opt_thinprovisioning:
>> +                     match_strncpy(buf, &args[0], sizeof(buf));
>> +                     attrs->thinprovisioning = atoi(buf);
>> +                     /* update the provisioning vpd page */
>> +                     lu_vpd[PCODE_OFFSET(0xb2)]->vpd_update(lu, NULL);
>> +                     break;
>>               case Opt_online:
>>                       match_strncpy(buf, &args[0], sizeof(buf));
>>                       if (atoi(buf))
>> @@ -1787,6 +1812,10 @@ int spc_lu_init(struct scsi_lu *lu)
>>
>>       lu->attrs.device_type = lu->dev_type_template.type;
>>       lu->attrs.qualifier = 0x0;
>> +     lu->attrs.thinprovisioning = 0;
>> +     lu->attrs.removable = 0;
>> +     lu->attrs.readonly = 0;
>> +     lu->attrs.sense_format = 0;
>>
>>       snprintf(lu->attrs.vendor_id, sizeof(lu->attrs.vendor_id),
>>                "%-16s", VENDOR_ID);
>> @@ -1819,9 +1848,15 @@ int spc_lu_init(struct scsi_lu *lu)
>>       if (!lu_vpd[pg])
>>               return -ENOMEM;
>>
>> -     lu->attrs.removable = 0;
>> -     lu->attrs.readonly = 0;
>> -     lu->attrs.sense_format = 0;
>> +     /* VPD page 0xb2 LOGICAL BLOCK PROVISIONING*/
>> +     pg = PCODE_OFFSET(0xb2);
>> +     lu_vpd[pg] = alloc_vpd(LBP_VPD_LEN);
>> +     if (!lu_vpd[pg])
>> +             return -ENOMEM;
>> +     lu_vpd[pg]->vpd_update = update_vpd_b2;
>> +     lu_vpd[pg]->vpd_update(lu, NULL);
>> +
>> +
>>       lu->dev_type_template.lu_offline(lu);
>>
>>       return 0;
>> diff --git a/usr/target.c b/usr/target.c
>> index 0b6be13..dd3ca91 100644
>> --- a/usr/target.c
>> +++ b/usr/target.c
>> @@ -1857,6 +1857,7 @@ tgtadm_err tgt_target_show_all(struct concat_buf *b)
>>                                _TAB3 "Removable media: %s\n"
>>                                _TAB3 "Prevent removal: %s\n"
>>                                _TAB3 "Readonly: %s\n"
>> +                              _TAB3 "Thin-provisioning: %s\n"
>>                                _TAB3 "Backing store type: %s\n"
>>                                _TAB3 "Backing store path: %s\n"
>>                                _TAB3 "Backing store flags: %s\n",
>> @@ -1871,6 +1872,7 @@ tgtadm_err tgt_target_show_all(struct concat_buf *b)
>>                                lu_prevent_removal(lu) ?
>>                                       "Yes" : "No",
>>                                lu->attrs.readonly ? "Yes" : "No",
>> +                              lu->attrs.thinprovisioning ? "Yes" : "No",
>>                                lu->bst ?
>>                                       (lu->bst->bs_name ? : "Unknown") :
>>                                       "None",
>> diff --git a/usr/tgtd.h b/usr/tgtd.h
>> index 726a3f5..03036ba 100644
>> --- a/usr/tgtd.h
>> +++ b/usr/tgtd.h
>> @@ -14,6 +14,7 @@ struct concat_buf;
>>  #define PRODUCT_ID_LEN               16
>>  #define PRODUCT_REV_LEN              4
>>  #define BLOCK_LIMITS_VPD_LEN 0x3C
>> +#define LBP_VPD_LEN          4
>>
>>  #define PCODE_SHIFT          7
>>  #define PCODE_OFFSET(x) (x & ((1 << PCODE_SHIFT) - 1))
>> @@ -72,6 +73,7 @@ struct lu_phy_attr {
>>       char qualifier;         /* Peripheral Qualifier */
>>       char removable;         /* Removable media */
>>       char readonly;          /* Read-Only media */
>> +     char thinprovisioning;  /* Use thin-provisioning for this LUN */
>>       char online;            /* Logical Unit online */
>>       char sense_format;      /* Descrptor format sense data supported */
>>                               /* For the following see READ CAPACITY (16) */
>> --
>> 1.7.3.1
>>
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



More information about the stgt mailing list