Whamcloud - gitweb
LU-15142 lctl: fixes for set_param -P and llog_print 32/45332/5
authorMikhail Pershin <mpershin@whamcloud.com>
Thu, 14 Oct 2021 14:16:21 +0000 (17:16 +0300)
committerOleg Drokin <green@whamcloud.com>
Sat, 20 Nov 2021 06:29:37 +0000 (06:29 +0000)
- properly handle permanent param deletion
- don't print skipped parameters in llog_print output
- add --raw option to llog_print to output all entries
  including markers

Signed-off-by: Mikhail Pershin <mpershin@whamcloud.com>
Change-Id: Id93a206a255dc885343efa293e1ee2672493e5e5
Reviewed-on: https://review.whamcloud.com/45332
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/obdclass/llog_internal.h
lustre/obdclass/llog_ioctl.c
lustre/obdclass/obd_config.c
lustre/tests/conf-sanity.sh
lustre/utils/lctl.c
lustre/utils/lustre_cfg.c
lustre/utils/obd.c

index 5be6058..55292ae 100644 (file)
@@ -80,7 +80,8 @@ int llog_cat_id2handle(const struct lu_env *env, struct llog_handle *cathandle,
 int class_config_dump_handler(const struct lu_env *env,
                              struct llog_handle *handle,
                              struct llog_rec_hdr *rec, void *data);
-int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size);
+int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size,
+                            unsigned int *cfg_flags, bool raw);
 int llog_process_or_fork(const struct lu_env *env,
                         struct llog_handle *loghandle,
                         llog_cb_t cb, void *data, void *catdata, bool fork);
index 2a0ee90..0802ac8 100644 (file)
@@ -201,10 +201,17 @@ static int llog_check_cb(const struct lu_env *env, struct llog_handle *handle,
        RETURN(rc);
 }
 
