OK, I've tried to implement a similar checking mechanism as
open iscsi do.
Please let me know what you think about it.
P.S.
I'm still checking the -b option when creating a new device
because currently this option is required.
In the code we don't distinguish between the different targets.
So I'm just syncing the validation to the code itself.
Thanks,
Doron
check for valid parameters when passing tgtadm commands
Signed-off-by: Doron Shoham <dorons@xxxxxxxxxxxx>
---
usr/tgtadm.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 136 insertions(+), 26 deletions(-)
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 9e90b40..6f62cdb 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -356,9 +356,27 @@ static int str_to_op(char *str)
}
}
-int main(int argc, char **argv)
+static int verify_mode_params(int argc, char **argv, char *allowed)
{
int ch, longindex;
+ int ret = 0;
+
+ optind = 0;
+
+ while ((ch = getopt_long(argc, argv, short_options,
+ long_options, &longindex)) >= 0) {
+ if (!strchr(allowed, ch)) {
+ ret = ch;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+int main(int argc, char **argv)
+{
+ int ch, longindex, rc;
int op, total, tid, rest, mode, dev_type, ac_dir;
uint32_t cid, hostno;
uint64_t sid, lun;
@@ -369,7 +387,7 @@ int main(int argc, char **argv)
struct tgtadm_req *req;
op = tid = mode = -1;
- total = cid = hostno = sid = lun = 0;
+ total = cid = hostno = sid = lun = rc = 0;
dev_type = TYPE_DISK;
ac_dir = ACCOUNT_TYPE_INCOMING;
rest = BUFSIZE;
@@ -384,6 +402,7 @@ int main(int argc, char **argv)
optind = 1;
while ((ch = getopt_long(argc, argv, short_options,
long_options, &longindex)) >= 0) {
+ errno = 0;
switch (ch) {
case 'L':
strncpy(req->lld, optarg, sizeof(req->lld));
@@ -396,6 +415,10 @@ int main(int argc, char **argv)
break;
case 't':
tid = strtol(optarg, NULL, 10);
+ if (errno == EINVAL) {
+ eprintf("invalid tid '%s'\n", optarg);
+ exit(EINVAL);
+ }
break;
case 's':
sid = strtoull(optarg, NULL, 10);
@@ -457,10 +480,7 @@ int main(int argc, char **argv)
}
if (optind < argc) {
- eprintf("unrecognized options: ");
- while (optind < argc)
- eprintf("%s", argv[optind++]);
- eprintf("\n");
+ eprintf("unrecognized option '%s'\n", argv[optind]);
usage(1);
}
@@ -484,35 +504,57 @@ int main(int argc, char **argv)
/* exit(EINVAL); */
}
- if ((name || value) && op != OP_UPDATE) {
- eprintf("only 'update' operation takes"
- " 'name' and 'value' options\n");
- exit(EINVAL);
- }
-
- if ((!name && value) || (name && !value)) {
- eprintf("'name' and 'value' options are necessary\n");
- exit(EINVAL);
- }
-
if (mode == MODE_TARGET) {
+ if ((tid <= 0 && (op != OP_SHOW))) {
+ eprintf("'tid' option is necessary\n");
+ exit(EINVAL);
+ }
switch (op) {
case OP_NEW:
+ if ((rc = verify_mode_params(argc, argv, "LmotT"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ if (!targetname) {
+ eprintf("creating a new target needs the name\n");
+ exit(EINVAL);
+ }
+ break;
case OP_DELETE:
+ case OP_SHOW:
+ if ((rc = verify_mode_params(argc, argv, "Lmot"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ break;
case OP_BIND:
case OP_UNBIND:
+ if ((rc = verify_mode_params(argc, argv, "LmotI"))) {
+ eprintf("target mode op delete: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ if (!address) {
+ eprintf("%s operation requires initiator-address\n", op==OP_BIND?"bind":"unbind");
+ exit(EINVAL);
+ }
+ break;
case OP_UPDATE:
- if (op == OP_NEW && !targetname) {
- eprintf("creating a new target needs the name\n");
+ if ((rc = verify_mode_params(argc, argv, "Lmotnv"))) {
+ eprintf("target mode op delete: option '-%c' is not "
+ "allowed/supported\n", rc);
exit(EINVAL);
}
-
- if (tid < 0) {
- eprintf("'tid' option is necessary\n");
+ if ((!name || !value)) {
+ eprintf("update operation requires 'name' and 'value' options\n");
exit(EINVAL);
}
break;
default:
+ eprintf("option %d not supported in target mode\n", op);
+ exit(EINVAL);
break;
}
}
@@ -520,29 +562,97 @@ int main(int argc, char **argv)
if (mode == MODE_ACCOUNT) {
switch (op) {
case OP_NEW:
+ if ((rc = verify_mode_params(argc, argv, "Lmoup"))) {
+ eprintf("logicalunit mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
if (!user || !password) {
eprintf("'user' and 'password' options is necessary\n");
exit(EINVAL);
}
break;
case OP_SHOW:
+ if ((rc = verify_mode_params(argc, argv, "Lmo"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
break;
case OP_DELETE:
+ if ((rc = verify_mode_params(argc, argv, "Lmou"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ break;
case OP_BIND:
+ if ((rc = verify_mode_params(argc, argv, "LmotuO"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ if (!user) {
+ eprintf("'user' option is necessary\n");
+ exit(EINVAL);
+ }
+ if (tid <= 0) {
+ eprintf("'tid' option is necessary\n");
+ exit(EINVAL);
+ }
+ break;
case OP_UNBIND:
+ if ((rc = verify_mode_params(argc, argv, "Lmou"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
if (!user) {
eprintf("'user' option is necessary\n");
exit(EINVAL);
}
-
- if ((op == OP_BIND || op == OP_UNBIND) && tid < 0) {
+ if (tid <= 0) {
eprintf("'tid' option is necessary\n");
exit(EINVAL);
}
break;
default:
- eprintf("the update operation can't"
- " handle accounts\n");
+ eprintf("option %d not supported in account mode\n", op);
+ exit(EINVAL);
+ break;
+ }
+ }
+
+ if (mode == MODE_DEVICE) {
+ if (tid <= 0) {
+ eprintf("'tid' option is necessary\n");
+ exit(EINVAL);
+ }
+ if (!lun) {
+ eprintf("'lun' option is necessary\n");
+ exit(EINVAL);
+ }
+ switch (op) {
+ case OP_NEW:
+ if ((rc = verify_mode_params(argc, argv, "Lmotlb"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ if (!path) {
+ eprintf("'backing-store' option is necessary\n");
+ exit(EINVAL);
+ }
+ break;
+ case OP_DELETE:
+ if ((rc = verify_mode_params(argc, argv, "Lmotl"))) {
+ eprintf("target mode: option '-%c' is not "
+ "allowed/supported\n", rc);
+ exit(EINVAL);
+ }
+ break;
+ default:
+ eprintf("option %d not supported in logicalunit mode\n", op);
exit(EINVAL);
break;
}
--
1.5.3.8
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
