From 474f2670d63b66a77ee3acb72b18bc7b5afbec84 Mon Sep 17 00:00:00 2001 From: Emoly Liu Date: Mon, 25 Apr 2022 10:56:55 +0800 Subject: [PATCH] LU-15706 llog: deal with "SKIP" pool llog records correctly If the requested "start" llog records by one ioctl are just the useful ones "protected" by one marker, they will don't know their cfg_flags(e.g. "SKIP" or not), and then will be mis-labeled in class_config_yaml_output(). To fix this issue, this patch does the following changes: - In kernel space, remember the marker cfg_flags for 10 records earlier than the requested "start" in function llog_print_cb(), so that the "start" can be output with its correct cfg_flags. - In user space, since the pool_new/add record will be marked as "SKIP" if its corresponding pool_destroy/remove record is logged later in mgs_pool_cmd(), these "SKIP" records won't be printed by function jt_llog_print_iter(), so lpd_ost_num doesn't need to be decreased in callback function llog_search_pool_cb() as well, otherwise, it will cause "lctl pool_destroy" error. Test-Parameters: standalonemgs=true testlist=ost-pools Test-Parameters: standalonemgs=true testlist=conf-sanity env=ONLY=123,HONOR_EXCEPT=y Fixes: 2a5b50d20717 ("LU-15142 lctl: fixes for set_param -P and llog_print") Signed-off-by: Emoly Liu Change-Id: Ie45127ac8b80a75eaeb7158559c690da52eef103 Reviewed-on: https://review.whamcloud.com/46951 Reviewed-by: Andreas Dilger Tested-by: jenkins Reviewed-by: Mike Pershin Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: Oleg Drokin --- lustre/obdclass/llog_internal.h | 2 ++ lustre/obdclass/llog_ioctl.c | 9 +++++++ lustre/obdclass/obd_config.c | 55 ++++++++++++++++++++++------------------- lustre/tests/ost-pools.sh | 3 +++ lustre/utils/obd.c | 8 ++++-- 5 files changed, 49 insertions(+), 28 deletions(-) diff --git a/lustre/obdclass/llog_internal.h b/lustre/obdclass/llog_internal.h index 55292ae..096f595 100644 --- a/lustre/obdclass/llog_internal.h +++ b/lustre/obdclass/llog_internal.h @@ -77,6 +77,8 @@ struct llog_handle *llog_handle_get(struct llog_handle *loghandle); int llog_handle_put(const struct lu_env *env, struct llog_handle *loghandle); int llog_cat_id2handle(const struct lu_env *env, struct llog_handle *cathandle, struct llog_handle **res, struct llog_logid *logid); +void llog_get_marker_cfg_flags(struct llog_rec_hdr *rec, + unsigned int *cfg_flags); int class_config_dump_handler(const struct lu_env *env, struct llog_handle *handle, struct llog_rec_hdr *rec, void *data); diff --git a/lustre/obdclass/llog_ioctl.c b/lustre/obdclass/llog_ioctl.c index 0802ac8..62a5b88 100644 --- a/lustre/obdclass/llog_ioctl.c +++ b/lustre/obdclass/llog_ioctl.c @@ -207,6 +207,7 @@ struct llog_print_data { bool lprd_raw; }; +#define MARKER_DIFF 10 static int llog_print_cb(const struct lu_env *env, struct llog_handle *handle, struct llog_rec_hdr *rec, void *data) { @@ -239,6 +240,14 @@ static int llog_print_cb(const struct lu_env *env, struct llog_handle *handle, } cur_index = rec->lrh_index; + if (from > MARKER_DIFF && cur_index >= from - MARKER_DIFF && + cur_index < from) { + /* LU-15706: try to remember the marker cfg_flag that the "from" + * is using, in case that the "from" record doesn't know its + * "SKIP" or not flag. + */ + llog_get_marker_cfg_flags(rec, &lprd->lprd_cfg_flags); + } if (cur_index < from) RETURN(0); if (to > 0 && cur_index > to) diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index 07e482e..8b2fcdc 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -2051,6 +2051,29 @@ parse_out: EXPORT_SYMBOL(class_config_parse_llog); /** + * Get marker cfg_flag + */ +void llog_get_marker_cfg_flags(struct llog_rec_hdr *rec, + unsigned int *cfg_flags) +{ + struct lustre_cfg *lcfg = (struct lustre_cfg *)(rec + 1); + struct cfg_marker *marker; + + if (lcfg->lcfg_command == LCFG_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; + } + CDEBUG(D_INFO, "index=%d, cm_flags=%#08x cfg_flags=%#08x\n", + rec->lrh_index, marker->cm_flags, *cfg_flags); + } +} + +/** * Parse config record and output dump in supplied buffer. * * This is separated from class_config_dump_handler() to use @@ -2085,28 +2108,14 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size, if (!ldata) return -ENOTTY; - 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; - } - + llog_get_marker_cfg_flags(rec, cfg_flags); + if ((lcfg->lcfg_command == LCFG_MARKER) && 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) + if ((*cfg_flags & CFG_F_SKIP) && !raw) return 0; /* form YAML entity */ @@ -2169,15 +2178,9 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size, } if (lcfg->lcfg_command == LCFG_MARKER) { - struct cfg_marker *marker = lustre_cfg_buf(lcfg, 1); + struct cfg_marker *marker; - 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; - } + marker = lustre_cfg_buf(lcfg, 1); ptr += snprintf(ptr, end - ptr, ", flags: %#04x", marker->cm_flags); ptr += snprintf(ptr, end - ptr, ", version: %d.%d.%d.%d", diff --git a/lustre/tests/ost-pools.sh b/lustre/tests/ost-pools.sh index 6def5e4..fb83be6 100755 --- a/lustre/tests/ost-pools.sh +++ b/lustre/tests/ost-pools.sh @@ -533,6 +533,9 @@ sub_test_5() { [[ $? -eq 0 ]] || error "pool_list $FSNAME failed." do_facet mgs lctl pool_add $FSNAME.$POOL $TGT_ALL + wait_update_facet $SINGLEMDS "lctl pool_list $FSNAME.$POOL | + wc -l" "$((OSTCOUNT + 1))" || + error "MDS: pool_list $FSNAME.$POOL failed" $LCMD pool_list $FSNAME.$POOL [[ $? -eq 0 ]] || error "pool_list $FSNAME.$POOL failed." diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index ce86ae1..3f71f03 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -3557,10 +3557,14 @@ static int llog_search_pool_cb(const char *record, void *data) if (lpd->lpd_cmd_type == LCFG_POOL_NEW || lpd->lpd_cmd_type == LCFG_POOL_DEL) { + /* In function mgs_pool_cmd(), a pool's pool_new/add + * record will be marked as "SKIP" if its pool_destroy/ + * remove record is logged later. That means the "SKIP" + * record won't be printed here and thus lpd_ost_num + * doesn't need to be decreased as well. + */ if (strstr(record, add_pool)) lpd->lpd_ost_num++; - if (strstr(record, rem_pool)) - lpd->lpd_ost_num--; } else if (lpd->lpd_ostname && lpd->lpd_ostname[0]) { if (strstr(record, lpd->lpd_ostname)) { lpd->lpd_pool_exists = true; -- 1.8.3.1