[Stgt-devel] [Patch 0/2] Re-format / resubmit patch - try 3.

FUJITA Tomonori fujita.tomonori
Sat Jun 2 03:53:03 CEST 2007


Thanks a lot, mostly looks ok. some minor things...

From: "Mark Harvey" <markh794 at gmail.com>
Subject: [Stgt-devel] [Patch 0/2] Re-format / resubmit patch - try 3.
Date: Fri, 1 Jun 2007 10:28:28 +1000

> From 04040395672afd987d3d439bd5657c57db1cf2f9 Mon Sep 17 00:00:00 2001
> From: Mark Harvey <markh794 at gmail.com>
> Date: Fri, 1 Jun 2007 06:00:43 +1000
> Subject: [PATCH] Add new module entry points for configuration
> 
> Add new module entry points.
>   <module>_lu_init(struct scsi_lu *lu) - called at setup time.
>   <module>_lu_exit(struct scsi_lu *lu) - called at shutdown time.
>   <module>_lu_config(struct scsi_lu *lu) - Any module unique configuration.
> 
> Moved logical unit configuration from target -> <module>
>   Along with above entry point <module>_lu_config(), will allow module
>   specific configuration.
>   No new functionality added yet.
> 
> Add a dump_cdb(struct scsi_cmd *cmd) - Called from spc_illegal_op()
> 
> Update - All modules use common INQUIRY routine.
> 
> Signed-off-by: Mark Harvey <markh794 at gmail.com>
> 
> diff --git a/usr/mmc.c b/usr/mmc.c
> index e9cc479..ae0a0ca 100644
> --- a/usr/mmc.c
> +++ b/usr/mmc.c
> @@ -121,11 +121,32 @@ static int mmc_read_capacity(int host_no, struct scsi_cmd *cmd)
>  	return SAM_STAT_GOOD;
>  }
>  
> +static int mmc_lu_init(struct scsi_lu *lu)
> +{
> +	if (spc_lu_init(lu))
> +		return -ENOMEM;
> +
> +	memcpy(lu->attributes->ProductIdent, "VIRTUAL-CDROM", 16);
> +	lu->attributes->sense_format = 1;
> +	lu->attributes->version_desc[0] = 0x02A0; /* MMC3, no version claimed */
> +	lu->attributes->version_desc[1] = 0x0960; /* iSCSI */
> +	lu->attributes->version_desc[2] = 0x0300; /* SPC-3 */
> +
> +	return 0;
> +}
> +
> +static int mmc_lu_exit(struct scsi_lu *lu)
> +{
> +	return 0;
> +}
> +
>  struct device_type_template mmc_template = {
> -	.type	= TYPE_ROM,
> -	.name	= "cdrom/dvd",
> -	.pid	= "VIRTUAL-CDROM",
> -	.ops	= {
> +	.type		= TYPE_ROM,
> +	.name		= "cdrom/dvd",
> +	.lu_init	= mmc_lu_init,
> +	.lu_exit	= mmc_lu_exit,
> +	.lu_config	= spc_lu_config,
> +	.ops		= {
>  		{spc_test_unit,},
>  		{spc_illegal_op,},
>  		{spc_illegal_op,},
> diff --git a/usr/osd.c b/usr/osd.c
> index 46bf0a0..0ce8fde 100644
> --- a/usr/osd.c
> +++ b/usr/osd.c
> @@ -20,6 +20,7 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>   * 02110-1301 USA
>   */
> +#include <errno.h>
>  #include <inttypes.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -33,101 +34,6 @@
>  #include "scsi.h"
>  #include "spc.h"
>  
> -#define PRODUCT_ID	"OSD"
> -#define PRODUCT_REV	"0"
> -
> -static int osd_inquiry(int host_no, struct scsi_cmd *cmd)
> -{
> -	uint8_t *data, *scb = cmd->scb;
> -	int len, ret = SAM_STAT_CHECK_CONDITION;
> -	unsigned char key = ILLEGAL_REQUEST, asc = 0x25;
> -
> -	/* EVPD means need a page code */
> -	if ((scb[1] & 0x3) == 0 && scb[2] != 0)
> -		goto sense;
> -
> -	data = valloc(pagesize);
> -	if (!data) {
> -		key = HARDWARE_ERROR;
> -		asc = 0;
> -		goto sense;
> -	}
> -	memset(data, 0, pagesize);
> -
> -	dprintf("%x %x\n", scb[1], scb[2]);
> -
> -	data[0] = TYPE_OSD;
> -	if (!cmd->dev)
> -		data[0] = TYPE_NO_LUN;
> -
> -	if ((scb[1] & 0x1) == 0) {
> -		data[2] = 5;  /* modern version */
> -		data[3] = 0x02;  /* modern response format */
> -		data[7] = 0x02;  /* support command queueing */
> -		memset(data + 8, 0x20, 28);
> -		memcpy(data + 8,
> -		       VENDOR_ID, min_t(size_t, strlen(VENDOR_ID), 8));
> -		memcpy(data + 16,
> -		       PRODUCT_ID, min_t(size_t, strlen(PRODUCT_ID), 16));
> -		memcpy(data + 32,
> -		       PRODUCT_REV, min_t(size_t, strlen(PRODUCT_REV), 4));
> -		len = 36;
> -		if (cmd->dev) {
> -			data[58] = 0x03;
> -			data[59] = 0x40;  /* osd */
> -			data[60] = 0x09;
> -			data[61] = 0x60;  /* iscsi */
> -			data[62] = 0x03;
> -			data[63] = 0x00;  /* spc3 */
> -			len = 64;
> -		}
> -		data[4] = len - 5;  /* additional length */
> -		ret = SAM_STAT_GOOD;
> -	} else {
> -		if (!cmd->dev)
> -			goto sense;
> -
> -		data[1] = scb[2];
> -		if (scb[2] == 0x0) {
> -			/* supported VPD pages */
> -			data[3] = 3;
> -			data[4] = 0x0;
> -			data[5] = 0x80;
> -			data[6] = 0x83;
> -			len = 7;
> -			ret = SAM_STAT_GOOD;
> -		} else if (scb[2] == 0x80) {
> -			/* unit serial number "    " */
> -			data[3] = 4;
> -			memset(data + 4, 0x20, 4);
> -			len = 8;
> -			ret = SAM_STAT_GOOD;
> -		} else if (scb[2] == 0x83) {
> -			/* device identification */
> -			data[3] = SCSI_ID_LEN + 4;
> -			data[4] = 0x1;
> -			data[5] = 0x1;
> -			data[7] = SCSI_ID_LEN;
> -			if (cmd->dev)
> -				memcpy(data + 8, cmd->dev->scsi_id, SCSI_ID_LEN);
> -			len = SCSI_ID_LEN + 8;
> -			ret = SAM_STAT_GOOD;
> -		}
> -	}
> -
> -	if (ret != SAM_STAT_GOOD)
> -		goto sense;
> -
> -	cmd->len = min_t(int, len, scb[4]);
> -	cmd->uaddr = (unsigned long) data;
> -
> -	return SAM_STAT_GOOD;
> -sense:
> -	sense_data_build(cmd, key, asc, 0);
> -	cmd->len = 0;
> -	return SAM_STAT_CHECK_CONDITION;
> -}
> -
>  static int osd_varlen_cdb(int host_no, struct scsi_cmd *cmd)
>  {
>  	unsigned char key = ILLEGAL_REQUEST, asc = 0x25;
> @@ -145,22 +51,37 @@ 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 osd_lu_init(struct scsi_lu *lu)
> +{
> +	if (spc_lu_init(lu))
> +		return -ENOMEM;
> +
> +	memcpy(lu->attributes->ProductIdent, "OSD", 16);
> +	lu->attributes->sense_format = 1;
> +	lu->attributes->version_desc[0] = 0x0340; /* OSD */
> +	lu->attributes->version_desc[1] = 0x0960; /* iSCSI */
> +	lu->attributes->version_desc[2] = 0x0300; /* SPC-3 */
> +
> +	return 0;
> +}
> +
> +static int osd_lu_exit(struct scsi_lu *lu)
>  {
> -	lu->d_sense = 1;
> +	return 0;
>  }
>  
>  struct device_type_template osd_template = {
>  	.type		= TYPE_OSD,
>  	.name		= "osd",
> -	.device_init	= device_osd_init,
> +	.lu_init	= osd_lu_init,
> +	.lu_exit	= osd_lu_exit,
>  	.ops		= {
>  		[0x00 ... 0x0f] = {spc_illegal_op},
>  
>  		/* 0x10 */
>  		{spc_illegal_op,},
>  		{spc_illegal_op,},
> -		{osd_inquiry,},
> +		{spc_inquiry,},
>  		{spc_illegal_op,},
>  		{spc_illegal_op,},
>  		{spc_illegal_op,},
> diff --git a/usr/parser.h b/usr/parser.h
> index 1ff6016..bf210bf 100644
> --- a/usr/parser.h
> +++ b/usr/parser.h
> @@ -17,7 +17,7 @@ struct match_token {
>  typedef struct match_token match_table_t[];
>  
>  /* Maximum number of arguments that match_token will find in a pattern */
> -enum {MAX_OPT_ARGS = 3};
> +enum {MAX_OPT_ARGS = 17};

For what?


>  /* Describe the location within a string of a substring */
>  typedef struct {
> diff --git a/usr/sbc.c b/usr/sbc.c
> index a22d3b0..5357087 100644
> --- a/usr/sbc.c
> +++ b/usr/sbc.c
> @@ -337,11 +337,31 @@ sense:
>  	return SAM_STAT_CHECK_CONDITION;
>  }
>  
> +static int sbc_lu_init(struct scsi_lu *lu)
> +{
> +	if (spc_lu_init(lu))
> +		return -ENOMEM;
> +
> +	memcpy(lu->attributes->ProductIdent, "VIRTUAL-DISK", 16);
> +	lu->attributes->version_desc[0] = 0x04C0; /* SBC-3 no version claimed */
> +	lu->attributes->version_desc[1] = 0x0960; /* iSCSI */
> +	lu->attributes->version_desc[2] = 0x0300; /* SPC-3 */
> +
> +	return 0;
> +}
> +
> +static int sbc_lu_exit(struct scsi_lu *lu)
> +{
> +	return 0;
> +}
> +
>  struct device_type_template sbc_template = {
> -	.type	= TYPE_DISK,
> -	.name	= "disk",
> -	.pid	= "VIRTUAL-DISK",
> -	.ops	= {
> +	.type		= TYPE_DISK,
> +	.name		= "disk",
> +	.lu_init	= sbc_lu_init,
> +	.lu_exit	= sbc_lu_exit,
> +	.lu_config	= spc_lu_config,
> +	.ops		= {
>  		{spc_test_unit,},
>  		{spc_illegal_op,},
>  		{spc_illegal_op,},
> diff --git a/usr/scsi.c b/usr/scsi.c
> index 1a6929f..728dfe9 100644
> --- a/usr/scsi.c
> +++ b/usr/scsi.c
> @@ -40,7 +40,7 @@
>  
>  void sense_data_build(struct scsi_cmd *cmd, uint8_t key, uint8_t asc, uint8_t asq)
>  {
> -	if (cmd->dev && cmd->dev->d_sense) {
> +	if (cmd->dev && cmd->dev->attributes->sense_format) {
>  		/* descriptor format */
>  
>  		cmd->sense_buffer[0] = 0x72;  /* current, not deferred */
> @@ -121,3 +121,4 @@ int scsi_cmd_perform(int host_no, struct scsi_cmd *cmd)
>  	unsigned char op = cmd->scb[0];
>  	return cmd->c_target->dev_type_template.ops[op].cmd_perform(host_no, cmd);
>  }
> +
> diff --git a/usr/spc.c b/usr/spc.c
> index b922a45..e9a74d9 100644
> --- a/usr/spc.c
> +++ b/usr/spc.c
> @@ -19,6 +19,7 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>   * 02110-1301 USA
>   */
> +#include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -27,12 +28,15 @@
>  #include "list.h"
>  #include "util.h"
>  #include "tgtd.h"
> +#include "parser.h"
>  #include "target.h"
>  #include "driver.h"
> +#include "tgtadm_error.h"
>  #include "scsi.h"
>  #include "spc.h"
>  
>  #define PRODUCT_REV	"0"
> +#define BLK_SHIFT	9
>  
>  int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>  {
> @@ -40,7 +44,7 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>  	uint8_t *data;
>  	uint8_t *scb = cmd->scb;
>  	unsigned char device_type = cmd->c_target->dev_type_template.type;
> -	char *product_id = cmd->c_target->dev_type_template.pid;
> +	struct physicalAttributes * attributes = cmd->dev->attributes;
>  	unsigned char key = ILLEGAL_REQUEST, asc = 0x24;
>  
>  	if (((scb[1] & 0x3) == 0x3) || (!(scb[1] & 0x3) && scb[2]))
> @@ -57,22 +61,21 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>  	dprintf("%x %x\n", scb[1], scb[2]);
>  
>  	if (!(scb[1] & 0x3)) {
> +		int i;
>  		data[0] = device_type;
> -		data[2] = 4;
> +		data[1] = (attributes->isRemovable) ? 0x80 : 0;
> +		data[2] = 5;	/* SPC-3 */
>  		data[3] = 0x42;
> -		data[4] = 59;
>  		data[7] = 0x02;
>  		memset(data + 8, 0x20, 28);
> -		strncpy((char *)data + 8, VENDOR_ID, 8);
> -		strncpy((char *)data + 16, product_id, 16);
> -		strncpy((char *)data + 32, PRODUCT_REV, 4);
> -		data[58] = 0x03;
> -		data[59] = 0x20;
> -		data[60] = 0x09;
> -		data[61] = 0x60;
> -		data[62] = 0x03;
> -		data[63] = 0x00;
> -		len = 64;
> +		strncpy((char *)data + 8, attributes->VendorIdent, 8);
> +		strncpy((char *)data + 16, attributes->ProductIdent, 16);
> +		strncpy((char *)data + 32, attributes->ProductRev, 4);
> +		for (i=0, len=58; i < VERSION_DESCRIPTOR_LEN; i++, len += 2) {
> +			data[len] = (attributes->version_desc[i] >> 8) & 0xff;
> +			data[len+1] = attributes->version_desc[i] & 0xff;
> +		}
> +		data[4] = len - 5;	/* Additional Length */
>  		ret = SAM_STAT_GOOD;
>  	} else if (scb[1] & 0x2) {
>  		/* CmdDt bit is set */
> @@ -101,13 +104,13 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>  			len = 4 + SCSI_SN_LEN;
>  			ret = SAM_STAT_GOOD;
>  
> -			if (cmd->dev && strlen(cmd->dev->scsi_sn)) {
> +			if (cmd->dev && strlen(cmd->dev->attributes->scsi_sn)) {
>  				uint8_t *p;
>  				char *q;
>  
>  				p = data + 4 + tmp - 1;
> -				q = cmd->dev->scsi_sn + SCSI_SN_LEN - 1;
> -
> +				q = cmd->dev->attributes->scsi_sn +
> +								SCSI_SN_LEN - 1;
>  				for (; tmp > 0; tmp--, q)
>  					*(p--) = *(q--);
>  			}
> @@ -120,7 +123,8 @@ int spc_inquiry(int host_no, struct scsi_cmd *cmd)
>  			data[5] = 0x1;
>  			data[7] = tmp;
>  			if (cmd->dev)
> -				strncpy((char *) data + 8, cmd->dev->scsi_id,
> +				strncpy((char *) data + 8,
> +					cmd->dev->attributes->scsi_id,
>  				        SCSI_ID_LEN);
>  			len = tmp + 8;
>  			ret = SAM_STAT_GOOD;
> @@ -238,9 +242,98 @@ int spc_request_sense(int host_no, struct scsi_cmd *cmd)
>  	return SAM_STAT_GOOD;
>  }
>  
> +void dump_cdb(struct scsi_cmd *cmd)
> +{
> +	uint8_t * cdb = cmd->scb;
> +
> +	switch(cmd->scb_len) {
> +	case 6:
> +		dprintf("SCSI CMD: %02x %02x %02x %02x %02d %02x",
> +			cdb[0], cdb[1], cdb[2], cdb[3], cdb[4], cdb[5]);
> +		break;
> +	case 10:
> +		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 12:
> +		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;
> +	case 16:
> +		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;
> +	}
> +}
> +
>  int spc_illegal_op(int host_no, struct scsi_cmd *cmd)
>  {
> +	dump_cdb(cmd);
>  	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_lu_config(struct scsi_lu *lu, char * params) {
> +	int err = 0;
> +	char *p;
> +
> +	if (!strncmp("targetOps", params, 9))
> +		params = params + 10;

Hacky, but the interface between tgtadm and tgtd is hacky. So it's ok
now and I'll fix it later on.


> +	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->attributes->scsi_id, &args[0],
> +				      sizeof(lu->attributes->scsi_id) - 1);
> +			break;
> +		case Opt_scsisn:
> +			match_strncpy(lu->attributes->scsi_sn, &args[0],
> +				      sizeof(lu->attributes->scsi_sn) - 1);
> +			break;
> +		default:
> +			err = TGTADM_INVALID_REQUEST;
> +		}
> +	}
> +	return err;
> +}
> +
> +int spc_lu_init(struct scsi_lu * lu)
> +{
> +	lu->attributes = zalloc(sizeof(struct physicalAttributes));
> +	if(!lu->attributes)
> +		return -ENOMEM;
> +
> +	memcpy(lu->attributes->VendorIdent, VENDOR_ID, 8);
> +	memcpy(lu->attributes->ProductRev, "0001", 4);
> +	lu->attributes->isRemovable = 0;
> +	lu->attributes->sense_format = 0;
> +	lu->attributes->onLine = 0;
> +	lu->attributes->reset = 1;
> +
> +	return 0;
> +}
> diff --git a/usr/spc.h b/usr/spc.h
> index f81e74c..1036b70 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_lu_init(struct scsi_lu *lu);
> +extern int spc_lu_config(struct scsi_lu *lu, char * params);
> +extern void dump_cdb(struct scsi_cmd *cmd);
>  
>  #endif
> diff --git a/usr/target.c b/usr/target.c
> index 23f450f..e3fa1e0 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"
> @@ -247,13 +246,19 @@ int tgt_device_create(int tid, uint64_t lun, char *args)
>  	lu->lun = lun;
>  	lu->lu_state = SCSI_LU_RUNNING;
>  
> -	snprintf(lu->scsi_id, sizeof(lu->scsi_id),
> -		 "deadbeaf%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);
> +	if (target->dev_type_template.lu_init)
> +		err = target->dev_type_template.lu_init(lu);
> +
> +	if(!err) {
> +		snprintf(lu->attributes->scsi_id,
> +			sizeof(lu->attributes->scsi_id),
> +			"deadbeaf%d:%" PRIu64, tid, lun);
> +		snprintf(lu->attributes->scsi_sn,
> +			sizeof(lu->attributes->scsi_sn),
> +			"beaf%d%" PRIu64, tid, lun);
> +	}
>  
>  	list_for_each_entry(pos, &target->device_list, device_siblings) {
>  		if (lu->lun < pos->lun)
> @@ -262,13 +267,14 @@ int tgt_device_create(int tid, uint64_t lun, char *args)
>  	list_add_tail(&lu->device_siblings, &pos->device_siblings);
>  
>  	dprintf("Add a logical unit %" PRIu64 " to the target %d\n", lun, tid);
> -	return 0;
> +	return err;
>  }
>  
>  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 +287,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.lu_exit(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 +347,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 +363,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.lu_config)
> +		err = target->dev_type_template.lu_config(lu, params);
>  
>  	return err;
>  }
> @@ -474,8 +453,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 +462,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);
> @@ -1148,8 +1128,8 @@ int tgt_target_show_all(char *buf, int rest)
>  				 _TAB3 "Size: %s\n"
>  				 _TAB3 "Backing store: %s\n",
>  				 lu->lun,
> -				 lu->scsi_id,
> -				 lu->scsi_sn,
> +				 lu->attributes->scsi_id,
> +				 lu->attributes->scsi_sn,
>  				 print_disksize(lu->size),
>  				 lu->path);
>  
> 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/tgtadm.h b/usr/tgtadm.h
> index fb83753..d53cb40 100644
> --- a/usr/tgtadm.h
> +++ b/usr/tgtadm.h
> @@ -4,31 +4,7 @@
>  #define TGT_IPC_NAMESPACE	"TGT_IPC_ABSTRACT_NAMESPACE"
>  #define TGT_LLD_NAME_LEN	64
>  
> -enum tgtadm_errno {
> -	TGTADM_SUCCESS,
> -	TGTADM_UNKNOWN_ERR,
> -	TGTADM_NOMEM,
> -	TGTADM_NO_DRIVER,
> -	TGTADM_NO_TARGET,
> -
> -	TGTADM_NO_LUN,
> -	TGTADM_NO_SESSION,
> -	TGTADM_NO_CONNECTION,
> -	TGTADM_TARGET_EXIST,
> -	TGTADM_LUN_EXIST,
> -
> -	TGTADM_ACL_EXIST,
> -	TGTADM_USER_EXIST,
> -	TGTADM_NO_USER,
> -	TGTADM_TOO_MANY_USER,
> -	TGTADM_INVALID_REQUEST,
> -
> -	TGTADM_OUTACCOUNT_EXIST,
> -	TGTADM_TARGET_ACTIVE,
> -	TGTADM_LUN_ACTIVE,
> -	TGTADM_UNSUPPORTED_OPERATION,
> -	TGTADM_UNKNOWN_PARAM,
> -};
> +#include "tgtadm_error.h"
>  
>  enum tgtadm_op {
>  	OP_NEW,
> diff --git a/usr/tgtadm_error.h b/usr/tgtadm_error.h
> new file mode 100644
> index 0000000..319a4ad
> --- /dev/null
> +++ b/usr/tgtadm_error.h
> @@ -0,0 +1,30 @@
> +#ifndef TGTADM_ERROR_H
> +#define TGTADM_ERROR_H
> +
> +enum tgtadm_errno {
> +	TGTADM_SUCCESS,
> +	TGTADM_UNKNOWN_ERR,
> +	TGTADM_NOMEM,
> +	TGTADM_NO_DRIVER,
> +	TGTADM_NO_TARGET,
> +
> +	TGTADM_NO_LUN,
> +	TGTADM_NO_SESSION,
> +	TGTADM_NO_CONNECTION,
> +	TGTADM_TARGET_EXIST,
> +	TGTADM_LUN_EXIST,
> +
> +	TGTADM_ACL_EXIST,
> +	TGTADM_USER_EXIST,
> +	TGTADM_NO_USER,
> +	TGTADM_TOO_MANY_USER,
> +	TGTADM_INVALID_REQUEST,
> +
> +	TGTADM_OUTACCOUNT_EXIST,
> +	TGTADM_TARGET_ACTIVE,
> +	TGTADM_LUN_ACTIVE,
> +	TGTADM_UNSUPPORTED_OPERATION,
> +	TGTADM_UNKNOWN_PARAM,
> +};
> +
> +#endif
> diff --git a/usr/tgtd.h b/usr/tgtd.h
> index 06e0dd9..8cda864 100644
> --- a/usr/tgtd.h
> +++ b/usr/tgtd.h
> @@ -5,6 +5,7 @@
>  
>  #define SCSI_ID_LEN	24
>  #define SCSI_SN_LEN	8
> +#define VERSION_DESCRIPTOR_LEN 8
>  
>  #define VENDOR_ID	"IET"
>  
> @@ -29,13 +30,27 @@ struct tgt_cmd_queue {
>  	struct list_head queue;
>  };
>  
> +struct physicalAttributes {

We don't use mixed-case names (chapter 4 in the CodingStyle).

I like something like struct lu_phy_attr though I'm not good at
naming. Suggestions?


> +	char	scsi_id[SCSI_ID_LEN];

char scsi_id[SCSI_ID_LEN];


> +	char	scsi_sn[SCSI_SN_LEN];
> +
> +	/* SCSI Inquiry Params */
> +	char	VendorIdent[9];

ditto.

char vendor_id[9];


> +	char	ProductIdent[17];
> +	char	ProductRev[5];
> +	uint16_t	version_desc[VERSION_DESCRIPTOR_LEN];
> +
> +	char	isRemovable;	/* Removable media */
> +	char	onLine;
> +	char	reset;		/* Power-on or reset */
> +	char	sense_format;	/* sense data format */
> +};

Ditto.


>  struct scsi_lu {
>  	int fd;
>  	uint64_t addr; /* persistent mapped address */
>  	uint64_t size;
>  	uint64_t lun;
> -	char scsi_id[SCSI_ID_LEN];
> -	char scsi_sn[SCSI_SN_LEN];
>  	char *path;
>  
>  	/* the list of devices belonging to a target */
> @@ -48,7 +63,7 @@ struct scsi_lu {
>  	uint64_t reserve_id;
>  
>  	/* TODO: needs a structure for lots of device parameters */
> -	uint8_t d_sense;
> +	struct physicalAttributes * attributes;

No space please after *.


>  };
>  
>  struct scsi_cmd {
> @@ -121,7 +136,9 @@ struct device_type_template {
>  	char *name;
>  	char *pid;
>  
> -	void (*device_init)(struct scsi_lu *dev);
> +	int (*lu_init)(struct scsi_lu *lu);
> +	int (*lu_exit)(struct scsi_lu *lu);
> +	int (*lu_config)(struct scsi_lu *lu, char *arg);
>  
>  	struct device_type_operations ops[256];
>  };
> -- 
> 1.5.1.3
> 



More information about the stgt mailing list