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
