From 2a5b50d207173ca1ac71be8dfc39f98a2773bc3a Mon Sep 17 00:00:00 2001 From: Mikhail Pershin Date: Thu, 14 Oct 2021 17:16:21 +0300 Subject: [PATCH] LU-15142 lctl: fixes for set_param -P and llog_print - 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 Change-Id: Id93a206a255dc885343efa293e1ee2672493e5e5 Reviewed-on: https://review.whamcloud.com/45332 Reviewed-by: John L. Hammond Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/obdclass/llog_internal.h | 3 +- lustre/obdclass/llog_ioctl.c | 23 +++++++++++--- lustre/obdclass/obd_config.c | 58 +++++++++++++++++++++++++++++++++-- lustre/tests/conf-sanity.sh | 68 ++++++++++++++++++++++++++++++++++------- lustre/utils/lctl.c | 5 +-- lustre/utils/lustre_cfg.c | 45 +++++++++++---------------- lustre/utils/obd.c | 37 ++++++++++++++-------- 7 files changed, 178 insertions(+), 61 deletions(-) diff --git a/lustre/obdclass/llog_internal.h b/lustre/obdclass/llog_internal.h index 5be6058..55292ae 100644 --- a/lustre/obdclass/llog_internal.h +++ b/lustre/obdclass/llog_internal.h @@ -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); diff --git a/lustre/obdclass/llog_ioctl.c b/lustre/obdclass/llog_ioctl.c index 2a0ee90..0802ac8 100644 --- a/lustre/obdclass/llog_ioctl.c +++ b/lustre/obdclass/llog_ioctl.c @@ -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; diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index ae052f2..113acc1 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -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", diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index 12f1155..8ae2476 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -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" diff --git a/lustre/utils/lctl.c b/lustre/utils/lctl.c index 8f4db50..600ccf5 100644 --- a/lustre/utils/lctl.c +++ b/lustre/utils/lctl.c @@ -463,9 +463,10 @@ command_t cmdlist[] = { "print log header information.\n" "usage: llog_info \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 [--start ] [--end 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 --log_idx \n"}, diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c index 665e7f8..8394f85 100644 --- a/lustre/utils/lustre_cfg.c +++ b/lustre/utils/lustre_cfg.c @@ -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); diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index 5a4ee67..0cf763a 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -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); -- 1.8.3.1