Re: [PATCH] Add support for SBC GET_LBA_STATUS opcode

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

 



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


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

  Powered by Linux