[Stgt-devel] PATCH 2 of 6 - tgt core patch

FUJITA Tomonori fujita.tomonori
Sun May 13 09:37:49 CEST 2007


From: "Mark Harvey" <markh794 at gmail.com>
Subject: [Stgt-devel] PATCH 2 of 6 - tgt core patch
Date: Fri, 11 May 2007 13:17:44 +1000

Thanks a lot for your work.

Can we focus on tgt core changes first? If VTL code doesn't break tgt,
I'm happy to merge it.


> diff --git a/doc/tgt-setup b/doc/tgt-setup
> index e27f104..bb1074c 100755
> --- a/doc/tgt-setup
> +++ b/doc/tgt-setup
> @@ -2,7 +2,7 @@
>  
>  set -x
>  
> -export PATH=`pwd`/usr:$PATH
> +export PATH=`pwd`/../usr:$PATH
>  
>  P=`ps -ef|grep -v grep|grep tgtd|wc -l`
>  if [ "X"$P == "X0" ]; then

I think that we need a new directory for scripts.


> diff --git a/usr/Makefile b/usr/Makefile
> index 7ff74cb..62d0805 100644
> --- a/usr/Makefile
> +++ b/usr/Makefile
> @@ -61,3 +61,9 @@ endif
>  
>  clean:
>  	rm -f *.o $(PROGRAMS) iscsi/*.o ibmvio/*.o xen/*.o
> +
> +distclean:
> +	rm -f *.o $(PROGRAMS) iscsi/*.o ibmvio/*.o xen/*.o TAGS *.rej

We need this? git-clean ?


> +tags:
> +	etags *.c *.h iscsi/*.c iscsi/*.h ibmvio/*.c xen/*.c xen/*.h

I use gtags instead of etags. There are various tools for tags so I
don't think that we need something like this.


> diff --git a/usr/mmc.c b/usr/mmc.c
> index e9cc479..d53d0f3 100644
> --- a/usr/mmc.c
> +++ b/usr/mmc.c
> @@ -121,10 +121,24 @@ static int mmc_read_capacity(int host_no, struct scsi_cmd *cmd)
>  	return SAM_STAT_GOOD;
>  }
>  
> +static int device_mmc_init(struct scsi_lu *lu)
> +{
> +	lu->d_sense = 1;
> +	return 0;
> +}
> +
> +static int device_mmc_shutdown(struct scsi_lu *lu)
> +{
> +	return 0;
> +}
> +
>  struct device_type_template mmc_template = {
>  	.type	= TYPE_ROM,
>  	.name	= "cdrom/dvd",
>  	.pid	= "VIRTUAL-CDROM",
> +	.device_init = device_mmc_init,
> +	.device_shutdown = device_mmc_shutdown,
> +	.device_config = spc_device_config,
>  	.ops	= {
>  		{spc_test_unit,},
>  		{spc_illegal_op,},
> diff --git a/usr/osd.c b/usr/osd.c
> index 46bf0a0..64e7b2a 100644
> --- a/usr/osd.c
> +++ b/usr/osd.c
> @@ -145,15 +145,22 @@ static int osd_varlen_cdb(int host_no, struct scsi_cmd *cmd)
>  	return cmd->c_target->bst->bs_cmd_submit(cmd);
>  }
>  
> -static void device_osd_init(struct scsi_lu *lu)
> +static int device_osd_init(struct scsi_lu *lu)
>  {
>  	lu->d_sense = 1;
> +	return 0;
> +}
> +
> +static int device_osd_shutdown(struct scsi_lu *lu)
> +{
> +	return 0;
>  }
>  
>  struct device_type_template osd_template = {
>  	.type		= TYPE_OSD,
>  	.name		= "osd",
>  	.device_init	= device_osd_init,
> +	.device_shutdown	= device_osd_shutdown,
>  	.ops		= {
>  		[0x00 ... 0x0f] = {spc_illegal_op},
>  
> diff --git a/usr/sbc.c b/usr/sbc.c
> index a22d3b0..879943a 100644
> --- a/usr/sbc.c
> +++ b/usr/sbc.c
> @@ -337,10 +337,21 @@ sense:
>  	return SAM_STAT_CHECK_CONDITION;
>  }
>  
> +static int sbc_init(struct scsi_lu *lu) {
> +	return 0;
> +}
> +
> +static int sbc_shutdown(struct scsi_lu *lu) {
> +	return 0;
> +}
> +
>  struct device_type_template sbc_template = {
>  	.type	= TYPE_DISK,
>  	.name	= "disk",
>  	.pid	= "VIRTUAL-DISK",
> +	.device_init = sbc_init,
> +	.device_shutdown = sbc_shutdown,
> +	.device_config = spc_device_config,
>  	.ops	= {
>  		{spc_test_unit,},
>  		{spc_illegal_op,},
> diff --git a/usr/sense_codes.h b/usr/sense_codes.h
> new file mode 100644
> index 0000000..0791dc3
> --- /dev/null
> +++ b/usr/sense_codes.h
> @@ -0,0 +1,105 @@
> +/*
> + * The SCSI sense key Additional Sense Code / Additional Sense Code Qualifier
> + *
> + * Copyright (C) 2007 Mark Harvey markh794 at gmail dot com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#define mk_sense_data(cmd, key, qual) \
> +	sense_data_build(cmd, key, ((qual >> 8) & 0xff), qual && 0xff)
> +
> +/*
> + * SENSE keys
> + */
> +#define NO_SENSE		0x00
> +#define	RECOVERED_ERROR		0x01
> +#define NOT_READY		0x02
> +#define MEDIUM_ERROR		0x03
> +#define	HARDWARE_ERROR		0x04
> +#define ILLEGAL_REQUEST		0x05
> +#define UNIT_ATTENTION		0x06
> +#define DATA_PROTECT		0x07
> +#define	BLANK_CHECK		0x08
> +
> +/* No Sense Errors */
> +#define NO_ADDITIONAL_SENSE		0x0000
> +#define E_MARK				0x0001
> +#define E_EOM				0x0002
> +#define E_BOM				0x0004
> +#define E_END_OF_DATA			0x0005
> +#define E_OP_IN_PROGRESS		0x0016
> +#define E_DRIVE_REQUIRES_CLEANING	0x8282
> +
> +/* Recovered Errors */
> +#define E_WRITE_ERROR			0x0c00
> +#define E_READ_ERROR			0x1100
> +#define E_RECOVERED_WITH_RETRYS		0x1701
> +#define E_MEDIA_LOAD_EJECT_ERROR	0x5300
> +#define E_FAILURE_PREDICTION		0x5d00
> +
> +/* Not ready */
> +#define E_CAUSE_NOT_REPORTABLE		0x0400
> +#define E_BECOMING_READY		0x0401
> +#define E_INITIALIZING_REQUIRED		0x0402
> +#define E_CLEANING_CART_INSTALLED	0x3003
> +#define E_CLEANING_FAILURE		0x3007
> +#define E_MEDIUM_NOT_PRESENT		0x3a00
> +#define E_LOGICAL_UNIT_NOT_CONFIG	0x3e00
> +
> +/* Medium Errors */
> +#define E_WRITE_ERROR			0x0c00
> +#define E_UNRECOVERED_READ		0x1100
> +#define E_RECORDED_ENTITY_NOT_FOUND	0x1400
> +#define E_UNKNOWN_FORMAT		0x3001
> +#define E_IMCOMPATIBLE_FORMAT		0x3002
> +#define E_MEDIUM_FORMAT_CORRUPT		0x3100
> +#define E_SEQUENTIAL_POSITION_ERR	0x3b00
> +#define E_WRITE_APPEND_ERR		0x5000
> +#define E_CARTRIDGE_FAULT		0x5200
> +#define E_MEDIA_LOAD_OR_EJECT_FAILED	0x5300
> +
> +/* Hardware Failure */
> +#define E_COMPRESSION_CHECK		0x0c04
> +#define E_DECOMPRESSION_CRC		0x110d
> +#define E_MECHANICAL_POSITIONING_ERROR	0x1501
> +#define E_MANUAL_INTERVENTION_REQ	0x0403
> +#define E_HARDWARE_FAILURE		0x4000
> +#define E_INTERNAL_TARGET_FAILURE	0x4400
> +#define E_ERASE_FAILURE			0x5100
> +
> +/* Illegal Request */
> +#define E_PARAMETER_LIST_LENGTH_ERR	0x1a00
> +#define E_INVALID_OP_CODE		0x2000
> +#define E_INVALID_FIELD_IN_CDB		0x2400
> +#define E_LUN_NOT_SUPPORTED		0x2500
> +#define E_INVALID_FIELD_IN_PARMS	0x2600
> +#define E_SAVING_PARMS_UNSUP		0x3900
> +#define E_MEDIUM_DEST_FULL		0x3b0d
> +#define E_MEDIUM_SRC_EMPTY		0x3b0e
> +#define E_POSITION_PAST_BOM		0x3b0c
> +#define E_MEDIUM_REMOVAL_PREVENTED	0x5302
> +#define E_BAD_MICROCODE_DETECTED	0x8283
> +
> +/* Unit Attention */
> +#define E_NOT_READY_TO_TRANSITION	0x2800
> +#define E_POWERON_RESET			0x2900
> +#define E_MODE_PARAMETERS_CHANGED	0x2a01
> +#define E_MICROCODE_DOWNLOADED		0x3f01
> +#define E_FAILURE_PREDICTION_FALSE	0x5dff
> +#define E_INQUIRY_DATA_HAS_CHANGED	0x3f03
> +
> +/* Data Protect */
> +#define E_WRITE_PROTECT			0x2700
> +#define E_MEDIUM_OVERWRITE_ATTEMPTED	0x300c
> diff --git a/usr/spc.c b/usr/spc.c
> index b922a45..7dd920a 100644
> --- a/usr/spc.c
> +++ b/usr/spc.c
> @@ -26,11 +26,14 @@
>  
>  #include "list.h"
>  #include "util.h"
> +#include "parser.h"
>  #include "tgtd.h"
>  #include "target.h"
> +#include "tgtadm.h"
>  #include "driver.h"
>  #include "scsi.h"
>  #include "spc.h"
> +#include "sense_codes.h"
>  
>  #define PRODUCT_REV	"0"
>  
> @@ -240,7 +243,74 @@ int spc_request_sense(int host_no, struct scsi_cmd *cmd)
>  
>  int spc_illegal_op(int host_no, struct scsi_cmd *cmd)
>  {
> +	uint8_t	*cdb = cmd->scb;
> +
> +	switch((cdb[0] & 0xe0) >> 5) {

how about using cmd->scb_len?


> +	case 0:	/* 6byte commands */
> +		dprintf("SCSI CMD: %02x %02x %02x %02x %02d %02x",
> +			cdb[0], cdb[1], cdb[2], cdb[3], cdb[4], cdb[5]);
> +		break;
> +	case 1: /* 10byte commands */
> +	case 2:
> +		dprintf("SCSI CMD: %02x %02x %02x %02x %02d %02x"
> +				" %02x %02x %02x %02x", 
> +			cdb[0], cdb[1], cdb[2], cdb[3], cdb[4], cdb[5],
> +			cdb[6], cdb[7], cdb[8], cdb[9]);
> +		break;
> +	case 3:	/* Exception to the rule */
> +	case 4:	/* 16byte commands */
> +		dprintf("SCSI CMD: %02x %02x %02x %02x %02d %02x"
> +				" %02x %02x %02x %02x %02x %02x" 
> +				" %02x %02x %02x %02x",
> +			cdb[0], cdb[1], cdb[2], cdb[3], cdb[4], cdb[5],
> +			cdb[6], cdb[7], cdb[8], cdb[9], cdb[10], cdb[11],
> +			cdb[12], cdb[13], cdb[14], cdb[15]);
> +		break;
> +	case 5:	/* 12byte commands */
> +		dprintf("SCSI CMD: %02x %02x %02x %02x %02d %02x"
> +				" %02x %02x %02x %02x %02x %02x" ,
> +			cdb[0], cdb[1], cdb[2], cdb[3], cdb[4], cdb[5],
> +			cdb[6], cdb[7], cdb[8], cdb[9], cdb[10], cdb[11]);
> +		break;
> +	}
>  	cmd->len = 0;
>  	sense_data_build(cmd, ILLEGAL_REQUEST, 0x20, 0);
>  	return SAM_STAT_CHECK_CONDITION;
>  }
> +
> +enum {
> +	Opt_scsiid, Opt_scsisn, Opt_err,
> +};
> +
> +static match_table_t tokens = {
> +	{Opt_scsiid, "scsi_id=%s"},
> +	{Opt_scsisn, "scsi_sn=%s"},
> +	{Opt_err, NULL},
> +};
> +
> +int spc_device_config(struct scsi_lu *lu, char * params) {
> +	int	err = 0;
> +	char	*p;
> +
> +	while ((p = strsep(&params, ",")) != NULL) {
> +		substring_t args[MAX_OPT_ARGS];
> +		int token;
> +		if (!*p)
> +			continue;
> +		dprintf("*p : %s\n", p);
> +		token = match_token(p, tokens, args);
> +		switch (token) {
> +		case Opt_scsiid:
> +			match_strncpy(lu->scsi_id, &args[0],
> +				      sizeof(lu->scsi_id) - 1);
> +			break;
> +		case Opt_scsisn:
> +			match_strncpy(lu->scsi_sn, &args[0],
> +				      sizeof(lu->scsi_sn) - 1);
> +			break;
> +		default:
> +			err = TGTADM_INVALID_REQUEST;
> +		}
> +	}
> +	return err;
> +}
> diff --git a/usr/spc.h b/usr/spc.h
> index f81e74c..5ced810 100644
> --- a/usr/spc.h
> +++ b/usr/spc.h
> @@ -7,5 +7,8 @@ extern int spc_start_stop(int host_no, struct scsi_cmd *cmd);
>  extern int spc_test_unit(int host_no, struct scsi_cmd *cmd);
>  extern int spc_request_sense(int host_no, struct scsi_cmd *cmd);
>  extern int spc_illegal_op(int host_no, struct scsi_cmd *cmd);
> +extern int spc_mode_sense(int host_no, struct scsi_cmd *cmd,
> +						struct list_head * head);
> +extern int spc_device_config(struct scsi_lu *lu, char * params);
>  
>  #endif
> diff --git a/usr/target.c b/usr/target.c
> index 23f450f..d3be173 100644
> --- a/usr/target.c
> +++ b/usr/target.c
> @@ -29,7 +29,6 @@
>  #include <sys/socket.h>
>  
>  #include "list.h"
> -#include "parser.h"
>  #include "util.h"
>  #include "tgtd.h"
>  #include "driver.h"
> @@ -249,11 +248,13 @@ int tgt_device_create(int tid, uint64_t lun, char *args)
>  
>  	snprintf(lu->scsi_id, sizeof(lu->scsi_id),
>  		 "deadbeaf%d:%" PRIu64, tid, lun);
> +	snprintf(lu->scsi_sn, sizeof(lu->scsi_sn),
> +		"beaf%d%" PRIu64, tid, lun);
>  
>  	tgt_cmd_queue_init(&lu->cmd_queue);
>  
>  	if (target->dev_type_template.device_init)
> -		target->dev_type_template.device_init(lu);
> +		err = target->dev_type_template.device_init(lu);
>  
>  	list_for_each_entry(pos, &target->device_list, device_siblings) {
>  		if (lu->lun < pos->lun)
> @@ -269,6 +270,7 @@ int tgt_device_destroy(int tid, uint64_t lun)
>  {
>  	struct target *target;
>  	struct scsi_lu *lu;
> +	int	err;
>  
>  	dprintf("%u %" PRIu64 "\n", tid, lun);
>  
> @@ -281,12 +283,14 @@ int tgt_device_destroy(int tid, uint64_t lun)
>  	if (!list_empty(&lu->cmd_queue.queue) || lu->cmd_queue.active_cmd)
>  		return TGTADM_LUN_ACTIVE;
>  
> +	err = target->dev_type_template.device_shutdown(lu);
> +
>  	free(lu->path);
>  	list_del(&lu->device_siblings);
>  
>  	target->bst->bs_close(lu);
>  	free(lu);
> -	return 0;
> +	return err;
>  }
>  
>  int device_reserve(struct scsi_cmd *cmd)
> @@ -339,20 +343,9 @@ int device_reserved(struct scsi_cmd *cmd)
>  	return -EBUSY;
>  }
>  
> -enum {
> -	Opt_scsiid, Opt_scsisn, Opt_err,
> -};
> -
> -static match_table_t tokens = {
> -	{Opt_scsiid, "scsi_id=%s"},
> -	{Opt_scsisn, "scsi_sn=%s"},
> -	{Opt_err, NULL},
> -};
> -
>  int tgt_device_update(int tid, uint64_t dev_id, char *params)
>  {
> -	int err = 0;
> -	char *p;
> +	int err = TGTADM_INVALID_REQUEST;
>  	struct target *target;
>  	struct scsi_lu *lu;
>  
> @@ -366,26 +359,8 @@ int tgt_device_update(int tid, uint64_t dev_id, char *params)
>  		return TGTADM_NO_LUN;
>  	}
>  
> -	while ((p = strsep(&params, ",")) != NULL) {
> -		substring_t args[MAX_OPT_ARGS];
> -		int token;
> -		if (!*p)
> -			continue;
> -		token = match_token(p, tokens, args);
> -
> -		switch (token) {
> -		case Opt_scsiid:
> -			match_strncpy(lu->scsi_id, &args[0],
> -				      sizeof(lu->scsi_id) - 1);
> -			break;
> -		case Opt_scsisn:
> -			match_strncpy(lu->scsi_sn, &args[0],
> -				      sizeof(lu->scsi_sn) - 1);
> -			break;
> -		default:
> -			err = TGTADM_INVALID_REQUEST;
> -		}
> -	}
> +	if (target->dev_type_template.device_config)
> +		err = target->dev_type_template.device_config(lu, params);
>  
>  	return err;
>  }
> @@ -474,8 +449,8 @@ int target_cmd_queue(int tid, struct scsi_cmd *cmd)
>  		cmd_post_perform(q, cmd);
>  
>  		dprintf("%" PRIx64 " %x %" PRIx64 " %" PRIu64 " %u %d %d\n",
> -			cmd->tag, cmd->scb[0], cmd->uaddr, cmd->offset, cmd->len,
> -			result, cmd->async);
> +			cmd->tag, cmd->scb[0], cmd->uaddr, cmd->offset,
> +			cmd->len, result, cmd->async);
>  
>  		set_cmd_processed(cmd);
>  		if (!cmd->async)
> @@ -483,7 +458,8 @@ int target_cmd_queue(int tid, struct scsi_cmd *cmd)
>  	} else {
>  		set_cmd_queued(cmd);
>  		dprintf("blocked %" PRIx64 " %x %" PRIu64 " %d\n",
> -			cmd->tag, cmd->scb[0], cmd->dev ? cmd->dev->lun : UINT64_MAX,
> +			cmd->tag, cmd->scb[0],
> +			cmd->dev ? cmd->dev->lun : UINT64_MAX,
>  			q->active_cmd);
>  
>  		list_add_tail(&cmd->qlist, &q->queue);
> diff --git a/usr/tgtadm.c b/usr/tgtadm.c
> index e55a785..3d551cc 100644
> --- a/usr/tgtadm.c
> +++ b/usr/tgtadm.c
> @@ -103,6 +103,7 @@ struct option const long_options[] = {
>  	{"initiator-address", required_argument, NULL, 'I'},
>  	{"user", required_argument, NULL, 'u'},
>  	{"password", required_argument, NULL, 'p'},
> +	{"params", required_argument, NULL, 'P'},
>  
>  	{"bus", required_argument, NULL, 'B'},
>  	{"target-type", required_argument, NULL, 'Y'},
> @@ -110,7 +111,7 @@ struct option const long_options[] = {
>  	{NULL, 0, NULL, 0},
>  };
>  
> -static char *short_options = "dhL:o:m:t:s:c:l:n:v:b:T:I:u:p:B:Y:O";
> +static char *short_options = "dhL:o:m:t:s:c:l:n:v:b:T:I:u:p:P:B:Y:O";
>  
>  static void usage(int status)
>  {
> @@ -355,7 +356,7 @@ int main(int argc, char **argv)
>  	int op, total, tid, rest, mode, t_type, ac_dir;
>  	uint32_t cid, hostno;
>  	uint64_t sid, lun;
> -	char *name, *value, *path, *targetname, *params, *address;
> +	char *name, *value, *path, *targetname, *params, *address, *targetOps;
>  	char *user, *password;
>  	char buf[BUFSIZE + sizeof(struct tgtadm_req)];
>  	struct tgtadm_req *req;
> @@ -365,7 +366,7 @@ int main(int argc, char **argv)
>  	t_type = TYPE_DISK;
>  	ac_dir = ACCOUNT_TYPE_INCOMING;
>  	rest = BUFSIZE;
> -	name = value = path = targetname = address = NULL;
> +	name = value = path = targetname = address = targetOps = NULL;
>  	user = password = NULL;
>  
>  	memset(buf, 0, sizeof(buf));
> @@ -396,6 +397,9 @@ int main(int argc, char **argv)
>  		case 'l':
>  			lun = strtoull(optarg, NULL, 10);
>  			break;
> +		case 'P':
> +			targetOps = optarg;
> +			break;
>  		case 'n':
>  			name = optarg;
>  			break;
> @@ -570,6 +574,10 @@ int main(int argc, char **argv)
>  	if (password)
>  		shprintf(total, params, rest, "%spassword=%s",
>  			 rest == BUFSIZE ? "" : ",", password);
> +	// Trailing ',' makes parsing params in modules easier..
> +	if (targetOps)
> +		shprintf(total, params, rest, "%stargetOps %s,",
> +			 rest == BUFSIZE ? "" : ",", targetOps);
>  
>  	req->len = sizeof(*req) + total;
>  
> diff --git a/usr/tgtd.h b/usr/tgtd.h
> index 06e0dd9..808fd7f 100644
> --- a/usr/tgtd.h
> +++ b/usr/tgtd.h
> @@ -49,6 +49,8 @@ struct scsi_lu {
>  
>  	/* TODO: needs a structure for lots of device parameters */
>  	uint8_t d_sense;
> +
> +	void * priv_p;
>  };
>  
>  struct scsi_cmd {
> @@ -121,7 +123,9 @@ struct device_type_template {
>  	char *name;
>  	char *pid;
>  
> -	void (*device_init)(struct scsi_lu *dev);
> +	int (*device_init)(struct scsi_lu *lu);
> +	int (*device_shutdown)(struct scsi_lu *lu);
> +	int (*device_config)(struct scsi_lu *lu, char *arg);
>  
>  	struct device_type_operations ops[256];
>  };



More information about the stgt mailing list