Whamcloud - gitweb
LU-11566 utils: fix lctl llog_print for large configs 15/33815/8
authorAndreas Dilger <adilger@whamcloud.com>
Mon, 10 Dec 2018 10:18:19 +0000 (03:18 -0700)
committerOleg Drokin <green@whamcloud.com>
Wed, 30 Jan 2019 02:40:35 +0000 (02:40 +0000)
If "lctl llog_print" is called for a large configuration, it will
overflow the 8KB buffer limit for OBD ioctl commands.  The kernel
snprintf calls try to overflow the supplied buffer.  Avoid that.
If the configuration is large, fetch the configuration records in
chunks and print them incrementally.

Add --start and --end options to llog_print and deprecate the use of
positional parameters, since positional parameters are increasingly
complex to parse as options are added, and are harder to use.

The callback for the configuration records will allow "lctl pool_*"
commands to be processed directly on the MGS.

Move existing llog_print test_60aa, test_60ab to conf-sanity as
test_123aa and test_123ab (rename set_param -F test_123 to test_123F).
Add new test_123ac and test_123ad for the new llog_print --start and
--end param, and update test_123aa to test old positional parameters.

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: Ib7d2ae893033bd4594646c980b7d0ddbd2b3a089
Reviewed-on: https://review.whamcloud.com/33815
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Reviewed-by: Ben Evans <bevans@cray.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/doc/lctl-llog_print.8
lustre/obdclass/obd_config.c
lustre/tests/conf-sanity.sh
lustre/tests/sanity.sh
lustre/utils/lctl.c
lustre/utils/obd.c

index 7230120..515036a 100644 (file)
@@ -3,9 +3,11 @@
 lctl llog_print \- print the content of a configuration log
 .SH SYNOPSIS
 .B lctl llog_print
 lctl llog_print \- print the content of a configuration log
 .SH SYNOPSIS
 .B lctl llog_print
+.RB [ --start
+.IR start_index ]
+.RB [ --end
+.IR end_index ]
 .RI < logname >
 .RI < logname >
-.RI [ start_index ]
-.RI [ end_index ]
 
 .SH DESCRIPTION
 .B lctl llog_print
 
 .SH DESCRIPTION
 .B lctl llog_print
@@ -18,6 +20,14 @@ saved via
 on the MGS.
 .SH OPTIONS
 .TP
 on the MGS.
 .SH OPTIONS
 .TP
+.B --end
+Stop printing records at
+.IR end_index .
+.TP
+.B --start
+Start printing records at
+.IR start_index .
+.TP
 .B logname
 The name of the configuration log, in the form
 .IR fsname - target ,
 .B logname
 The name of the configuration log, in the form
 .IR fsname - target ,
@@ -26,12 +36,13 @@ like
 or
 .BR lustrefs-MDT0000 .
 .TP
 or
 .BR lustrefs-MDT0000 .
 .TP
-.B start_index
+.I start_index
 The first record number in the config log to dump.  Note that deactivated
 records and comment records will not be printed.
 .TP
 The first record number in the config log to dump.  Note that deactivated
 records and comment records will not be printed.
 .TP
-.B end_index
-The last record number in the config log to dump.
+.I end_index
+The last record number in the config log to dump, including the specified
+index number.
 .SH EXAMPLES
 .TP
 To print all of the records from the testfs-client configuration log:
 .SH EXAMPLES
 .TP
 To print all of the records from the testfs-client configuration log:
