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 |