[stgt] [PATCH 1/1] tgtd: Patch to add bsoflags options o tgtd.

Daniel Henrique Debonzi debonzi at linux.vnet.ibm.com
Tue May 18 21:21:00 CEST 2010


FUJITA Tomonori wrote:
> On Fri, 14 May 2010 11:34:10 -0300
> Daniel Henrique Debonzi <debonzi at linux.vnet.ibm.com> wrote:
> 
>> Hi all,
>>
>> This patch makes possible to add sync and/or direct options to the backing store open flags.
>> The command format is like that:
>>
>> tgtadm --lld iscsi --mode logicalunit --op new --tid 1 --lun 1 --bsoflags="async direct" -b /home/debonzi/develop/iscsi-disk1
>> for creating and
> 
> Hmm, "async" option doesn't look supported in this patch. Note that we
> can't use "async" for bd_sync.c
> 
> If you want to use async and direct, then you can just specify
> bs_aio.c.

It was a typo. The right option is "sync".

> 
> I also think that we open a file with "sync", then we need to modify
> WCE bit too.
> 
> 
> What open options are you interested in using?

With this patch I want to control the O_SYNC and O_DIRECT flags on rdwr 
backing store. However I notice a problem on this patch after your 
comment. The problem is that if someone use another backing store that 
not rdrw it will not use the flags but "--op show" will show the 
bsoflags are used.

One idea to solve it and don't make necessary to touch all backing 
stores to avoid this issue is to create a new template like bs_rdrw_sync 
O_SYNC and O_DIRECT flags on open, and don't have the options as this 
patch does. This way I got want I am interested in and don't need to 
involve all the other stuff. What are your thoughts about it?