+struct llog_print_data {
+       struct obd_ioctl_data *lprd_data;
+       unsigned int           lprd_cfg_flags;
+       bool                   lprd_raw;
+};
+
 static int llog_print_cb(const struct lu_env *env, struct llog_handle *handle,
                         struct llog_rec_hdr *rec, void *data)
 {
-       struct obd_ioctl_data *ioc_data = data;
+       struct llog_print_data *lprd = data;
+       struct obd_ioctl_data *ioc_data = lprd->lprd_data;
        static int l, remains;
        static long from, to;
        static char *out;
@@ -252,7 +259,9 @@ static int llog_print_cb(const struct lu_env *env, struct llog_handle *handle,
        } else if (rec->lrh_type == OBD_CFG_REC) {
                int rc;
 
-               rc = class_config_yaml_output(rec, out, remains);
+               rc = class_config_yaml_output(rec, out, remains,
+                                             &lprd->lprd_cfg_flags,
+                                             lprd->lprd_raw);
                if (rc < 0)
                        RETURN(rc);
                l = rc;
@@ -388,14 +397,20 @@ int llog_ioctl(const struct lu_env *env, struct llog_ctxt *ctxt, int cmd,
                else if (rc)
                        GOTO(out_close, rc);
                break;
-       case OBD_IOC_LLOG_PRINT:
+       case OBD_IOC_LLOG_PRINT: {
+               struct llog_print_data lprd = {
+                       .lprd_data = data,
+                       .lprd_raw = data->ioc_u32_1,
+               };
+
                LASSERT(data->ioc_inllen1 > 0);
-               rc = llog_process(env, handle, llog_print_cb, data, NULL);
+               rc = llog_process(env, handle, llog_print_cb, &lprd, NULL);
                if (rc == -LLOG_EEMPTY)
                        rc = 0;
                else if (rc)
                        GOTO(out_close, rc);
                break;
+       }
        case OBD_IOC_LLOG_CANCEL: {
                struct llog_cookie cookie;
                struct llog_logid plain;
index ae052f2..113acc1 100644 (file)
@@ -2059,15 +2059,23 @@ EXPORT_SYMBOL(class_config_parse_llog);
  * - { index: 4, event: attach, device: lustrewt-clilov, type: lov,
  *     UUID: lustrewt-clilov_UUID }
  */
-int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
+int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size,
+                            unsigned int *cfg_flags, bool raw)
 {
        struct lustre_cfg *lcfg = (struct lustre_cfg *)(rec + 1);
        char *ptr = buf;
        char *end = buf + size;
        int rc = 0, i;
        struct lcfg_type_data *ldata;
+       int swab = 0;
 
        LASSERT(rec->lrh_type == OBD_CFG_REC);
+
+       if (lcfg->lcfg_version == __swab32(LUSTRE_CFG_VERSION)) {
+               lustre_swab_lustre_cfg(lcfg);
+               swab = 1;
+       }
+
        rc = lustre_cfg_sanity_check(lcfg, rec->lrh_len);
        if (rc < 0)
                return rc;
@@ -2076,7 +2084,28 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
        if (!ldata)
                return -ENOTTY;
 
-       if (lcfg->lcfg_command == LCFG_MARKER)
+       if (lcfg->lcfg_command == LCFG_MARKER) {
+               struct cfg_marker *marker = lustre_cfg_buf(lcfg, 1);
+
+               lustre_swab_cfg_marker(marker, swab,
+                                      LUSTRE_CFG_BUFLEN(lcfg, 1));
+               if (marker->cm_flags & CM_START) {
+                       *cfg_flags = CFG_F_MARKER;
+                       if (marker->cm_flags & CM_SKIP)
+                               *cfg_flags = CFG_F_SKIP;
+               } else if (marker->cm_flags & CM_END) {
+                       *cfg_flags = 0;
+               }
+               if (likely(!raw))
+                       return 0;
+       }
+
+       /* entries outside marker are skipped */
+       if (!(*cfg_flags & CFG_F_MARKER) && !raw)
+               return 0;
+
+       /* inside skipped marker */
+       if (*cfg_flags & CFG_F_SKIP && !raw)
                return 0;
 
        /* form YAML entity */
@@ -2138,6 +2167,31 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
                goto out_done;
        }
 
+       if (lcfg->lcfg_command == LCFG_MARKER) {
+               struct cfg_marker *marker = lustre_cfg_buf(lcfg, 1);
+
+               if (marker->cm_flags & CM_START) {
+                       *cfg_flags = CFG_F_MARKER;
+                       if (marker->cm_flags & CM_SKIP)
+                               *cfg_flags = CFG_F_SKIP;
+               } else if (marker->cm_flags & CM_END) {
+                       *cfg_flags = 0;
+               }
+               ptr += snprintf(ptr, end - ptr, ", flags: %#04x",
+                               marker->cm_flags);
+               ptr += snprintf(ptr, end - ptr, ", version: %d.%d.%d.%d",
+                               OBD_OCD_VERSION_MAJOR(marker->cm_vers),
+                               OBD_OCD_VERSION_MINOR(marker->cm_vers),
+                               OBD_OCD_VERSION_PATCH(marker->cm_vers),
+                               OBD_OCD_VERSION_FIX(marker->cm_vers));
+               ptr += snprintf(ptr, end - ptr, ", createtime: %lld",
+                               marker->cm_createtime);
+               ptr += snprintf(ptr, end - ptr, ", canceltime: %lld",
+                               marker->cm_canceltime);
+
+               goto out_done;
+       }
+
        for (i = 1; i < lcfg->lcfg_bufcount; i++) {
                if (LUSTRE_CFG_BUFLEN(lcfg, i) > 0) {
                        ptr += snprintf(ptr, end - ptr, ", %s: %s",
index 12f1155..8ae2476 100644 (file)
@@ -8892,22 +8892,32 @@ test_123ad() { # LU-11566
 run_test 123ad "llog_print shows all records"
 
 test_123ae() { # LU-11566
+       local max
+       local mgs_arg=""
+       local log
+       local id
+       local orig
+       local new
+       local rpcs
+
        remote_mgs_nodsh && skip "remote MGS with nodsh"
        [ -d $MOUNT/.lustre ] || setupall
 
-       local max=$($LCTL get_param -n osc.*-OST0000-*.max_dirty_mb | head -1)
-       local mgs_arg=""
-
+       max=$($LCTL get_param -n osc.*-OST0000-*.max_dirty_mb | head -1)
+       pgs=$($LCTL get_param -n osc.*-OST0000-*.max_pages_per_rpc | head -1)
        [[ $MGS_VERSION -gt $(version_code 2.13.54) ]] ||
                mgs_arg="--device MGS"
 
        if do_facet mgs "$LCTL help llog_cancel" 2>&1| grep -q -- --log_id; then
                # save one set_param -P record in case none exist
-               do_facet mgs $LCTL set_param -P osc.*.max_dirty_mb=$max
 
-               local log=params
-               local orig=$(do_facet mgs $LCTL $mgs_arg llog_print $log |
-                            tail -1 | awk '{ print $4 }' | tr -d , )
+               do_facet mgs $LCTL set_param -P osc.*.max_pages_per_rpc=$pgs
+               stack_trap "do_facet mgs $LCTL set_param -P -d \
+                               osc.*.max_pages_per_rpc"
+
+               log=params
+               orig=$(do_facet mgs $LCTL $mgs_arg llog_print $log |
+                       tail -1 | awk '{ print $4 }' | tr -d , )
                do_facet mgs $LCTL set_param -P osc.*.max_dirty_mb=$max
                do_facet mgs $LCTL $mgs_arg llog_print $log | tail -1 |
                        grep "parameter: osc.*.max_dirty_mb" ||
@@ -8915,9 +8925,8 @@ test_123ae() { # LU-11566
 
                # - { index: 71, event: set_param, device: general,
                #     param: osc.*.max_dirty_mb, value: 256 }
-               local id=$(do_facet mgs $LCTL $mgs_arg llog_print $log |
-                          tail -1 | awk '{ print $4 }' | tr -d , )
-
+               id=$(do_facet mgs $LCTL $mgs_arg llog_print $log |
+                    tail -1 | awk '{ print $4 }' | tr -d , )
                do_facet mgs $LCTL $mgs_arg llog_cancel $log --log_idx=$id
                local new=$(do_facet mgs $LCTL $mgs_arg llog_print $log |
                            tail -1 | awk '{ print $4 }' | tr -d , )
@@ -8928,18 +8937,25 @@ test_123ae() { # LU-11566
        # test old positional parameters for a while still
        if [ "$MGS_VERSION" -le $(version_code 3.1.53) ]; then
                log=$FSNAME-client
+
+               do_facet mgs $LCTL conf_param \
+                       $FSNAME-OST0000.osc.max_pages_per_rpc=$pgs
+               stack_trap "do_facet mgs $LCTL conf_param -d \
+                               $FSNAME-OST0000.osc.max_pages_per_rpc"
+
                orig=$(do_facet mgs $LCTL --device MGS llog_print $log |
                       tail -1 | awk '{ print $4 }' | tr -d , )
                do_facet mgs $LCTL conf_param $FSNAME-OST0000.osc.max_dirty_mb=$max
                do_facet mgs $LCTL --device MGS llog_print $log |
                        tail -1 | grep "parameter: osc.max_dirty_mb" ||
                        error "old conf_param wasn't stored in params log"
-
+               do_facet mgs $LCTL --device MGS llog_print $log
                # - { index: 71, event: conf_param, device: testfs-OST0000-osc,
                #     param: osc.max_dirty_mb=256 }
                id=$(do_facet mgs $LCTL --device MGS llog_print $log |
                     tail -1 | awk '{ print $4 }' | tr -d , )
                do_facet mgs $LCTL --device MGS llog_cancel $log $id
+               do_facet mgs $LCTL --device MGS llog_print $log
                new=$(do_facet mgs $LCTL --device MGS llog_print $log |
                      tail -1 | awk '{ print $4 }' | tr -d , )
                (( new == orig )) ||
@@ -8997,6 +9013,36 @@ test_123af() { #LU-13609
 }
 run_test 123af "llog_catlist can show all config files correctly"
 
+test_123ag() { # LU-15142
+       local rec
+       local orig_val
+
+       remote_mgs_nodsh && skip "remote MGS with nodsh"
+       (( $MGS_VERSION >= $(version_code 2.14.55) )) ||
+               skip "Need server version least 2.14.55"
+
+       [ -d $MOUNT/.lustre ] || setup
+
+       orig_val=$(do_facet mgs $LCTL get_param jobid_name)
+       stack_trap "do_facet mgs $LCTL set_param -P jobid_name=$orig_val"
+
+       do_facet mgs $LCTL set_param -P jobid_name="TESTNAME1"
+       do_facet mgs $LCTL set_param -P -d jobid_name
+       rec=$(do_facet mgs $LCTL --device MGS llog_print params |
+               grep -c jobid_name)
+       (( rec == 0 )) || error "parameter was not deleted, check #1"
+       do_facet mgs $LCTL set_param -P jobid_name="TESTNAME1"
+       rec=$(do_facet mgs $LCTL --device MGS llog_print params |
+               grep -c jobid_name)
+       (( rec == 1)) || error "parameter is not set"
+       # usage with ordinary set_param format works too
+       do_facet mgs $LCTL set_param -P -d jobid_name="ANY"
+       rec=$(do_facet mgs $LCTL --device MGS llog_print params |
+               grep -c jobid_name)
+       (( rec == 0 )) || error "parameter was not deleted, check #2"
+}
+run_test 123ag "llog_print skips values deleted by set_param -P -d"
+
 test_123F() {
        remote_mgs_nodsh && skip "remote MGS with nodsh"
 
index 8f4db50..600ccf5 100644 (file)
@@ -463,9 +463,10 @@ command_t cmdlist[] = {
         "print log header information.\n"
         "usage: llog_info <logname|FID>\n"},
        {"llog_print", jt_llog_print, 0,
-        "print log content information.\n"
+        "print all effective log records by default, or within given range.\n"
+        "With --raw option skipped records are printed as well.\n"
         "usage: llog_print <logname|FID> [--start <index>] [--end <index>j]\n"
-        "       print all records by default, or within given index range."},
+        "                  [--raw]\n"},
        {"llog_cancel", jt_llog_cancel, 0,
         "cancel one record in specified log.\n"
         "usage:llog_cancel <logname|FID> --log_idx <idx>\n"},
index 665e7f8..8394f85 100644 (file)
@@ -500,29 +500,33 @@ static int jt_lcfg_setparam_perm(int argc, char **argv,
        int i;
        int first_param;
        char *buf = NULL;
-       int len;
 
        first_param = optind;
        if (first_param < 0 || first_param >= argc)
                return CMD_HELP;
 
        for (i = first_param, rc = 0; i < argc; i++) {
-               len = strlen(argv[i]);
-
                buf = argv[i];
+               if (popt->po_delete) {
+                       char *end_pos;
+                       size_t len;
 
-               /* put an '=' on the end in case it doesn't have one */
-               if (popt->po_delete && argv[i][len - 1] != '=') {
-                       buf = malloc(len + 1);
-                       if (!buf) {
-                               rc = -ENOMEM;
-                               break;
+                       len = strlen(buf);
+                       /* Consider param ends at the first '=' in the buffer
+                        * and make sure it always ends with '=' as well
+                        */
+                       end_pos = memchr(buf, '=', len - 1);
+                       if (end_pos) {
+                               *(++end_pos) = '\0';
+                       } else if (buf[len - 1] != '=') {
+                               buf = malloc(len + 1);
+                               if (buf == NULL)
+                                       return -ENOMEM;
+                               sprintf(buf, "%s=", argv[i]);
                        }
-                       sprintf(buf, "%s=", argv[i]);
                }
 
                rc = lcfg_setparam_perm(argv[0], buf);
-
                if (buf != argv[i])
                        free(buf);
        }
@@ -677,21 +681,6 @@ display_name(const char *filename, struct stat *st, struct param_opts *popt)
        return param_name;
 }
 
-/* Find a character in a length limited string */
-/* BEWARE - kernel definition of strnchr has args in different order! */
-static char *strnchr(const char *p, char c, size_t n)
-{
-       if (!p)
-               return (0);
-
-       while (n-- > 0) {
-               if (*p == c)
-                       return ((char *)p);
-               p++;
-       }
-       return (0);
-}
-
 /**
  * Turns a lctl parameter string into a procfs/sysfs subdirectory path pattern.
  *
@@ -832,8 +821,8 @@ read_param(const char *path, const char *param_name, struct param_opts *popt)
        if (popt->po_show_path) {
                bool longbuf;
 
-               longbuf = strnchr(buf, buflen - 1, '\n') != NULL ||
-                       buflen + strlen(param_name) >= 80;
+               longbuf = memchr(buf, '\n', buflen - 1) ||
+                         buflen + strlen(param_name) >= 80;
                printf("%s=%s", param_name, longbuf ? "\n" : "");
        }
        printf("%s", buf);
index 5a4ee67..0cf763a 100644 (file)
@@ -2819,7 +2819,7 @@ out:
  */
 int jt_llog_print_iter(char *logname, long start, long end,
                       int (record_cb)(const char *record, void *private),
-                      void *private, bool reverse)
+                      void *private, bool reverse, bool raw)
 {
        struct obd_ioctl_data data = { 0 };
        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
@@ -2852,6 +2852,7 @@ retry:
                snprintf(endbuf, sizeof(endbuf), "%lu",
                         end < rec + inc - 1 ? end : rec + inc - 1);
 
+               data.ioc_u32_1 = raw ? 1 : 0;
                /* start and end record numbers are passed as ASCII digits */
                data.ioc_inlbuf2 = startbuf;
                data.ioc_inllen2 = strlen(startbuf) + 1;
@@ -2895,14 +2896,15 @@ out:
        return rc;
 }
 
-static int llog_parse_catalog_start_end(int *argc, char **argv[],
-                                       char **catalog, long *start, long *end)
+static int llog_parse_catalog_options(int *argc, char ***argv, char **catalog,
+                                     long *start, long *end, int *raw)
 {
        const struct option long_opts[] = {
        /* the --catalog option is not required, just for consistency */
        { .val = 'c',   .name = "catalog",      .has_arg = required_argument },
        { .val = 'e',   .name = "end",          .has_arg = required_argument },
        { .val = 'h',   .name = "help",         .has_arg = no_argument },
+       { .val = 'r',   .name = "raw",          .has_arg = no_argument },
        { .val = 's',   .name = "start",        .has_arg = required_argument },
        { .name = NULL } };
        char *cmd = (*argv)[0];
@@ -2913,7 +2915,7 @@ static int llog_parse_catalog_start_end(int *argc, char **argv[],
                return -EINVAL;
 
        /* now process command line arguments*/
-       while ((c = getopt_long(*argc, *argv, "c:e:hs:",
+       while ((c = getopt_long(*argc, *argv, "c:e:hrs:",
                                long_opts, NULL)) != -1) {
                switch (c) {
                case 'c':
@@ -2927,6 +2929,11 @@ static int llog_parse_catalog_start_end(int *argc, char **argv[],
                                return CMD_HELP;
                        }
                        break;
+               case 'r':
+                       if (!raw)
+                               return CMD_HELP;
+                       *raw = 1;
+                       break;
                case 's':
                        *start = strtol(optarg, &endp, 0);
                        if (*endp != '\0') {
@@ -3016,9 +3023,11 @@ int jt_llog_print(int argc, char **argv)
 {
        char *catalog = NULL;
        long start = 1, end = -1;
+       int raw = 0;
        int rc;
 
-       rc = llog_parse_catalog_start_end(&argc, &argv, &catalog, &start, &end);
+       rc = llog_parse_catalog_options(&argc, &argv, &catalog, &start, &end,
+                                       &raw);
        if (rc)
                return rc;
 
@@ -3026,7 +3035,7 @@ int jt_llog_print(int argc, char **argv)
                return CMD_INCOMPLETE;
 
        rc = jt_llog_print_iter(catalog, start, end, jt_llog_print_cb,
-                               NULL, false);
+                               NULL, false, !!raw);
 
        llog_default_device(LLOG_DFLT_DEV_RESET);
 
@@ -3041,7 +3050,7 @@ int jt_llog_print(int argc, char **argv)
  * The positional arguments option should eventually be phased out.
  */
 static int llog_parse_catalog_log_idx(int *argc, char ***argv, const char *opts,
-                                     int max_args, struct obd_ioctl_data *data)
+                                     struct obd_ioctl_data *data)
 {
        const struct option long_opts[] = {
        /* the --catalog option is not required, just for consistency */
@@ -3108,7 +3117,7 @@ int jt_llog_cancel(int argc, char **argv)
                return CMD_INCOMPLETE;
 
        /* Parse catalog file (in inlbuf1) and named parameters */
-       rc = llog_parse_catalog_log_idx(&argc, &argv, "c:hi:l:", 3, &data);
+       rc = llog_parse_catalog_log_idx(&argc, &argv, "c:hi:l:", &data);
 
        /*
         * Handle old positional parameters if not using named parameters,
@@ -3160,7 +3169,8 @@ int jt_llog_check(int argc, char **argv)
        char *cmd = argv[0];
        int rc;
 
-       rc = llog_parse_catalog_start_end(&argc, &argv, &catalog, &start, &end);
+       rc = llog_parse_catalog_options(&argc, &argv, &catalog, &start,
+                                       &end, NULL);
        if (rc)
                return rc;
 
@@ -3214,7 +3224,7 @@ int jt_llog_remove(int argc, char **argv)
        if (llog_default_device(LLOG_DFLT_MGS_SET))
                return CMD_INCOMPLETE;
 
-       rc = llog_parse_catalog_log_idx(&argc, &argv, "c:hl:", 2, &data);
+       rc = llog_parse_catalog_log_idx(&argc, &argv, "c:hl:", &data);
        if (rc)
                goto err;
 
@@ -3427,7 +3437,7 @@ static int llog_search_ost(char *logname, long last_index, char *ostname)
        for (end = last_index; end > 1; end -= inc) {
                start = end - inc > 0 ? end - inc : 1;
                rc = jt_llog_print_iter(logname, start, end, llog_search_ost_cb,
-                                       ostname, true);
+                                       ostname, true, false);
                if (rc)
                        break;
        }
@@ -3567,7 +3577,7 @@ static int llog_search_pool(char *logname, long last_index, char *fsname,
        for (end = last_index; end > 1; end -= inc) {
                start = end - inc > 0 ? end - inc : 1;
                rc = jt_llog_print_iter(logname, start, end,
-                                       llog_search_pool_cb, &lpd, true);
+                                       llog_search_pool_cb, &lpd, true, false);
                if (rc) {
                        if (rc == 1 && lpd.lpd_pool_exists)
                                rc = lpd.lpd_ost_num ? 1 : 0;
@@ -4969,7 +4979,8 @@ int llog_poollist(char *fsname, char *poolname)
                strncpy(lpld.lpld_poolname, poolname,
                        sizeof(lpld.lpld_poolname) - 1);
        snprintf(logname, sizeof(logname), "%s-client", fsname);
-       rc = jt_llog_print_iter(logname, 0, -1, llog_poollist_cb, &lpld, false);
+       rc = jt_llog_print_iter(logname, 0, -1, llog_poollist_cb, &lpld, false,
+                               false);
 
        if (poolname && poolname[0])
                printf("Pool: %s.%s\n", fsname, poolname);