index 869e64c..61625ff 100644 (file)
@@ -1944,11 +1944,11 @@ static struct lcfg_type_data *lcfg_cmd2data(__u32 cmd)
  */
 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)
 {
-       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;
+       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;
 
        LASSERT(rec->lrh_type == OBD_CFG_REC);
        rc = lustre_cfg_sanity_check(lcfg, rec->lrh_len);
 
        LASSERT(rec->lrh_type == OBD_CFG_REC);
        rc = lustre_cfg_sanity_check(lcfg, rec->lrh_len);
@@ -1965,24 +1965,37 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
        /* form YAML entity */
        ptr += snprintf(ptr, end - ptr, "- { index: %u, event: %s",
                        rec->lrh_index, ldata->ltd_name);
        /* form YAML entity */
        ptr += snprintf(ptr, end - ptr, "- { index: %u, event: %s",
                        rec->lrh_index, ldata->ltd_name);
+       if (end - ptr <= 0)
+               goto out_overflow;
 
 
-       if (lcfg->lcfg_flags)
+       if (lcfg->lcfg_flags) {
                ptr += snprintf(ptr, end - ptr, ", flags: %#08x",
                                lcfg->lcfg_flags);
                ptr += snprintf(ptr, end - ptr, ", flags: %#08x",
                                lcfg->lcfg_flags);
-       if (lcfg->lcfg_num)
+               if (end - ptr <= 0)
+                       goto out_overflow;
+       }
+       if (lcfg->lcfg_num) {
                ptr += snprintf(ptr, end - ptr, ", num: %#08x",
                                lcfg->lcfg_num);
                ptr += snprintf(ptr, end - ptr, ", num: %#08x",
                                lcfg->lcfg_num);
+               if (end - ptr <= 0)
+                       goto out_overflow;
+       }
        if (lcfg->lcfg_nid) {
                char nidstr[LNET_NIDSTR_SIZE];
 
                libcfs_nid2str_r(lcfg->lcfg_nid, nidstr, sizeof(nidstr));
                ptr += snprintf(ptr, end - ptr, ", nid: %s(%#llx)",
                                nidstr, lcfg->lcfg_nid);
        if (lcfg->lcfg_nid) {
                char nidstr[LNET_NIDSTR_SIZE];
 
                libcfs_nid2str_r(lcfg->lcfg_nid, nidstr, sizeof(nidstr));
                ptr += snprintf(ptr, end - ptr, ", nid: %s(%#llx)",
                                nidstr, lcfg->lcfg_nid);
+               if (end - ptr <= 0)
+                       goto out_overflow;
        }
 
        }
 
-       if (LUSTRE_CFG_BUFLEN(lcfg, 0) > 0)
+       if (LUSTRE_CFG_BUFLEN(lcfg, 0) > 0) {
                ptr += snprintf(ptr, end - ptr, ", device: %s",
                                lustre_cfg_string(lcfg, 0));
                ptr += snprintf(ptr, end - ptr, ", device: %s",
                                lustre_cfg_string(lcfg, 0));
+               if (end - ptr <= 0)
+                       goto out_overflow;
+       }
 
        if (lcfg->lcfg_command == LCFG_SET_PARAM) {
                /*
 
        if (lcfg->lcfg_command == LCFG_SET_PARAM) {
                /*
@@ -1995,7 +2008,7 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
                size_t len;
 
                if (tmp == NULL)
                size_t len;
 
                if (tmp == NULL)
-                       return -ENOTTY;
+                       goto out_done;
 
                ptr += snprintf(ptr, end - ptr, ", %s: ", ldata->ltd_bufs[0]);
                len = tmp - cfg_str + 1;
 
                ptr += snprintf(ptr, end - ptr, ", %s: ", ldata->ltd_bufs[0]);
                len = tmp - cfg_str + 1;
@@ -2009,16 +2022,25 @@ int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size)
        }
 
        for (i = 1; i < lcfg->lcfg_bufcount; i++) {
        }
 
        for (i = 1; i < lcfg->lcfg_bufcount; i++) {
-               if (LUSTRE_CFG_BUFLEN(lcfg, i) > 0)
+               if (LUSTRE_CFG_BUFLEN(lcfg, i) > 0) {
                        ptr += snprintf(ptr, end - ptr, ", %s: %s",
                                        ldata->ltd_bufs[i - 1],
                                        lustre_cfg_string(lcfg, i));
                        ptr += snprintf(ptr, end - ptr, ", %s: %s",
                                        ldata->ltd_bufs[i - 1],
                                        lustre_cfg_string(lcfg, i));
+                       if (end - ptr <= 0)
+                               goto out_overflow;
+               }
        }
 
 out_done:
        ptr += snprintf(ptr, end - ptr, " }\n");
        }
 
 out_done:
        ptr += snprintf(ptr, end - ptr, " }\n");
-       /* return consumed bytes */
+out_overflow:
+       /* Return consumed bytes.  If the buffer overflowed, zero last byte */
        rc = ptr - buf;
        rc = ptr - buf;
+       if (rc > size) {
+               rc = -EOVERFLOW;
+               *(end - 1) = '\0';
+       }
+
        return rc;
 }
 
        return rc;
 }
 
