Re: [PATCH] Add support for SBC GET_LBA_STATUS opcode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tomo,

Please find attached an updated patch,

> Can you use put/get_unaligend_* helper functions? I don't replace the
> existing such code with the helper functions but I want to use the
> functions for new code.
>

Do you want me to prepare a patch that does this conversion for all
other places too ?


regards
ronnie sahlberg


On Sun, Jun 3, 2012 at 9:38 AM, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
> On Thu, 31 May 2012 17:00:45 +1000
> ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
>
>> Tomo, please use this patch instead.
>>
>> It adds proper check for starting-lba so we return the right sense
>> code if it is out of range.
>>
>> It also adds proper service action handling so that we will report
>> that we support both varients (reacapacity16 and getlbastatus)
>> of opcoe 0x9e
>>
>>
>> That way sg_opcodes show them both properly, like this :
>> sg_opcodes iscsi://10.1.1.125/iqn.ronnie.test/1
>>   IET       VIRTUAL-DISK  0001
>>   Peripheral device type: disk
>>
>> Opcode  Service    CDB    Name
>> (hex)   action(h)  size
>> -----------------------------------------------
>>  00                  6    Test Unit Ready
>>  03                  6    Request Sense
>>  04                  6    Format Unit
>>  08                  6    Read(6)
>>  0a                  6    Write(6)
>>  12                  6    Inquiry
>>  15                  6    Mode select(6)
>>  16                  6    Reserve(6)
>>  17                  6    Release(6)
>>  1a                  6    Mode sense(6)
>>  1b                  6    Start stop unit
>>  1d                  6    Send diagnostic
>>  1e                  6    Prevent allow medium removal
>>  25                 10    Read capacity(10)
>>  28                 10    Read(10)
>>  2a                 10    Write(10)
>>  2f                 10    Verify(10)
>>  34                 10    Pre-fetch(10)
>>  35                 10    Synchronize cache(10)
>>  41                 10    Write same(10)
>>  42                 10    Unmap
>>  55                 10    Mode select(10)
>>  5a                 10    Mode sense(10)
>>  5e        0        10    Persistent reserve in, read keys
>>  5e        1        10    Persistent reserve in, read reservation
>>  5e        2        10    Persistent reserve in, report capabilities
>>  5f        0        10    Persistent reserve out, register
>>  5f        1        10    Persistent reserve out, reserve
>>  5f        2        10    Persistent reserve out, release
>>  5f        3        10    Persistent reserve out, clear
>>  5f        4        10    Persistent reserve out, preempt
>>  5f        6        10    Persistent reserve out, register and ignore
>> existing key
>>  5f        7        10    Persistent reserve out, register and move
>>  88                 16    Read(16)
>>  8a                 16    Write(16)
>>  8f                 16    Verify(16)
>>  90                 16    Pre-fetch(16)
>>  91                 16    Synchronize cache(16)
>>  93                 16    Write same(16)
>>  9e       10        16    Read capacity(16)
>>  9e       12        16    Get LBA status
>>  a0                 12    Report luns
>>  a3        c        12    Report supported operation codes
>>  a8                 12    Read(12)
>>  aa                 12    Write(12)
>>  af                 12    Verify(12)
>>
>> tgtd does support a pretty fair number of opcodes !
>
> Looks great. Thanks a lot.
>
> I have some minor comments.
>
>> Add GET_LBA_STATUS for thin provisioned luns if SEEK_DATA/SEEK_HOLE
>> is available.
>>
>> Update get lba status to return proper sense code if starting-lba
>> is out of range.
>>
>> Break readcapacity16 and getlbastatus that are both using opcode 0x9e into
>> using the service-handle mechanism.
>> This means that the sg_opcoes now also reports both these opcodes correctly.
>> They are both opcoe 0x9e but different service actions.
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> ---
>>  usr/sbc.c  |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  usr/scsi.h |    1 +
>>  usr/spc.c  |    2 +-
>>  usr/tgtd.h |    4 ++
>>  4 files changed, 130 insertions(+), 6 deletions(-)
>>
>> diff --git a/usr/sbc.c b/usr/sbc.c
>> index cf2b609..a480a16 100644
>> --- a/usr/sbc.c
>> +++ b/usr/sbc.c
>> @@ -23,6 +23,9 @@
>>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>>   * 02110-1301 USA
>>   */
>> +#define _FILE_OFFSET_BITS 64
>> +#define __USE_GNU
>> +
>>  #include <errno.h>
>>  #include <stdio.h>
>>  #include <stdlib.h>
>> @@ -30,6 +33,7 @@
>>  #include <stdint.h>
>>  #include <unistd.h>
>>  #include <linux/fs.h>
>> +#include <sys/types.h>
>>
>>  #include "list.h"
>>  #include "util.h"
>> @@ -45,6 +49,23 @@
>>
>>  static unsigned int blk_shift = DEFAULT_BLK_SHIFT;
>>
>> +static off_t find_next_data(struct scsi_lu *dev, off_t offset)
>> +{
>> +#ifdef SEEK_DATA
>> +     return lseek64(dev->fd, offset, SEEK_DATA);
>> +#else
>> +     return offset;
>> +#endif
>> +}
>> +static off_t find_next_hole(struct scsi_lu *dev, off_t offset)
>> +{
>> +#ifdef SEEK_HOLE
>> +     return lseek64(dev->fd, offset, SEEK_HOLE);
>> +#else
>> +     return dev->size;
>> +#endif
>> +}
>> +
>>  static int sbc_mode_page_update(struct scsi_cmd *cmd, uint8_t *data, int *changed)
>>  {
>>       uint8_t pcode = data[0] & 0x3f;
>> @@ -421,7 +442,7 @@ sense:
>>       return SAM_STAT_CHECK_CONDITION;
>>  }
>>
>> -static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
>> +static int sbc_readcapacity16(int host_no, struct scsi_cmd *cmd)
>>  {
>>       uint32_t *data;
>>       unsigned int bshift;
>> @@ -437,9 +458,6 @@ static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
>>               goto sense;
>>       }
>>
>> -     if (cmd->scb[1] != SAI_READ_CAPACITY_16)
>> -             goto sense;
>> -
>>       if (scsi_get_in_length(cmd) < 12)
>>               goto overflow;
>>
>> @@ -468,6 +486,107 @@ sense:
>>       return SAM_STAT_CHECK_CONDITION;
>>  }
>>
>> +static int sbc_getlbastatus(int host_no, struct scsi_cmd *cmd)
>> +{
>> +     int len = 32;
>> +     uint64_t offset;
>> +     uint32_t pdl;
>> +     int type;
>> +     char *buf;
>> +     unsigned char key = ILLEGAL_REQUEST;
>> +     uint16_t asc = ASC_INVALID_OP_CODE;
>> +
>> +     if (cmd->dev->attrs.removable && !cmd->dev->attrs.online) {
>> +             key = NOT_READY;
>> +             asc = ASC_MEDIUM_NOT_PRESENT;
>> +             goto sense;
>> +     }
>> +
>> +     if (scsi_get_in_length(cmd) < 24)
>> +             goto overflow;
>> +
>> +     len = scsi_get_in_length(cmd);
>> +     buf = scsi_get_in_buffer(cmd);
>> +     memset(buf, 0, len);
>> +
>> +     offset = __be64_to_cpu(*(uint64_t *)&cmd->scb[2])
>> +                     << cmd->dev->blk_shift;
>
> Can you use put/get_unaligend_* helper functions? I don't replace the
> existing such code with the helper functions but I want to use the
> functions for new code.
>
>
>> +     if (offset >= cmd->dev->size) {
>> +             key = ILLEGAL_REQUEST;
>> +             asc = ASC_LBA_OUT_OF_RANGE;
>> +             goto sense;
>> +     }
>> +
>> +     pdl = 4;
>> +     *(uint32_t *)&buf[0] = __cpu_to_be32(pdl);
>
> Ditto.
>
>> +     type = 0;
>> +     while (len >= 4 + pdl + 16) {
>> +             off_t next_offset;
>> +
>> +             *(uint32_t *)&buf[0] = __cpu_to_be32(pdl + 16);
>
> Ditto.
>
>> +             if (offset >= cmd->dev->size)
>> +                     break;
>> +
>> +             next_offset = (type == 0) ?
>> +                     find_next_hole(cmd->dev, offset) :
>> +                     find_next_data(cmd->dev, offset);
>> +             if (next_offset == offset) {
>> +                     type = 1 - type;
>> +                     continue;
>> +             }
>> +
>> +             *(uint64_t *)&buf[4 + pdl +  0] =
>> +                     __cpu_to_be64(offset >> cmd->dev->blk_shift);
>> +             *(uint64_t *)&buf[4 + pdl +  8] =
>> +                     __cpu_to_be32((next_offset - offset)
>> +                             >> cmd->dev->blk_shift);
>
> Ditto.
>
>> +             buf[4 + pdl + 12] = type;
>
> Ditto.
>
>> +             pdl += 16;
>> +             type = 1 - type;
>> +             offset = next_offset;
>> +     }
>> +     len = 4 + pdl;
>> +
>> +overflow:
>> +     scsi_set_in_resid_by_actual(cmd, len);
>> +     return SAM_STAT_GOOD;
>> +
>> +sense:
>> +     sense_data_build(cmd, key, asc);
>> +     return SAM_STAT_CHECK_CONDITION;
>> +}
>> +
>> +struct service_action sbc_service_actions[] = {
>> +     {0x10, sbc_readcapacity16},
>> +     {0x12, sbc_getlbastatus},
>
> better to use SAI_*?
>
>> +     {0, NULL}
>> +};
>> +
>> +
>> +static int sbc_service_action(int host_no, struct scsi_cmd *cmd)
>> +{
>> +     uint8_t action;
>> +     unsigned char op = cmd->scb[0];
>> +     struct service_action *service_action, *actions;
>> +
>> +     action = cmd->scb[1] & 0x1f;
>> +     actions = cmd->dev->dev_type_template.ops[op].service_actions;
>> +
>> +     service_action = find_service_action(actions, action);
>> +
>> +     if (!service_action) {
>> +             scsi_set_in_resid_by_actual(cmd, 0);
>> +             sense_data_build(cmd, ILLEGAL_REQUEST,
>> +                             ASC_INVALID_FIELD_IN_CDB);
>> +             return SAM_STAT_CHECK_CONDITION;
>> +     }
>> +
>> +     return service_action->cmd_perform(host_no, cmd);
>> +}
>> +
>>  static int sbc_sync_cache(int host_no, struct scsi_cmd *cmd)
>>  {
>>       int ret;
>> @@ -711,7 +830,7 @@ static struct device_type_template sbc_template = {
>>               {spc_illegal_op,},
>>               {spc_illegal_op,},
>>               {spc_illegal_op,},
>> -             {sbc_service_action,},
>> +             {sbc_service_action, sbc_service_actions,},
>>               {spc_illegal_op,},
>>
>>               /* 0xA0 */
>> diff --git a/usr/scsi.h b/usr/scsi.h
>> index 0a02c36..2b994f9 100644
>> --- a/usr/scsi.h
>> +++ b/usr/scsi.h
>> @@ -80,6 +80,7 @@
>>  #define WRITE_SAME_16         0x93
>>  #define SERVICE_ACTION_IN     0x9e
>>  #define      SAI_READ_CAPACITY_16  0x10
>> +#define      SAI_GET_LBA_STATUS    0x12
>>  #define REPORT_LUNS           0xa0
>>  #define MOVE_MEDIUM           0xa5
>>  #define EXCHANGE_MEDIUM       0xa6
>> diff --git a/usr/spc.c b/usr/spc.c
>> index a7f9a36..117c9f3 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -794,7 +794,7 @@ struct service_action maint_in_service_actions[] = {
>>       {0, NULL}
>>  };
>>
>> -static struct service_action *
>> +struct service_action *
>>  find_service_action(struct service_action *service_action, uint32_t action)
>>  {
>>       while (service_action->cmd_perform) {
>> diff --git a/usr/tgtd.h b/usr/tgtd.h
>> index b303e21..aa9b9d5 100644
>> --- a/usr/tgtd.h
>> +++ b/usr/tgtd.h
>> @@ -353,4 +353,8 @@ int call_program(const char *cmd,
>>
>>  void update_lbppbe(struct scsi_lu *lu, int blksize);
>>
>> +struct service_action *
>> +find_service_action(struct service_action *service_action,
>> +                 uint32_t action);
>> +
>>  #endif
>> --
>> 1.7.3.1
>

Attachment: 0001-SBC-Add-GET_LBA_STATUS-command.patch.gz
Description: GNU Zip compressed data

Attachment: 0001-SBC-Add-GET_LBA_STATUS-command.patch
Description: Binary data


[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux