From c037373ef0cc5a00a06f30a08f6f66977d46fb4e Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Wed, 20 Nov 2024 15:31:15 +0100 Subject: [PATCH] LU-18462 nodemap: sanity checks for positional parameters A number of nodemap commands just take positional parameters instead of options: - nodemap_activate - nodemap_add - nodemap_del - nodemap_test_nid In this case, we need to check manually that we got the correct number of arguments, and return a proper error message otherwise. These commands are also modified to grok named options, in addition to positional parameters that we will have to keep for some time for backward compatibility reasons. Test-Parameters: trivial testlist=sanity-sec Signed-off-by: Sebastien Buisson Change-Id: Ie515e8e03f30581c7fb84efa3f881769fc02a397 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57087 Reviewed-by: Marc Vef Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin Tested-by: jenkins Tested-by: Maloo --- lustre/doc/Makefile.am | 2 + lustre/doc/lctl-nodemap-add.8 | 15 ++-- lustre/doc/lctl-nodemap-del.8 | 8 +- lustre/doc/lctl-nodemap-modify.8 | 2 +- lustre/doc/lctl-nodemap-test-nid.8 | 38 +++++++++ lustre/doc/lctl-nodemap_test_nid.8 | 1 + lustre/utils/lctl.c | 12 +-- lustre/utils/obd.c | 164 ++++++++++++++++++------------------- 8 files changed, 141 insertions(+), 101 deletions(-) create mode 100644 lustre/doc/lctl-nodemap-test-nid.8 create mode 100644 lustre/doc/lctl-nodemap_test_nid.8 diff --git a/lustre/doc/Makefile.am b/lustre/doc/Makefile.am index e9fc4d7..aa8ea8c 100644 --- a/lustre/doc/Makefile.am +++ b/lustre/doc/Makefile.am @@ -253,6 +253,8 @@ SERVER_MANFILES = \ lctl-nodemap_set_fileset.8 \ lctl-nodemap-set-sepol.8 \ lctl-nodemap_set_sepol.8 \ + lctl-nodemap-test-nid.8 \ + lctl-nodemap_test_nid.8 \ lctl-nodemap-add-offset.8 \ lctl-nodemap_add_offset.8 \ lctl-nodemap-del-offset.8 \ diff --git a/lustre/doc/lctl-nodemap-add.8 b/lustre/doc/lctl-nodemap-add.8 index eee2a47..102f3f9 100644 --- a/lustre/doc/lctl-nodemap-add.8 +++ b/lustre/doc/lctl-nodemap-add.8 @@ -5,8 +5,9 @@ lctl-nodemap_add \- create a new nodemap to define client behavior .SY "lctl nodemap_add" or .SY "lctl nodemap add" -.BR "[ " -d "|" --dynamic " ] " \fINODEMAP_NAME -.br +.RB [ -d | --dynamic ] +.BI --name " NODEMAP_NAME" +.YS .SH DESCRIPTION .B nodemap_add creates and names a new nodemap to which NID ranges, process identities, @@ -18,14 +19,14 @@ and filesystem access permission of those NID(s). Adds a temporary in-memory nodemap on the local node instead of a persistent one. .TP -.I NODEMAP_NAME -The name to give the new nodemap. It can be any alphanumeric string of -maximum length 16, except +.BI --name " NODEMAP_NAME" +The name to give the new nodemap. It can be any alphanumeric string of maximum +length 16, except .RB \(dq default \(dq. .SH EXAMPLES .EX -.B # lctl nodemap_add remotesite -.B # lctl nodemap_add othersite +.B # lctl nodemap_add --name remotesite +.B # lctl nodemap_add --name othersite .EE .SH AVAILABILITY .B lctl nodemap_add diff --git a/lustre/doc/lctl-nodemap-del.8 b/lustre/doc/lctl-nodemap-del.8 index e787116..75c8584 100644 --- a/lustre/doc/lctl-nodemap-del.8 +++ b/lustre/doc/lctl-nodemap-del.8 @@ -5,7 +5,7 @@ lctl-nodemap_del \- delete an existing nodemap .SY "lctl nodemap_del" or .SY "lctl nodemap del" -.I NODEMAP_NAME +.BI --name " NODEMAP_NAME" .YS .SH DESCRIPTION .B nodemap_del @@ -14,12 +14,12 @@ NID ranges will be removed as well, and existing clients will be moved to the default nodemap. .SH OPTIONS .TP -.I NODEMAP_NAME +.BI --name " NODEMAP_NAME" The name of the nodemap to delete. The default nodemap cannot be deleted. .SH EXAMPLES .EX -.B # lctl nodemap_del remotesite -.B # lctl nodemap_del othersite +.B # lctl nodemap_del --name remotesite +.B # lctl nodemap_del --name othersite .EE .SH AVAILABILITY .B lctl nodemap_del diff --git a/lustre/doc/lctl-nodemap-modify.8 b/lustre/doc/lctl-nodemap-modify.8 index ac6fe61..61b29af 100644 --- a/lustre/doc/lctl-nodemap-modify.8 +++ b/lustre/doc/lctl-nodemap-modify.8 @@ -15,7 +15,7 @@ modifies a property of the given nodemap. .SH OPTIONS .TP .BI --name " NODEMAP_NAME" -Rhe name of the nodemap to modify +The name of the nodemap to modify .TP .BI --property " PROPERTY_NAME" One of the following properties: diff --git a/lustre/doc/lctl-nodemap-test-nid.8 b/lustre/doc/lctl-nodemap-test-nid.8 new file mode 100644 index 0000000..30a0428 --- /dev/null +++ b/lustre/doc/lctl-nodemap-test-nid.8 @@ -0,0 +1,38 @@ +.TH LCTL-NODEMAP_TEST_NID 8 2024-11-22 Lustre "Lustre Configuration Utilities" +.SH NAME +lctl-nodemap_test_nid \- test a nid for nodemap membership +.SH SYNOPSIS +.SY "lctl nodemap_test_nid" +.BI --nid " NID" +.YS +.SH DESCRIPTION +.B nodemap_test_nid +tests +.I NID +for nodemap membership, +and returns the name of the nodemap it belongs to. +.SH OPTIONS +.TP +.BI --nid " NID" +The nid to test membership of. +.SH EXAMPLES +.EX +.B # lctl nodemap_test_nid --nid 192.168.10.101@tcp0 +.B # lctl nodemap_test_nid --nid 10.10.0.55@o2ib3 +.EE +.SH AVAILABILITY +.B lctl nodemap_test_nid +is part of the +.BR lustre (7) +filesystem package since release 2.6.0 +.\" Added in commit v2_5_56_0-13-g4642f30970 +.SH SEE ALSO +.BR lustre (7), +.BR lctl-nodemap-activate (8), +.BR lctl-nodemap-add (8), +.BR lctl-nodemap-add-idmap (8), +.BR lctl-nodemap-add-range (8), +.BR lctl-nodemap-del (8), +.BR lctl-nodemap-del-idmap (8), +.BR lctl-nodemap-del-range (8), +.BR lctl-nodemap-modify (8) diff --git a/lustre/doc/lctl-nodemap_test_nid.8 b/lustre/doc/lctl-nodemap_test_nid.8 new file mode 100644 index 0000000..6abac83 --- /dev/null +++ b/lustre/doc/lctl-nodemap_test_nid.8 @@ -0,0 +1 @@ +.so man8/lctl-nodemap-test-nid.8 diff --git a/lustre/utils/lctl.c b/lustre/utils/lctl.c index cffbada..a46694c 100644 --- a/lustre/utils/lctl.c +++ b/lustre/utils/lctl.c @@ -166,10 +166,10 @@ command_t nodemap_cmdlist[] = { "usage: nodemap activate {0|1}"}, {.pc_name = "add", .pc_func = jt_nodemap_add, .pc_help = "add a new nodemap\n" - "usage: nodemap add [-d|--dynamic] NODEMAP_NAME"}, + "usage: nodemap add [-d|--dynamic] --name NODEMAP_NAME"}, {.pc_name = "del", .pc_func = jt_nodemap_del, .pc_help = "remove a nodemap\n" - "usage: nodemap del NODEMAP_NAME"}, + "usage: nodemap del --name NODEMAP_NAME"}, {.pc_name = "add_range", .pc_func = jt_nodemap_add_range, .pc_help = "add a nid range to a nodemap\n" "usage: nodemap add_range --name NODEMAP_NAME --range NID_RANGE"}, @@ -203,7 +203,7 @@ command_t nodemap_cmdlist[] = { "usage: nodemap set_sepol --name NODEMAP_NAME --sepol SEPOL"}, {.pc_name = "test_nid", .pc_func = jt_nodemap_test_nid, .pc_help = "test a nid for nodemap membership\n" - "usage: nodemap test_nid NID"}, + "usage: nodemap test_nid --nid NID"}, {.pc_name = "test_id", .pc_func = jt_nodemap_test_id, .pc_help = "test a nodemap id pair for mapping\n" "usage: nodemap test_id --nid NID --idtype {uid|gid|projid} --id ID"}, @@ -591,10 +591,10 @@ command_t cmdlist[] = { "usage: nodemap_activate {0|1}"}, {"nodemap_add", jt_nodemap_add, 0, "add a new nodemap\n" - "usage: nodemap_add [-d|--dynamic] NODEMAP_NAME"}, + "usage: nodemap_add [-d|--dynamic] --name NODEMAP_NAME"}, {"nodemap_del", jt_nodemap_del, 0, "remove a nodemap\n" - "usage: nodemap_del NODEMAP_NAME"}, + "usage: nodemap_del --name NODEMAP_NAME"}, {"nodemap_add_range", jt_nodemap_add_range, 0, "add a nid range to a nodemap\n" "usage: nodemap_add_range --name NODEMAP_NAME --range NID_RANGE"}, @@ -624,7 +624,7 @@ command_t cmdlist[] = { "usage: nodemap_set_sepol --name NODEMAP_NAME --sepol SEPOL"}, {"nodemap_test_nid", jt_nodemap_test_nid, 0, "test a nid for nodemap membership\n" - "usage: nodemap_test_nid NID"}, + "usage: nodemap_test_nid --nid NID"}, {"nodemap_test_id", jt_nodemap_test_id, 0, "test a nodemap id pair for mapping\n" "Usage: nodemap_test_id --nid NID --idtype ID_TYPE --id ID"}, diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index 3db76d6..3e4988d 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -3996,13 +3996,17 @@ out: */ int jt_nodemap_activate(int argc, char **argv) { - int rc; + int rc = EXIT_SUCCESS; - rc = nodemap_cmd(LCFG_NODEMAP_ACTIVATE, false, NULL, 0, - argv[0], argv[1], NULL); + if (argc != 2) + return CMD_HELP; - if (rc != 0) + errno = -nodemap_cmd(LCFG_NODEMAP_ACTIVATE, false, NULL, 0, + argv[0], argv[1], NULL); + if (errno) { + rc = EXIT_FAILURE; perror(argv[0]); + } return rc; } @@ -4019,56 +4023,60 @@ int jt_nodemap_activate(int argc, char **argv) */ int jt_nodemap_add(int argc, char **argv) { + char *nodemap_name = NULL; bool dynamic = false; - char *nm_name = NULL; - int c, rc; + int c, rc = EXIT_SUCCESS; static struct option long_opts[] = { { .val = 'd', .name = "dynamic", .has_arg = no_argument }, { .val = 'h', .name = "help", .has_arg = no_argument }, + { .val = 'n', .name = "name", .has_arg = required_argument }, { .name = NULL } }; - while ((c = getopt_long(argc, argv, "dh", + while ((c = getopt_long(argc, argv, "dhn:", long_opts, NULL)) != -1) { switch (c) { case 'd': dynamic = true; break; + case 'n': + nodemap_name = optarg; + break; case 'h': default: - goto add_usage; + return CMD_HELP; } } - if (optind < argc) - nm_name = argv[optind]; - - if (!nm_name) { - fprintf(stderr, "nodemap_add: missing nodemap name\n"); -add_usage: - fprintf(stderr, - "usage: nodemap_add [-d|--dynamic] NODEMAP_NAME\n"); - return -EINVAL; + if (!nodemap_name) { + if (optind >= argc) { + fprintf(stderr, "nodemap_add: missing nodemap name\n"); + return CMD_HELP; + } + nodemap_name = argv[optind]; } if (!dynamic && !is_mgs()) { fprintf(stderr, "nodemap_add: non-dynamic nodemap only allowed on MGS node\n"); - goto add_usage; + return CMD_HELP; } - rc = llapi_nodemap_exists(nm_name); - if (rc == 0) { - fprintf(stderr, "error: %s existing nodemap name\n", nm_name); - return 1; + if (llapi_nodemap_exists(nodemap_name) == 0) { + fprintf(stderr, "error: nodemap '%s' already exists\n", + nodemap_name); + errno = EINVAL; + goto out; } - rc = nodemap_cmd(LCFG_NODEMAP_ADD, dynamic, NULL, 0, argv[0], - nm_name, NULL); + errno = -nodemap_cmd(LCFG_NODEMAP_ADD, dynamic, NULL, 0, argv[0], + nodemap_name, NULL); - if (rc != 0) +out: + if (errno) { + rc = EXIT_FAILURE; perror(argv[0]); - + } return rc; } @@ -4084,45 +4092,47 @@ add_usage: */ int jt_nodemap_del(int argc, char **argv) { - char *nm_name = NULL; - int c, rc; + char *nodemap_name = NULL; + int c, rc = EXIT_SUCCESS; static struct option long_opts[] = { - { .val = 'h', .name = "help", .has_arg = no_argument }, + { .val = 'h', .name = "help", .has_arg = no_argument }, + { .val = 'n', .name = "name", .has_arg = required_argument }, { .name = NULL } }; - while ((c = getopt_long(argc, argv, "h", - long_opts, NULL)) != -1) { + while ((c = getopt_long(argc, argv, "hn:", long_opts, NULL)) != -1) { switch (c) { + case 'n': + nodemap_name = optarg; + break; case 'h': default: - goto del_usage; + return CMD_HELP; } } - if (optind < argc) - nm_name = argv[optind]; - - if (!nm_name) { - fprintf(stderr, "nodemap_del: missing nodemap name\n"); -del_usage: - fprintf(stderr, - "usage: nodemap_del NODEMAP_NAME\n"); - return -EINVAL; + if (!nodemap_name) { + if (argc != 2) { + fprintf(stderr, "nodemap_del: missing nodemap name\n"); + return CMD_HELP; + } + nodemap_name = argv[1]; } - rc = llapi_nodemap_exists(nm_name); - if (rc != 0) { - fprintf(stderr, "error: %s not existing nodemap name\n", - nm_name); - return rc; + if (llapi_nodemap_exists(nodemap_name) != 0) { + fprintf(stderr, "error: nodemap '%s' does not exist\n", + nodemap_name); + errno = EINVAL; + goto out; } - rc = nodemap_cmd(LCFG_NODEMAP_DEL, false, NULL, 0, argv[0], - nm_name, NULL); + errno = -nodemap_cmd(LCFG_NODEMAP_DEL, false, NULL, 0, argv[0], + nodemap_name, NULL); - if (rc != 0) +out: + if (errno) { + rc = EXIT_FAILURE; perror(argv[0]); - + } return rc; } @@ -4140,36 +4150,38 @@ int jt_nodemap_test_nid(int argc, char **argv) { char rawbuf[MAX_IOC_BUFLEN]; char *nid = NULL; - int c, rc; + int c, rc = EXIT_SUCCESS; static struct option long_opts[] = { - { .val = 'h', .name = "help", .has_arg = no_argument }, + { .val = 'h', .name = "help", .has_arg = no_argument }, + { .val = 'n', .name = "nid", .has_arg = required_argument }, { .name = NULL } }; - while ((c = getopt_long(argc, argv, "dh", - long_opts, NULL)) != -1) { + while ((c = getopt_long(argc, argv, "hn:", long_opts, NULL)) != -1) { switch (c) { + case 'n': + nid = optarg; + break; case 'h': default: - goto test_nid_usage; + return CMD_HELP; } } - if (optind < argc) - nid = argv[optind]; - if (!nid) { - fprintf(stderr, "nodemap_test_nid: missing NID\n"); -test_nid_usage: - fprintf(stderr, - "usage: nodemap_test_nid NID\n"); - return -EINVAL; + if (argc != 2) + return CMD_HELP; + nid = argv[1]; } - rc = nodemap_cmd(LCFG_NODEMAP_TEST_NID, false, &rawbuf, - sizeof(rawbuf), argv[0], nid, NULL); - if (rc == 0) + errno = -nodemap_cmd(LCFG_NODEMAP_TEST_NID, false, &rawbuf, + sizeof(rawbuf), argv[0], nid, NULL); + if (errno) { + rc = EXIT_FAILURE; + perror(argv[0]); + } else { printf("%s\n", (char *)rawbuf); + } return rc; } @@ -4216,26 +4228,12 @@ int jt_nodemap_test_id(int argc, char **argv) break; case 'h': default: - goto test_id_usage; + return CMD_HELP; } } - if (!nidstr) { - fprintf(stderr, "nodemap_test_id: missing NID\n"); -test_id_usage: - fprintf(stderr, - "usage: nodemap_test_id --nid NID --idtype IDTYPE --id ID\n"); - return -EINVAL; - } - if (!typestr) { - fprintf(stderr, - "nodemap_test_id: missing idtype, must be one of uid, gid, projid\n"); - goto test_id_usage; - } - if (!idstr) { - fprintf(stderr, "nodemap_test_id: missing id\n"); - goto test_id_usage; - } + if (!nidstr || !typestr || !idstr) + return CMD_HELP; rc = nodemap_cmd(LCFG_NODEMAP_TEST_ID, false, &rawbuf, sizeof(rawbuf), argv[0], nidstr, typestr, idstr, NULL); -- 1.8.3.1