Whamcloud - gitweb
LU-18462 nodemap: sanity checks for positional parameters 87/57087/14
authorSebastien Buisson <sbuisson@ddn.com>
Wed, 20 Nov 2024 14:31:15 +0000 (15:31 +0100)
committerOleg Drokin <green@whamcloud.com>
Wed, 26 Mar 2025 03:59:45 +0000 (03:59 +0000)
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 <sbuisson@ddn.com>
Change-Id: Ie515e8e03f30581c7fb84efa3f881769fc02a397
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57087
Reviewed-by: Marc Vef <mvef@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
lustre/doc/Makefile.am
lustre/doc/lctl-nodemap-add.8
lustre/doc/lctl-nodemap-del.8
lustre/doc/lctl-nodemap-modify.8
lustre/doc/lctl-nodemap-test-nid.8 [new file with mode: 0644]
lustre/doc/lctl-nodemap_test_nid.8 [new file with mode: 0644]
lustre/utils/lctl.c
lustre/utils/obd.c

index e9fc4d7..aa8ea8c 100644 (file)
@@ -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               \
index eee2a47..102f3f9 100644 (file)
@@ -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
index e787116..75c8584 100644 (file)
@@ -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
index ac6fe61..61b29af 100644 (file)
@@ -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 (file)
index 0000000..30a0428
--- /dev/null
@@ -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 (file)
index 0000000..6abac83
--- /dev/null
@@ -0,0 +1 @@
+.so man8/lctl-nodemap-test-nid.8
index cffbada..a46694c 100644 (file)
@@ -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"},
index 3db76d6..3e4988d 100644 (file)
@@ -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);