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 <emoly@whamcloud.com>
Change-Id: Ie45127ac8b80a75eaeb7158559c690da52eef103
Reviewed-on: https://review.whamcloud.com/46951
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/obdclass/llog_internal.h b/lustre/obdclass/llog_internal.h
index 55292ae..096f595 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (executable)
--- 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 (file)
--- 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;