index 58cb879..d484155 100644 (file)
@@ -30,6 +30,7 @@ export MULTIOP=${MULTIOP:-multiop}
 . $LUSTRE/tests/test-framework.sh
 init_test_env $@
 . ${CONFIG:=$LUSTRE/tests/cfg/$NAME.sh}
 . $LUSTRE/tests/test-framework.sh
 init_test_env $@
 . ${CONFIG:=$LUSTRE/tests/cfg/$NAME.sh}
+get_lustre_env
 
 # use small MDS + OST size to speed formatting time
 # do not use too small MDSSIZE/OSTSIZE, which affect the default journal size
 
 # use small MDS + OST size to speed formatting time
 # do not use too small MDSSIZE/OSTSIZE, which affect the default journal size
@@ -8452,12 +8453,104 @@ test_122() {
 }
 run_test 122 "Check OST sequence update"
 
 }
 run_test 122 "Check OST sequence update"
 
-test_123() {
+test_123aa() {
+       remote_mgs_nodsh && skip "remote MGS with nodsh"
+       [ -d $MOUNT/.lustre ] || setupall
+
+       # test old logid format until removal from llog_ioctl.c::str2logid()
+       if [ $MGS_VERSION -lt $(version_code 3.1.53) ]; then
+               do_facet mgs $LCTL dl | grep MGS
+               do_facet mgs "$LCTL --device %MGS llog_print \
+                             \\\\\\\$$FSNAME-client 1 10" ||
+                       error "old llog_print failed"
+       fi
+
+       # test new logid format
+       if [ $MGS_VERSION -ge $(version_code 2.9.53) ]; then
+               do_facet mgs "$LCTL --device MGS llog_print $FSNAME-client" ||
+                       error "new llog_print failed"
+       fi
+}
+run_test 123aa "llog_print works with FIDs and simple names"
+
+test_123ab() {
+       remote_mgs_nodsh && skip "remote MGS with nodsh"
+       [[ $MGS_VERSION -gt $(version_code 2.11.51) ]] ||
+               skip "Need server with working llog_print support"
+
+       [ -d $MOUNT/.lustre ] || setupall
+
+       local yaml
+       local orig_val
+
+       orig_val=$(do_facet mgs $LCTL get_param jobid_name)
+       do_facet mgs $LCTL set_param -P jobid_name="testname"
+
+       yaml=$(do_facet mgs $LCTL --device MGS llog_print params |
+              grep jobid_name | tail -n 1)
+
+       local param=$(awk '{ print $10 }' <<< "$yaml")
+       local val=$(awk '{ print $12 }' <<< "$yaml")
+       #return to the default
+       do_facet mgs $LCTL set_param -P jobid_name=$orig_val
+       [ $val = "testname" ] || error "bad value: $val"
+       [ $param = "jobid_name," ] || error "Bad param: $param"
+}
+run_test 123ab "llog_print params output values from set_param -P"
+
+test_123ac() { # LU-11566
+       remote_mgs_nodsh && skip "remote MGS with nodsh"
+       do_facet mgs "$LCTL help llog_print" 2>&1 | grep -q -- --start ||
+               skip "Need 'lctl llog_print --start' on MGS"
+
+       local start=10
+       local end=50
+
+       [ -d $MOUNT/.lustre ] || setupall
+
+       # - { index: 10, event: add_uuid, nid: 192.168.20.1@tcp(0x20000c0a81401,
+       #     node: 192.168.20.1@tcp }
+       do_facet mgs $LCTL --device MGS \
+               llog_print --start $start --end $end $FSNAME-client | tr -d , |
+               while read DASH BRACE INDEX idx EVENT BLAH BLAH BLAH; do
+               (( idx >= start )) || error "llog_print index $idx < $start"
+               (( idx <= end )) || error "llog_print index $idx > $end"
+       done
+}
+run_test 123ac "llog_print with --start and --end"
+
+test_123ad() { # LU-11566
+       remote_mgs_nodsh && skip "remote MGS with nodsh"
+       # older versions of lctl may not print all records properly
+       do_facet mgs "$LCTL help llog_print" 2>&1 | grep -q -- --start ||
+               skip "Need 'lctl llog_print --start' on MGS"
+
+       [ -d $MOUNT/.lustre ] || setupall
+
+       # append a new record, to avoid issues if last record was cancelled
+       local old=$($LCTL get_param -n osc.*-OST0000-*.max_dirty_mb | head -1)
+       do_facet mgs $LCTL conf_param $FSNAME-OST0000.osc.max_dirty_mb=$old
+
+       # logid:            [0x3:0xa:0x0]:0
+       # flags:            4 (plain)
+       # records_count:    72
+       # last_index:       72
+       local num=$(do_facet mgs $LCTL --device MGS llog_info $FSNAME-client |
+                   awk '/last_index:/ { print $2 - 1 }')
+
+       # - { index: 71, event: set_timeout, num: 0x14, param: sys.timeout=20 }
+       local last=$(do_facet mgs $LCTL --device MGS llog_print $FSNAME-client |
+                    tail -1 | awk '{ print $4 }' | tr -d , )
+       (( last == num )) || error "llog_print only showed $last/$num records"
+}
+run_test 123ad "llog_print shows all records"
+
+test_123F() {
        setupall
        local yaml_file="$TMP/$tfile.yaml"
        do_facet mgs rm "$yaml_file"
        setupall
        local yaml_file="$TMP/$tfile.yaml"
        do_facet mgs rm "$yaml_file"
-       local cfgfiles=$(do_facet mgs "lctl --device MGS llog_catlist |"\
-                       " sed 's/config_log://'")
+       local cfgfiles=$(do_facet mgs "lctl --device MGS llog_catlist" |
+                        sed 's/config_log://')
 
        # set jobid_var to a different value for test
        local orig_val=$(do_facet mgs $LCTL get_param jobid_var)
 
        # set jobid_var to a different value for test
        local orig_val=$(do_facet mgs $LCTL get_param jobid_var)
@@ -8489,7 +8582,7 @@ test_123() {
 
        do_facet mgs rm "$yaml_file"
 }
 
        do_facet mgs rm "$yaml_file"
 }
-run_test 123 "clear and reset all parameters using set_param -F"
+run_test 123F "clear and reset all parameters using set_param -F"
 
 test_124()
 {
 
 test_124()
 {
index 8e23777..da48ab8 100755 (executable)
@@ -6242,48 +6242,6 @@ test_60a() {
 }
 run_test 60a "llog_test run from kernel module and test llog_reader"
 
 }
 run_test 60a "llog_test run from kernel module and test llog_reader"
 
-test_60aa() {
-       remote_mgs_nodsh && skip "remote MGS with nodsh"
-
-       # test old logid format
-       if [ $MGS_VERSION -le $(version_code 3.1.53) ]; then
-               do_facet mgs $LCTL dl | grep MGS
-               do_facet mgs "$LCTL --device %MGS llog_print \\\\\\\$$FSNAME-client" ||
-                       error "old llog_print failed"
-       fi
-
-       # test new logid format
-       if [ $MGS_VERSION -ge $(version_code 2.9.53) ]; then
-               do_facet mgs "$LCTL --device MGS llog_print $FSNAME-client" ||
-                       error "new llog_print failed"
-       fi
-}
-run_test 60aa "llog_print works with FIDs and simple names"
-
-test_60ab() {
-       # test llog_print with params
-
-       [[ $MDS1_VERSION -gt $(version_code 2.11.51) ]] ||
-               skip "Need server version greater than 2.11.51"
-
-       local yaml
-       local orig_val
-
-       orig_val=$(do_facet mgs $LCTL get_param jobid_name)
-       do_facet mgs $LCTL set_param -P jobid_name="testname"
-
-       yaml=$(do_facet mgs $LCTL --device MGS llog_print params |
-           grep jobid_name | tail -n 1)
-
-       local param=`awk '{ print $10 }' <<< "$yaml"`
-       local val=`awk '{ print $12 }' <<< "$yaml"`
-       #return to the default
-       do_facet mgs $LCTL set_param -P jobid_name=$orig_val
-       [ $val = "testname" ] || error "bad value: $val"
-       [ $param = "jobid_name," ] || error "Bad param: $param"
-}
-run_test 60ab "llog_print params output values from set_param -P"
-
 test_60b() { # bug 6411
        [ $PARALLEL == "yes" ] && skip "skip parallel run"
 
 test_60b() { # bug 6411
        [ $PARALLEL == "yes" ] && skip "skip parallel run"
 
index 4ed25e8..65b2742 100644 (file)
@@ -505,8 +505,8 @@ command_t cmdlist[] = {
         "       oid, ogr and ogen are hexadecimal."},
        {"llog_print", jt_llog_print, 0,
         "print log content information.\n"
         "       oid, ogr and ogen are hexadecimal."},
        {"llog_print", jt_llog_print, 0,
         "print log content information.\n"
-        "usage: llog_print <logname|[FID]> [start_index [end_index]]\n"
-        "       print all records from index 1 by default."},
+        "usage: llog_print <logname|FID> [--start <index>] [--end <index>j]\n"
+        "       print all records by default, or within given index range."},
        {"llog_check", jt_llog_check, 0,
         "print log content information.\n"
         "usage: llog_check <logname|[FID]> [start_index] [end_index]\n"
        {"llog_check", jt_llog_check, 0,
         "print log content information.\n"
         "usage: llog_check <logname|[FID]> [start_index] [end_index]\n"
index d22a053..8435d2f 100644 (file)
@@ -2654,89 +2654,269 @@ int jt_llog_info(int argc, char **argv)
         return rc;
 }
 
         return rc;
 }
 
-int jt_llog_print(int argc, char **argv)
+int jt_llog_print_cb(const char *record, void *private)
 {
 {
-        struct obd_ioctl_data data;
-        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
-        int rc;
+       printf("%s\n", record);
 
 
-        if (argc != 2 && argc != 4)
-                return CMD_HELP;
+       return 0;
+}
 
 
-        memset(&data, 0, sizeof(data));
-        data.ioc_dev = cur_device;
-        data.ioc_inllen1 = strlen(argv[1]) + 1;
-        data.ioc_inlbuf1 = argv[1];
-        if (argc == 4) {
-                data.ioc_inllen2 = strlen(argv[2]) + 1;
-                data.ioc_inlbuf2 = argv[2];
-                data.ioc_inllen3 = strlen(argv[3]) + 1;
-                data.ioc_inlbuf3 = argv[3];
-        } else {
-                char from[2] = "1", to[3] = "-1";
-                data.ioc_inllen2 = strlen(from) + 1;
-                data.ioc_inlbuf2 = from;
-                data.ioc_inllen3 = strlen(to) + 1;
-                data.ioc_inlbuf3 = to;
-        }
-       data.ioc_inllen4 = sizeof(rawbuf) - __ALIGN_KERNEL(sizeof(data), 8) -
-                          __ALIGN_KERNEL(data.ioc_inllen1, 8) -
-                          __ALIGN_KERNEL(data.ioc_inllen2, 8) -
-                          __ALIGN_KERNEL(data.ioc_inllen3, 8);
-        memset(buf, 0, sizeof(rawbuf));
-       rc = llapi_ioctl_pack(&data, &buf, sizeof(rawbuf));
-        if (rc) {
-                fprintf(stderr, "error: %s: invalid ioctl\n",
-                        jt_cmdname(argv[0]));
-                return rc;
-        }
+/**
+ * Iterate over llog records, typically YAML-formatted configuration logs
+ *
+ * \param logname[in]  name of llog file or FID
+ * \param start[in]    first record to process
+ * \param end[in]      last record to process (inclusive)
+ * \param cb[in]       callback for records. Return -ve error, or +ve abort.
+ * \param private[in,out] private data passed to the \a record_cb function
+ */
+int jt_llog_print_iter(char *logname, long start, long end,
+                      int (record_cb)(const char *record, void *private),
+                      void *private)
+{
+       struct obd_ioctl_data data = { 0 };
+       char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
+       char startbuf[16], endbuf[16];
+       static long inc = sizeof(rawbuf) / 128;
+       long rec;
+       int rc = 0;
 
 
-        rc = l_ioctl(OBD_DEV_ID, OBD_IOC_LLOG_PRINT, buf);
-        if (rc == 0)
-                fprintf(stdout, "%s", ((struct obd_ioctl_data*)buf)->ioc_bulk);
-        else
-                fprintf(stderr, "OBD_IOC_LLOG_PRINT failed: %s\n",
-                        strerror(errno));
+       if (end == -1)
+               end = 0x7fffffff;
 
 
-        return rc;
+       data.ioc_dev = cur_device;
+       data.ioc_inlbuf1 = logname;
+       data.ioc_inllen1 = strlen(logname) + 1;
+
+       /*
+        * Estimate about 128 characters per configuration record.  Not all
+        * records will be printed in any case, so they should easily fit.  If
+        * not, the kernel will return -EOVERFLOW and ask for fewer records.
+        *
+        * We don't want to request records from the kernel one-at-a-time, as
+        * it restarts the config llog iteration from the beginning, so we
+        * fetch multiple records from the kernel per call and split locally.
+        */
+       for (rec = start; rec < end; rec += inc) {
+               char *record = ((struct obd_ioctl_data *)buf)->ioc_bulk;
+               char *ptr;
+
+retry:
+               snprintf(startbuf, sizeof(startbuf), "%lu", rec);
+               snprintf(endbuf, sizeof(endbuf), "%lu",
+                        end < rec + inc - 1 ? end : rec + inc - 1);
+
+               /* start and end record numbers are passed as ASCII digits */
+               data.ioc_inlbuf2 = startbuf;
+               data.ioc_inllen2 = strlen(startbuf) + 1;
+               data.ioc_inlbuf3 = endbuf;
+               data.ioc_inllen3 = strlen(endbuf) + 1;
+
+               data.ioc_inllen4 = sizeof(rawbuf) -
+                       __ALIGN_KERNEL(sizeof(data), 8) -
+                       __ALIGN_KERNEL(data.ioc_inllen1, 8) -
+                       __ALIGN_KERNEL(data.ioc_inllen2, 8) -
+                       __ALIGN_KERNEL(data.ioc_inllen3, 8);
+               memset(buf, 0, sizeof(rawbuf));
+               rc = llapi_ioctl_pack(&data, &buf, sizeof(rawbuf));
+               if (rc) {
+                       fprintf(stderr, "%s: invalid ioctl data\n", logname);
+                       goto out;
+               }
+
+               rc = l_ioctl(OBD_DEV_ID, OBD_IOC_LLOG_PRINT, buf);
+               if (rc == -EOVERFLOW && inc > 2) {
+                       inc /= 2;
+                       goto retry;
+               }
+               if (rc) {
+                       fprintf(stderr, "%s: OBD_IOC_LLOG_PRINT failed: %s\n",
+                               logname, strerror(errno));
+                       rc = -errno;
+                       goto out;
+               }
+
+               /* There is no "end of list" marker, record was not modified */
+               if (strcmp(record, logname) == 0)
+                       break;
+
+               do {
+                       ptr = strchr(record, '\n');
+                       if (ptr)
+                               *ptr = '\0';
+                       rc = record_cb(record, private);
+                       if (rc) {
+                               if (rc > 0)
+                                       rc = 0;
+                               goto out;
+                       }
+
+                       if (ptr)
+                               record = ptr + 1;
+               } while (ptr && *(ptr + 1));
+       }
+
+out:
+       return rc;
+}
+
+static int llog_parse_catalog_start_end(int *argc, char **argv[],
+                                       char **catalog, long *start, long *end)
+{
+       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 = 's',   .name = "start",        .has_arg = required_argument },
+       { .name = NULL } };
+       char *cmd = (*argv)[0];
+       char *endp;
+       int c;
+
+       if (catalog == NULL || start == NULL || end == NULL)
+               return -EINVAL;
+
+       /* now process command line arguments*/
+       while ((c = getopt_long(*argc, *argv, "c:e:hs:",
+                               long_opts, NULL)) != -1) {
+               switch (c) {
+               case 'c':
+                       *catalog = optarg;
+                       break;
+               case 'e':
+                       *end = strtol(optarg, &endp, 0);
+                       if (*endp != '\0') {
+                               fprintf(stderr, "%s: bad end value '%s'\n",
+                                       cmd, optarg);
+                               return CMD_HELP;
+                       }
+                       break;
+               case 's':
+                       *start = strtol(optarg, &endp, 0);
+                       if (*endp != '\0') {
+                               fprintf(stderr, "%s: bad start value '%s'\n",
+                                       cmd, optarg);
+                               return CMD_HELP;
+                       }
+                       break;
+               case 'h':
+               default:
+                       return CMD_HELP;
+               }
+       }
+       *argc -= optind;
+       *argv += optind;
+
+       /* support old optional positional parameters only if they were
+        * not already specified with named arguments: logname [start [end]]
+        */
+       if (*argc >= 1) {
+               if (*catalog) {
+                       fprintf(stderr,
+                               "%s: catalog is set, unknown argument '%s'\n",
+                               cmd, (*argv)[0]);
+                       return CMD_HELP;
+               }
+               *catalog = (*argv)[0];
+               (*argc)--;
+               (*argv)++;
+       }
+
+       if (*argc >= 1) {
+               if (*start != 1) {
+                       fprintf(stderr,
+                               "%s: --start is set, unknown argument '%s'\n",
+                               cmd, (*argv)[0]);
+                       return CMD_HELP;
+               }
+
+               *start = strtol((*argv)[0], &endp, 0);
+               if (*endp != '\0') {
+                       fprintf(stderr, "%s: bad start value '%s'\n",
+                               cmd, (*argv)[0]);
+                       return CMD_HELP;
+               }
+               (*argc)--;
+               (*argv)++;
+       }
+       if (*argc >= 1) {
+               if (*end != -1) {
+                       fprintf(stderr,
+                               "%s: --end is set, unknown argument '%s'\n",
+                               cmd, (*argv)[0]);
+                       return CMD_HELP;
+               }
+
+               *end = strtol((*argv)[0], &endp, 0);
+               if (*endp != '\0') {
+                       fprintf(stderr, "%s: bad end value '%s'\n",
+                               cmd, (*argv)[0]);
+                       return CMD_HELP;
+               }
+               (*argc)--;
+               (*argv)++;
+       }
+       if (*argc > 1) {
+               fprintf(stderr, "%s: unknown argument '%s'\n", cmd, (*argv)[0]);
+               return CMD_HELP;
+       }
+
+       if (*end != -1 && *end < *start) {
+               fprintf(stderr, "%s: end '%lu' less than than start '%lu'\n",
+                       cmd, *end, *start);
+               return CMD_HELP;
+       }
+
+       return 0;
+}
+
+int jt_llog_print(int argc, char **argv)
+{
+       char *catalog = NULL;
+       long start = 1, end = -1;
+       int rc;
+
+       rc = llog_parse_catalog_start_end(&argc, &argv, &catalog, &start, &end);
+       if (rc)
+               return rc;
+
+       rc = jt_llog_print_iter(catalog, start, end, jt_llog_print_cb, NULL);
+
+       return rc;
 }
 
 static int llog_cancel_parse_optional(int argc, char **argv,
                                      struct obd_ioctl_data *data)
 {
 }
 
 static int llog_cancel_parse_optional(int argc, char **argv,
                                      struct obd_ioctl_data *data)
 {
-       int cOpt;
-       const char *const short_opts = "c:l:i:h";
        const struct option long_opts[] = {
        { .val = 'c',   .name = "catalog",      .has_arg = required_argument },
        { .val = 'h',   .name = "help",         .has_arg = no_argument },
        { .val = 'i',   .name = "log_idx",      .has_arg = required_argument },
        { .val = 'l',   .name = "log_id",       .has_arg = required_argument },
        { .name = NULL } };
        const struct option long_opts[] = {
        { .val = 'c',   .name = "catalog",      .has_arg = required_argument },
        { .val = 'h',   .name = "help",         .has_arg = no_argument },
        { .val = 'i',   .name = "log_idx",      .has_arg = required_argument },
        { .val = 'l',   .name = "log_id",       .has_arg = required_argument },
        { .name = NULL } };
+       int c;
 
        /* sanity check */
 
        /* sanity check */
-       if (!data || argc <= 1) {
+       if (!data || argc <= 1)
                return -1;
                return -1;
-       }
 
 
-       /*now process command line arguments*/
-       while ((cOpt = getopt_long(argc, argv, short_opts,
-                                       long_opts, NULL)) != -1) {
-               switch (cOpt) {
+       /* now process command line arguments*/
+       while ((c = getopt_long(argc, argv, "c:hi:l:",
+                               long_opts, NULL)) != -1) {
+               switch (c) {
                case 'c':
                        data->ioc_inllen1 = strlen(optarg) + 1;
                        data->ioc_inlbuf1 = optarg;
                        break;
                case 'c':
                        data->ioc_inllen1 = strlen(optarg) + 1;
                        data->ioc_inlbuf1 = optarg;
                        break;
-
                case 'l':
                        data->ioc_inllen2 = strlen(optarg) + 1;
                        data->ioc_inlbuf2 = optarg;
                        break;
                case 'l':
                        data->ioc_inllen2 = strlen(optarg) + 1;
                        data->ioc_inlbuf2 = optarg;
                        break;
-
                case 'i':
                        data->ioc_inllen3 = strlen(optarg) + 1;
                        data->ioc_inlbuf3 = optarg;
                        break;
                case 'i':
                        data->ioc_inllen3 = strlen(optarg) + 1;
                        data->ioc_inlbuf3 = optarg;
                        break;
-
                case 'h':
                default:
                        return -1;
                case 'h':
                default:
                        return -1;
@@ -2789,7 +2969,7 @@ int jt_llog_cancel(int argc, char **argv)
                        data.ioc_inlbuf3 = argv[2];
                }
        } else {
                        data.ioc_inlbuf3 = argv[2];
                }
        } else {
-               if (llog_cancel_parse_optional(argc, argv, &data) != 0)
+               if (llog_cancel_parse_optional(argc, argv, &data))
                        return CMD_HELP;
        }
 
                        return CMD_HELP;
        }