[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(¶ms, ",")) != 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