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
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