Thanks,
Daniel Debonzi
> 
> 
>> tgtadm --op update --mode logicalunit --tid 1 --lun 1 --params "bsoflags=sync direct"
>> for updating.
>>
>> It is the first time I am sending a patch to this list, 
>> so if I missed something please let me know.
>>
>> Regards,
>> Daniel Debonzi
>>
>>
>> Date: Thu, 29 Apr 2010 17:27:52 -0300
>> Subject: [PATCH 1/1] tgtd: Patch to add bsoflags options o tgtd.
>>
>> From: Daniel Henrique Debonzi <debonzi at linux.vnet.ibm.com>
>>
>> This patch make possible to set bsoflags using tgtadm to be passed to the backstore open
>> function. Supported bsoflags options are sync and direct.
>>
>> Signed-off-by: Daniel Henrique Debonzi <debonzi at linux.vnet.ibm.com>
>> ---
>>  usr/bs_rdwr.c |    2 +-
>>  usr/spc.c     |   17 ++++++++++++++++-
>>  usr/target.c  |   27 +++++++++++++++++++++------
>>  usr/tgtadm.c  |   23 +++++++++++++++++------
>>  usr/tgtd.h    |    1 +
>>  usr/util.c    |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>  usr/util.h    |    2 ++
>>  7 files changed, 103 insertions(+), 14 deletions(-)
>>
>> diff --git a/usr/tgtd.h b/usr/tgtd.h
>> index 3323a9b..1b22f9a 100644
>> --- a/usr/tgtd.h
>> +++ b/usr/tgtd.h
>> @@ -150,6 +150,7 @@ struct scsi_lu {
>>  	uint64_t size;
>>  	uint64_t lun;
>>  	char *path;
>> +	int bsoflags;
>>  
>>  	/* the list of devices belonging to a target */
>>  	struct list_head device_siblings;
>> diff --git a/usr/util.c b/usr/util.c
>> index e91453d..ab3ba8c 100644
>> diff --git a/usr/util.h b/usr/util.h
>> index 6e1fd6f..24a5eb7 100644
>> --- a/usr/util.h
>> +++ b/usr/util.h
>> @@ -57,6 +57,8 @@
>>  extern int chrdev_open(char *modname, char *devpath, uint8_t minor, int *fd);
>>  extern int backed_file_open(char *path, int oflag, uint64_t *size);
>>  extern int set_non_blocking(int fd);
>> +extern int str_to_open_flags(char *buf);
>> +extern char *open_flags_to_str(char *dest, int flags);
>>  
>>  #define zalloc(size)			\
>>  ({					\
>> --- a/usr/util.c
>> +++ b/usr/util.c
>> @@ -136,3 +136,48 @@ int set_non_blocking(int fd)
>>  	}
>>  	return err;
>>  }
>> +
>> +int str_to_open_flags(char *buf)
>> +{
>> +	char *bsoflags_tok = NULL;
>> +	int open_flags = 0;
>> +
>> +	bsoflags_tok = strtok(buf, " \0");
>> +	while (bsoflags_tok != NULL) {
>> +		while (*bsoflags_tok == ' ')
>> +			bsoflags_tok++;
>> +		if (!strncmp(bsoflags_tok, "sync", 4))
>> +			open_flags |= O_SYNC;
>> +		else if (!strncmp(bsoflags_tok, "direct", 6))
>> +			open_flags |= O_DIRECT;
>> +		else if (!strncmp(bsoflags_tok, "none", 4)) {
>> +			open_flags = 0;
>> +			break;
>> +		}
>> +		else {
>> +			eprintf("bsoflag option %s not supported\n",
>> +				bsoflags_tok);
>> +			return -1;
>> +		}
>> +
>> +		bsoflags_tok = strtok(NULL, " ");
>> +	}
>> +
>> +	return open_flags;
>> +}
>> +
>> +char *open_flags_to_str(char *dest, int flags)
>> +{
>> +	*dest = '\0';
>> +
>> +	if (flags == 0)
>> +		strcat(dest, "none");
>> +	if (flags & O_SYNC)
>> +		strcat(dest, "sync");
>> +	if (flags & O_DIRECT) {
>> +		if (*dest)
>> +			strcat(dest, " ");
>> +		strcat(dest, "direct");
>> +	}
>> +	return dest;
>> +}
>> diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
>> index 6068479..7b593c8 100644
>> --- a/usr/bs_rdwr.c
>> +++ b/usr/bs_rdwr.c
>> @@ -130,7 +130,7 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
>>  
>>  static int bs_rdwr_open(struct scsi_lu *lu, char *path, int *fd, uint64_t *size)
>>  {
>> -	*fd = backed_file_open(path, O_RDWR| O_LARGEFILE, size);
>> +	*fd = backed_file_open(path, O_RDWR | O_LARGEFILE | lu->bsoflags, size);
>>  	if (*fd < 0)
>>  		return *fd;
>>  
>> diff --git a/usr/spc.c b/usr/spc.c
>> index 14a3ee1..4535c80 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -1581,6 +1581,7 @@ enum {
>>  	Opt_removable, Opt_online,
>>  	Opt_mode_page,
>>  	Opt_path,
>> +	Opt_bsoflags,
>>  	Opt_err,
>>  };
>>  
>> @@ -1595,6 +1596,7 @@ static match_table_t tokens = {
>>  	{Opt_online, "online=%s"},
>>  	{Opt_mode_page, "mode_page=%s"},
>>  	{Opt_path, "path=%s"},
>> +	{Opt_bsoflags, "bsoflags=%s"},
>>  	{Opt_err, NULL},
>>  };
>>  
>> @@ -1626,7 +1628,8 @@ int lu_config(struct scsi_lu *lu, char *params, match_fn_t *fn)
>>  
>>  	while ((p = strsep(&params, ",")) != NULL) {
>>  		substring_t args[MAX_OPT_ARGS];
>> -		int token;
>> +		int token, lu_bsoflags = 0;
>> +
>>  		if (!*p)
>>  			continue;
>>  		token = match_token(p, tokens, args);
>> @@ -1676,6 +1679,18 @@ int lu_config(struct scsi_lu *lu, char *params, match_fn_t *fn)
>>  			match_strncpy(buf, &args[0], sizeof(buf));
>>  			err = tgt_device_path_update(lu->tgt, lu, buf);
>>  			break;
>> +		case Opt_bsoflags:
>> +			match_strncpy(buf, &args[0], sizeof(buf));
>> +			lu_bsoflags = str_to_open_flags(buf);
>> +			if (lu_bsoflags == -1) {
>> +				err = TGTADM_INVALID_REQUEST;
>> +				break;
>> +			}
>> +			lu->bsoflags = lu_bsoflags;
>> +			/* Keep the same path and update only the flags. */
>> +			strcpy(buf, lu->path);
>> +			err = tgt_device_path_update(lu->tgt, lu, buf);
>> +			break;
>>  		default:
>>  			err |= fn ? fn(lu, p) : TGTADM_INVALID_REQUEST;
>>  		}
>> diff --git a/usr/target.c b/usr/target.c
>> index c848757..17f8a04 100644
>> --- a/usr/target.c
>> +++ b/usr/target.c
>> @@ -416,20 +416,21 @@ __device_lookup(int tid, uint64_t lun, struct target **t)
>>  }
>>  
>>  enum {
>> -	Opt_path, Opt_bstype, Opt_err,
>> +	Opt_path, Opt_bstype, Opt_bsoflags, Opt_err,
>>  };
>>  
>>  static match_table_t device_tokens = {
>>  	{Opt_path, "path=%s"},
>>  	{Opt_bstype, "bstype=%s"},
>> +	{Opt_bsoflags, "bsoflags=%s"},
>>  	{Opt_err, NULL},
>>  };
>>  
>>  int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
>>  		      int backing)
>>  {
>> -	char *p, *path = NULL, *bstype = NULL;
>> -	int ret = 0;
>> +	char *p, *path = NULL, *bstype = NULL, *bsoflags = NULL;
>> +	int ret = 0, lu_bsoflags;
>>  	struct target *target;
>>  	struct scsi_lu *lu, *pos;
>>  	struct device_type_template *t;
>> @@ -452,6 +453,8 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
>>  		case Opt_bstype:
>>  			bstype = match_strdup(&args[0]);
>>  			break;
>> +		case Opt_bsoflags:
>> +			bsoflags = match_strdup(&args[0]);
>>  		default:
>>  			break;
>>  		}
>> @@ -482,6 +485,12 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
>>  		}
>>  	}
>>  
>> +	lu_bsoflags = str_to_open_flags(bsoflags);
>> +	if (lu_bsoflags == -1) {
>> +		ret = TGTADM_INVALID_REQUEST;
>> +		goto out;
>> +	}
>> +
>>  	t = device_type_lookup(dev_type);
>>  	if (!t) {
>>  		eprintf("Unknown device type %d\n", dev_type);
>> @@ -499,6 +508,8 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
>>  	lu->bst = bst;
>>  	lu->tgt = target;
>>  	lu->lun = lun;
>> +	lu->bsoflags = lu_bsoflags;
>> +
>>  	tgt_cmd_queue_init(&lu->cmd_queue);
>>  	INIT_LIST_HEAD(&lu->registration_list);
>>  	lu->prgeneration = 0;
>> @@ -565,6 +576,8 @@ out:
>>  		free(bstype);
>>  	if (path)
>>  		free(path);
>> +	if (bsoflags)
>> +		free(bsoflags);
>>  	return ret;
>>  fail_bs_init:
>>  	if (lu->bst->bs_exit)
>> @@ -1582,10 +1595,10 @@ static char *print_type(int type)
>>  	return name;
>>  }
>>  
>> -
>>  int tgt_target_show_all(char *buf, int rest)
>>  {
>>  	int total = 0, max = rest;
>> +	char strflags[128];
>>  	struct target *target;
>>  	struct scsi_lu *lu;
>>  	struct acl_entry *acl;
>> @@ -1622,7 +1635,8 @@ int tgt_target_show_all(char *buf, int rest)
>>  				 _TAB3 "Online: %s\n"
>>  				 _TAB3 "Removable media: %s\n"
>>  				 _TAB3 "Backing store type: %s\n"
>> -				 _TAB3 "Backing store path: %s\n",
>> +				 _TAB3 "Backing store path: %s\n"
>> +				 _TAB3 "Backing store flags: %s\n",
>>  				 lu->lun,
>>    				 print_type(lu->attrs.device_type),
>>  				 lu->attrs.scsi_id,
>> @@ -1633,7 +1647,8 @@ int tgt_target_show_all(char *buf, int rest)
>>  				 lu->bst ?
>>  					(lu->bst->bs_name ? : "Unknown") :
>>  					"None",
>> -				 lu->path ? : "None");
>> +				 lu->path ? : "None",
>> +				 open_flags_to_str(strflags, lu->bsoflags));
>>  
>>  		if (!strcmp(tgt_drivers[target->lid]->name, "iscsi")) {
>>  			int i, aid;
>> diff --git a/usr/tgtadm.c b/usr/tgtadm.c
>> index 695c1c6..5dbdeaa 100644
>> --- a/usr/tgtadm.c
>> +++ b/usr/tgtadm.c
>> @@ -112,6 +112,7 @@ struct option const long_options[] = {
>>  	{"value", required_argument, NULL, 'v'},
>>  	{"backing-store", required_argument, NULL, 'b'},
>>  	{"bstype", required_argument, NULL, 'E'},
>> +	{"bsoflags", required_argument, NULL, 'f'},
>>  	{"targetname", required_argument, NULL, 'T'},
>>  	{"initiator-address", required_argument, NULL, 'I'},
>>  	{"user", required_argument, NULL, 'u'},
>> @@ -126,7 +127,7 @@ struct option const long_options[] = {
>>  	{NULL, 0, NULL, 0},
>>  };
>>  
>> -static char *short_options = "dhL:o:m:t:s:c:l:n:v:b:E:T:I:u:p:H:P:B:Y:O:C:";
>> +static char *short_options = "dhL:o:m:t:s:c:l:n:v:b:E:f:T:I:u:p:H:P:B:Y:O:C:";
>>  
>>  static void usage(int status)
>>  {
>> @@ -153,12 +154,15 @@ Linux SCSI Target Framework Administration Utility, version %s\n\
>>                          enable the target to accept the specific initiators.\n\
>>    --lld [driver] --mode target --op unbind --tid=[id] --initiator-address=[src]\n\
>>                          disable the specific permitted initiators.\n\
>> -  --lld [driver] --mode logicalunit --op new --tid=[id] --lun=[lun] --backing-store=[path] --bstype=[type]\n\
>> +  --lld [driver] --mode logicalunit --op new --tid=[id] --lun=[lun] \\\n\
>> +                        --backing-store=[path] --bstype=[type] --bsoflags=[options]\n\
>>                          add a new logical unit with [lun] to the specific\n\
>>                          target with [id]. The logical unit is offered\n\
>>                          to the initiators. [path] must be block device files\n\
>>                          (including LVM and RAID devices) or regular files.\n\
>>                          bstype option is optional.\n\
>> +                        bsoflags supported options are sync and direct.	\n\
>> +                        Use \"none\" to update with no flags. It is optional.\n				\
>>    --lld [driver] --mode logicalunit --op delete --tid=[id] --lun=[lun]\n\
>>                          delete the specific logical unit with [lun] that\n\
>>                          the target with [id] has.\n\
>> @@ -431,6 +435,7 @@ int main(int argc, char **argv)
>>  	uint64_t sid, lun;
>>  	char *name, *value, *path, *targetname, *params, *address, *targetOps;
>>  	char *bstype;
>> +	char *bsoflags;
>>  	char *user, *password;
>>  	char *buf;
>>  	size_t bufsz = BUFSIZE + sizeof(struct tgtadm_req);
>> @@ -445,7 +450,7 @@ int main(int argc, char **argv)
>>  	ac_dir = ACCOUNT_TYPE_INCOMING;
>>  	rest = BUFSIZE;
>>  	name = value = path = targetname = address = targetOps = bstype = NULL;
>> -	user = password = NULL;
>> +	bsoflags = user = password = NULL;
>>  
>>  	buf = valloc(bufsz);
>>  	if (!buf) {
>> @@ -516,6 +521,9 @@ int main(int argc, char **argv)
>>  		case 'H':
>>  			hostno = strtol(optarg, NULL, 10);
>>  			break;
>> +		case 'f':
>> +			bsoflags = optarg;
>> +			break;
>>  		case 'E':
>>  			bstype = optarg;
>>  			break;
>> @@ -619,7 +627,7 @@ int main(int argc, char **argv)
>>  	if (mode == MODE_ACCOUNT) {
>>  		switch (op) {
>>  		case OP_NEW:
>> -			rc = verify_mode_params(argc, argv, "LmoupC");
>> +			rc = verify_mode_params(argc, argv, "LmoupfC");
>>  			if (rc) {
>>  				eprintf("logicalunit mode: option '-%c' is not "
>>  					  "allowed/supported\n", rc);
>> @@ -696,7 +704,7 @@ int main(int argc, char **argv)
>>  		}
>>  		switch (op) {
>>  		case OP_NEW:
>> -			rc = verify_mode_params(argc, argv, "LmotlbEYC");
>> +			rc = verify_mode_params(argc, argv, "LmoftlbEYC");
>>  			if (rc) {
>>  				eprintf("target mode: option '-%c' is not "
>>  					  "allowed/supported\n", rc);
>> @@ -717,7 +725,7 @@ int main(int argc, char **argv)
>>  			}
>>  			break;
>>  		case OP_UPDATE:
>> -			rc = verify_mode_params(argc, argv, "LmotlPC");
>> +			rc = verify_mode_params(argc, argv, "LmoftlPC");
>>  			if (rc) {
>>  				eprintf("option '-%c' not supported in "
>>  					"logicalunit mode\n", rc);
>> @@ -756,7 +764,9 @@ int main(int argc, char **argv)
>>  	else if (bstype)
>>  		shprintf(total, params, rest, "%sbstype=%s",
>>  			 rest == BUFSIZE ? "" : ",", bstype);
>> +	if (bsoflags)
>> +		shprintf(total, params, rest, "%sbsoflags=%s",
>> +			 rest == BUFSIZE ? "" : ",", bsoflags);
>>  	if (targetname)
>>  		shprintf(total, params, rest, "%stargetname=%s",
>>  			 rest == BUFSIZE ? "" : ",", targetname);
>> -- 
>> 1.6.3.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe stgt" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



More information about the stgt mailing list