Whamcloud - gitweb
LU-2149 llog: lctl to work with llogs from command line 54/4254/7
authorAmir Shehata <amir.shehata@intel.com>
Wed, 18 Sep 2013 16:07:56 +0000 (09:07 -0700)
committerOleg Drokin <oleg.drokin@intel.com>
Thu, 14 Nov 2013 02:50:31 +0000 (02:50 +0000)
- mgs_list_logs() is introduced to get all of configs and
  mgs_iocontrol handles that and also cancel/remove ioctl
- remove obsoleted code from mdc_iocontrol
- add handler to list llog catalogs from mdt_iocontrol too
- lctl llog_print outputs records in YAML format

Signed-off-by: Amir Shehata <amir.shehata@intel.com>
Change-Id: Ic6e392403da18a2d7578aafdddf01007c3b3c30e
Reviewed-on: http://review.whamcloud.com/4254
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
lustre/include/lustre_log.h
lustre/mdc/mdc_request.c
lustre/mdt/mdt_handler.c
lustre/mgs/mgs_handler.c
lustre/mgs/mgs_llog.c
lustre/obdclass/llog_internal.h
lustre/obdclass/llog_ioctl.c
lustre/obdclass/obd_config.c
lustre/utils/lctl.c
lustre/utils/obd.c

index c519160..682596e 100644 (file)
@@ -229,6 +229,8 @@ int obd_llog_finish(struct obd_device *obd, int count);
 /* llog_ioctl.c */
 int llog_ioctl(const struct lu_env *env, struct llog_ctxt *ctxt, int cmd,
               struct obd_ioctl_data *data);
+int llog_catalog_list(const struct lu_env *env, struct dt_device *d,
+                     int count, struct obd_ioctl_data *data);
 
 /* llog_net.c */
 int llog_initiator_connect(struct llog_ctxt *ctxt);
index aa8e39d..89fec8e 100644 (file)
@@ -1857,7 +1857,6 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
         struct obd_device *obd = exp->exp_obd;
         struct obd_ioctl_data *data = karg;
         struct obd_import *imp = obd->u.cli.cl_import;
-        struct llog_ctxt *ctxt;
         int rc;
         ENTRY;
 
@@ -1910,22 +1909,6 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
         case IOC_OSC_SET_ACTIVE:
                 rc = ptlrpc_set_import_active(imp, data->ioc_offset);
                 GOTO(out, rc);
-        case OBD_IOC_PARSE: {
-                ctxt = llog_get_context(exp->exp_obd, LLOG_CONFIG_REPL_CTXT);
-               rc = class_config_parse_llog(NULL, ctxt, data->ioc_inlbuf1,
-                                            NULL);
-                llog_ctxt_put(ctxt);
-                GOTO(out, rc);
-        }
-#ifdef __KERNEL__
-        case OBD_IOC_LLOG_INFO:
-        case OBD_IOC_LLOG_PRINT: {
-                ctxt = llog_get_context(obd, LLOG_CONFIG_REPL_CTXT);
-               rc = llog_ioctl(NULL, ctxt, cmd, data);
-                llog_ctxt_put(ctxt);
-                GOTO(out, rc);
-        }
-#endif
         case OBD_IOC_POLL_QUOTACHECK:
                 rc = mdc_quota_poll_check(exp, (struct if_quotacheck *)karg);
                 GOTO(out, rc);
index 0861208..3f89faf 100644 (file)
@@ -5455,6 +5455,9 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
                 rc = mdt_ioc_version_get(mti, karg);
                 break;
         }
+       case OBD_IOC_CATLOGLIST:
+               rc = llog_catalog_list(&env, mdt->mdt_bottom, 0, karg);
+               break;
        default:
                rc = -EOPNOTSUPP;
                CERROR("%s: Not supported cmd = %d, rc = %d\n",
index 1b5f2e0..50385c3 100644 (file)
@@ -803,6 +803,9 @@ out_free:
                break;
         }
 
+       case OBD_IOC_CATLOGLIST:
+               rc = mgs_list_logs(&env, mgs, data);
+               break;
        case OBD_IOC_LLOG_CANCEL:
        case OBD_IOC_LLOG_REMOVE:
         case OBD_IOC_LLOG_CHECK:
index 4cb0eb0..3bfd6a8 100644 (file)
@@ -3688,6 +3688,43 @@ int mgs_erase_logs(const struct lu_env *env, struct mgs_device *mgs, char *fsnam
         RETURN(rc);
 }
 
+/* list all logs for the given fs */
+int mgs_list_logs(const struct lu_env *env, struct mgs_device *mgs,
+                 struct obd_ioctl_data *data)
+{
+       cfs_list_t               list;
+       struct mgs_direntry     *dirent, *n;
+       char                    *out, *suffix;
+       int                      l, remains, rc;
+
+       ENTRY;
+
+       /* Find all the logs in the CONFIGS directory */
+       rc = class_dentry_readdir(env, mgs, &list);
+       if (rc) {
+               CERROR("%s: can't read %s dir = %d\n",
+                      mgs->mgs_obd->obd_name, MOUNT_CONFIGS_DIR, rc);
+               RETURN(rc);
+       }
+
+       out = data->ioc_bulk;
+       remains = data->ioc_inllen1;
+       cfs_list_for_each_entry_safe(dirent, n, &list, list) {
+               cfs_list_del(&dirent->list);
+               suffix = strrchr(dirent->name, '-');
+               if (suffix != NULL) {
+                       l = snprintf(out, remains, "config log: $%s\n",
+                                    dirent->name);
+                       out += l;
+                       remains -= l;
+               }
+               mgs_direntry_free(dirent);
+               if (remains <= 0)
+                       break;
+       }
+       RETURN(rc);
+}
+
 /* from llog_swab */
 static void print_lustre_cfg(struct lustre_cfg *lcfg)
 {
index e103eec..0752141 100644 (file)
@@ -89,7 +89,7 @@ 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_parse_rec(struct llog_rec_hdr *rec, char *buf, int size);
+int class_config_yaml_output(struct llog_rec_hdr *rec, char *buf, int size);
 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 9de449f..3391618 100644 (file)
@@ -222,7 +222,7 @@ 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_parse_rec(rec, out, remains);
+               rc = class_config_yaml_output(rec, out, remains);
                if (rc < 0)
                        RETURN(rc);
                l = rc;
@@ -368,7 +368,7 @@ int llog_ioctl(const struct lu_env *env, struct llog_ctxt *ctxt, int cmd,
                        GOTO(out_close, rc = -EINVAL);
 
                if (handle->lgh_hdr->llh_flags & LLOG_F_IS_PLAIN) {
-                       rc = llog_cancel_rec(NULL, handle, cookie.lgc_index);
+                       rc = llog_cancel_rec(env, handle, cookie.lgc_index);
                        GOTO(out_close, rc);
                } else if (!(handle->lgh_hdr->llh_flags & LLOG_F_IS_CAT)) {
                        GOTO(out_close, rc = -EINVAL);
@@ -427,3 +427,50 @@ out_close:
        RETURN(rc);
 }
 EXPORT_SYMBOL(llog_ioctl);
+
+int llog_catalog_list(const struct lu_env *env, struct dt_device *d,
+                     int count, struct obd_ioctl_data *data)
+{
+       int                      size, i;
+       struct llog_catid       *idarray;
+       struct llog_logid       *id;
+       char                    *out;
+       int                      l, remains, rc = 0;
+
+       ENTRY;
+
+       if (count == 0) { /* get total number of logs */
+               rc = llog_osd_get_cat_list(env, d, 0, 0, NULL);
+               if (rc < 0)
+                       RETURN(rc);
+               count = rc;
+       }
+
+       size = sizeof(*idarray) * count;
+
+       OBD_ALLOC_LARGE(idarray, size);
+       if (!idarray)
+               RETURN(-ENOMEM);
+
+       rc = llog_osd_get_cat_list(env, d, 0, count, idarray);
+       if (rc)
+               GOTO(out, rc);
+
+       out = data->ioc_bulk;
+       remains = data->ioc_inllen1;
+       for (i = 0; i < count; i++) {
+               id = &idarray[i].lci_logid;
+               l = snprintf(out, remains,
+                            "catalog log: #"DOSTID"#%08x\n",
+                            POSTID(&id->lgl_oi),
+                            id->lgl_ogen);
+               out += l;
+               remains -= l;
+               if (remains <= 0)
+                       break;
+       }
+out:
+       OBD_FREE_LARGE(idarray, size);
+       RETURN(rc);
+}
+EXPORT_SYMBOL(llog_catalog_list);
index 43b79b8..94352df 100644 (file)
@@ -1637,6 +1637,114 @@ parse_out:
 }
 EXPORT_SYMBOL(class_config_parse_llog);
 
+struct lcfg_type_data {
+       __u32    ltd_type;
+       char    *ltd_name;
+       char    *ltd_bufs[4];
+} lcfg_data_table[] = {
+       { LCFG_ATTACH, "attach", { "type", "UUID", "3", "4" } },
+       { LCFG_DETACH, "detach", { "1", "2", "3", "4" } },
+       { LCFG_SETUP, "setup", { "UUID", "node", "options", "failout" } },
+       { LCFG_CLEANUP, "cleanup", { "1", "2", "3", "4" } },
+       { LCFG_ADD_UUID, "add_uuid", { "node", "2", "3", "4" }  },
+       { LCFG_DEL_UUID, "del_uuid", { "1", "2", "3", "4" }  },
+       { LCFG_MOUNTOPT, "new_profile", { "name", "lov", "lmv", "4" }  },
+       { LCFG_DEL_MOUNTOPT, "del_mountopt", { "1", "2", "3", "4" } , },
+       { LCFG_SET_TIMEOUT, "set_timeout", { "parameter", "2", "3", "4" }  },
+       { LCFG_SET_UPCALL, "set_upcall", { "1", "2", "3", "4" }  },
+       { LCFG_ADD_CONN, "add_conn", { "node", "2", "3", "4" }  },
+       { LCFG_DEL_CONN, "del_conn", { "1", "2", "3", "4" }  },
+       { LCFG_LOV_ADD_OBD, "add_osc", { "ost", "index", "gen", "UUID" } },
+       { LCFG_LOV_DEL_OBD, "del_osc", { "1", "2", "3", "4" } },
+       { LCFG_PARAM, "set_param", { "parameter", "value", "3", "4" } },
+       { LCFG_MARKER, "marker", { "1", "2", "3", "4" } },
+       { LCFG_LOG_START, "log_start", { "1", "2", "3", "4" } },
+       { LCFG_LOG_END, "log_end", { "1", "2", "3", "4" } },
+       { LCFG_LOV_ADD_INA, "add_osc_inactive", { "1", "2", "3", "4" }  },
+       { LCFG_ADD_MDC, "add_mdc", { "mdt", "index", "gen", "UUID" } },
+       { LCFG_DEL_MDC, "del_mdc", { "1", "2", "3", "4" } },
+       { LCFG_SPTLRPC_CONF, "security", { "parameter", "2", "3", "4" } },
+       { LCFG_POOL_NEW, "new_pool", { "fsname", "pool", "3", "4" }  },
+       { LCFG_POOL_ADD, "add_pool", { "fsname", "pool", "ost", "4" } },
+       { LCFG_POOL_REM, "remove_pool", { "fsname", "pool", "ost", "4" } },
+       { LCFG_POOL_DEL, "del_pool", { "fsname", "pool", "3", "4" } },
+       { LCFG_SET_LDLM_TIMEOUT, "set_ldlm_timeout",
+         { "parameter", "2", "3", "4" } },
+       { 0, NULL, { NULL, NULL, NULL, NULL } }
+};
+
+static struct lcfg_type_data *lcfg_cmd2data(__u32 cmd)
+{
+       int i = 0;
+
+       while (lcfg_data_table[i].ltd_type != 0) {
+               if (lcfg_data_table[i].ltd_type == cmd)
+                       return &lcfg_data_table[i];
+               i++;
+       }
+       return NULL;
+}
+
+/**
+ * parse config record and output dump in supplied buffer.
+ * This is separated from class_config_dump_handler() to use
+ * for ioctl needs as well
+ *
+ * Sample Output:
+ * - { 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)
+{
+       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);
+       if (rc < 0)
+               return rc;
+
+       ldata = lcfg_cmd2data(lcfg->lcfg_command);
+       if (ldata == NULL)
+               return -ENOTTY;
+
+       if (lcfg->lcfg_command == LCFG_MARKER)
+               return 0;
+
+       /* form YAML entity */
+       ptr += snprintf(ptr, end - ptr, "- { event: %s", ldata->ltd_name);
+
+       if (lcfg->lcfg_flags)
+               ptr += snprintf(ptr, end - ptr, ", flags: %#08x",
+                               lcfg->lcfg_flags);
+       if (lcfg->lcfg_num)
+               ptr += snprintf(ptr, end - ptr, ", num: %#08x",
+                               lcfg->lcfg_num);
+       if (lcfg->lcfg_nid)
+               ptr += snprintf(ptr, end - ptr, ", nid: %s("LPX64")",
+                               libcfs_nid2str(lcfg->lcfg_nid),
+                               lcfg->lcfg_nid);
+
+       if (LUSTRE_CFG_BUFLEN(lcfg, 0) > 0)
+               ptr += snprintf(ptr, end - ptr, ", device: %s",
+                               lustre_cfg_string(lcfg, 0));
+
+       for (i = 1; i < lcfg->lcfg_bufcount; i++) {
+               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, " }\n");
+       /* return consumed bytes */
+       rc = ptr - buf;
+       return rc;
+}
+
 /**
  * parse config record and output dump in supplied buffer.
  * This is separated from class_config_dump_handler() to use
@@ -1683,6 +1791,7 @@ int class_config_parse_rec(struct llog_rec_hdr *rec, char *buf, int size)
                                        lustre_cfg_string(lcfg, i));
                }
        }
+       ptr += snprintf(ptr, end - ptr, "\n");
        /* return consumed bytes */
        rc = ptr - buf;
        RETURN(rc);
@@ -1703,7 +1812,7 @@ int class_config_dump_handler(const struct lu_env *env,
 
        if (rec->lrh_type == OBD_CFG_REC) {
                class_config_parse_rec(rec, outstr, 256);
-               LCONSOLE(D_WARNING, "   %s\n", outstr);
+               LCONSOLE(D_WARNING, "   %s", outstr);
        } else {
                LCONSOLE(D_WARNING, "unhandled lrh_type: %#x\n", rec->lrh_type);
                rc = -EINVAL;
index fa1063a..c83b9dd 100644 (file)
@@ -366,58 +366,63 @@ command_t cmdlist[] = {
        {"lfsck_stop", jt_lfsck_stop, 0, "stop lfsck(s)\n"
         "usage: lfsck_stop <-M | --device [MDT,OST]_device> [-h | --help]"},
 
-        {"==== obsolete (DANGEROUS) ====", jt_noop, 0, "obsolete (DANGEROUS)"},
-        /* some test scripts still use these */
-        {"cfg_device", jt_obd_device, 0,
-         "set current device to <name>\n"
-         "usage: device <name>"},
-        {"recover", jt_obd_recover, 0,
-         "try to restore a lost connection immediately\n"
-         "usage: recover [MDC/OSC device]"},
-        /* saving for sanity 44a */
-        {"lov_getconfig", jt_obd_lov_getconfig, 0,
-         "read lov configuration from an mds device\n"
-         "usage: lov_getconfig <mountpoint>"},
-        /* Llog operations */
-        {"llog_catlist", jt_llog_catlist, 0,
-         "list all catalog logs on current device.\n"
-         "usage: llog_catlist"},
-        {"llog_info", jt_llog_info, 0,
-         "print log header information.\n"
-         "usage: llog_info <$logname|#oid#ogr#ogen>\n"
-         "       oid, ogr and ogen are hexadecimal."},
-        {"llog_print", jt_llog_print, 0,
-         "print log content information.\n"
-         "usage: llog_print <$logname|#oid#ogr#ogen> [from] [to]\n"
-         "       oid, ogr and ogen are hexadecimal.\n"
-         "       print all records from index 1 by default."},
-        {"llog_check", jt_llog_check, 0,
-         "print log content information.\n"
-         "usage: llog_check <$logname|#oid#ogr#ogen> [from] [to]\n"
-         "       oid, ogr and ogen are hexadecimal.\n"
-         "       check all records from index 1 by default."},
-         {"llog_cancel", jt_llog_cancel, 0,
-         "cancel one record in log.\n"
-         "usage: llog_cancel <catalog id|catalog name> <log id> <index>"},
-        {"llog_remove", jt_llog_remove, 0,
-         "remove one log from catalog, erase it from disk.\n"
-         "usage: llog_remove <catalog id|catalog name> <log id>"},
-        /* network operations */
-        {"add_interface", jt_ptl_add_interface, 0, "add interface entry\n"
-         "usage: add_interface ip [netmask]"},
-        {"del_interface", jt_ptl_del_interface, 0, "del interface entry\n"
-         "usage: del_interface [ip]"},
-        {"add_route", jt_ptl_add_route, 0,
-         "add an entry to the portals routing table\n"
+       {"==== obsolete (DANGEROUS) ====", jt_noop, 0, "obsolete (DANGEROUS)"},
+       /* some test scripts still use these */
+       {"cfg_device", jt_obd_device, 0,
+        "set current device to <name>\n"
+        "usage: device <name>"},
+       {"recover", jt_obd_recover, 0,
+        "try to restore a lost connection immediately\n"
+        "usage: recover [MDC/OSC device]"},
+       /* saving for sanity 44a */
+       {"lov_getconfig", jt_obd_lov_getconfig, 0,
+        "read lov configuration from an mds device\n"
+        "usage: lov_getconfig <mountpoint>"},
+       /* Llog operations */
+       {"llog_catlist", jt_llog_catlist, 0,
+        "list all catalog logs on current device.\n"
+        "usage: llog_catlist"},
+       {"llog_info", jt_llog_info, 0,
+        "print log header information.\n"
+        "usage: llog_info <$logname|#oid#ogr#ogen>\n"
+        "       oid, ogr and ogen are hexadecimal."},
+       {"llog_print", jt_llog_print, 0,
+        "print log content information.\n"
+        "usage: llog_print <$logname|#oid#ogr#ogen> [from] [to]\n"
+        "       oid, ogr and ogen are hexadecimal.\n"
+        "       print all records from index 1 by default."},
+       {"llog_check", jt_llog_check, 0,
+        "print log content information.\n"
+        "usage: llog_check <$logname|#oid#ogr#ogen> [from] [to]\n"
+        "       oid, ogr and ogen are hexadecimal.\n"
+        "       check all records from index 1 by default."},
+        {"llog_cancel", jt_llog_cancel, 0,
+        "cancel one record in log.\n"
+        "This command supports both positional and optional arguments\n"
+        "usage (positional args): "
+        "llog_cancel <catalog id|catalog name> [log id] <index>\n"
+        "usage (optional args): "
+        "llog_cancel --catalog <catalog id|catalog name> --log_id <log_id> "
+        "--log_idx <index>"},
+       {"llog_remove", jt_llog_remove, 0,
+        "remove one log from catalog or plain log, erase it from disk.\n"
+        "usage: llog_remove <catalog id|catalog name> <log id>"},
+       /* network operations */
+       {"add_interface", jt_ptl_add_interface, 0, "add interface entry\n"
+        "usage: add_interface ip [netmask]"},
+       {"del_interface", jt_ptl_del_interface, 0, "del interface entry\n"
+        "usage: del_interface [ip]"},
+       {"add_route", jt_ptl_add_route, 0,
+        "add an entry to the portals routing table\n"
         "usage: add_route <gateway> [<hops> [<priority>]]"},
-        {"del_route", jt_ptl_del_route, 0,
-         "delete route via gateway to targets from the portals routing table\n"
-         "usage: del_route <gateway> [<target>] [<target>]"},
-        {"set_route", jt_ptl_notify_router, 0,
-         "enable/disable routes via gateway in the portals routing table\n"
-         "usage: set_route <gateway> <up/down> [<time>]"},
+       {"del_route", jt_ptl_del_route, 0,
+        "delete route via gateway to targets from the portals routing table\n"
+        "usage: del_route <gateway> [<target>] [<target>]"},
+       {"set_route", jt_ptl_notify_router, 0,
+        "enable/disable routes via gateway in the portals routing table\n"
+        "usage: set_route <gateway> <up/down> [<time>]"},
 
-        { 0, 0, 0, NULL }
+       { 0, 0, 0, NULL }
 };
 
 int lctl_main(int argc, char **argv)
index 854def8..a0c45b5 100644 (file)
@@ -2706,41 +2706,118 @@ int jt_llog_print(int argc, char **argv)
         return rc;
 }
 
+static int llog_cancel_parse_optional(int argc, char **argv,
+                                     struct obd_ioctl_data *data)
+{
+       int cOpt;
+       const char *const short_options = "c:l:i:h";
+       const struct option long_options[] = {
+               {"catalog", required_argument, NULL, 'c'},
+               {"log_id", required_argument, NULL, 'l'},
+               {"log_idx", required_argument, NULL, 'i'},
+               {"help", no_argument, NULL, 'h'},
+               {NULL, 0, NULL, 0}
+       };
+
+       /* sanity check */
+       if (!data || argc <= 1) {
+               return -1;
+       }
+
+       /*now process command line arguments*/
+       while ((cOpt = getopt_long(argc, argv, short_options,
+                                       long_options, NULL)) != -1) {
+               switch (cOpt) {
+               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 'i':
+                       data->ioc_inllen3 = strlen(optarg) + 1;
+                       data->ioc_inlbuf3 = optarg;
+                       break;
+
+               case 'h':
+               default:
+                       return -1;
+               }
+       }
+
+       if ((data->ioc_inlbuf1 == NULL) || (data->ioc_inlbuf3 == NULL)) {
+               /* missing mandatory parameters */
+               return -1;
+       }
+
+       return 0;
+}
+
 int jt_llog_cancel(int argc, char **argv)
 {
-        struct obd_ioctl_data data;
-        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
-        int rc;
-
-        if (argc != 4)
-                return CMD_HELP;
+       struct obd_ioctl_data data;
+       char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
+       int rc, i;
+
+       /* check that the arguments provided are either all
+        * optional or all positional.  No mixing allowed
+        *
+        * if argc is 4 or 3 then check all arguments to ensure that none
+        * of them start with a '-'.  If so then this is invalid.
+        * Otherwise if arg is > 4 then assume that this is optional
+        * arguments, and parse as such ignoring any thing that's not
+        * optional.  The result is that user must use optional arguments
+        * for all mandatory parameters.  Code will ignore all other args
+        *
+        * The positional arguments option should eventually be phased out.
+        */
+       memset(&data, 0, sizeof(data));
+       data.ioc_dev = cur_device;
 
-        memset(&data, 0, sizeof(data));
-        data.ioc_dev = cur_device;
-        data.ioc_inllen1 = strlen(argv[1]) + 1;
-        data.ioc_inlbuf1 = argv[1];
-        data.ioc_inllen2 = strlen(argv[2]) + 1;
-        data.ioc_inlbuf2 = argv[2];
-        data.ioc_inllen3 = strlen(argv[3]) + 1;
-        data.ioc_inlbuf3 = argv[3];
-        memset(buf, 0, sizeof(rawbuf));
-        rc = obd_ioctl_pack(&data, &buf, sizeof(rawbuf));
-        if (rc) {
-                fprintf(stderr, "error: %s: invalid ioctl\n",
-                        jt_cmdname(argv[0]));
-                return rc;
-        }
+       if (argc == 3 || argc == 4) {
+               for (i = 1; i < argc; i++) {
+                       if (argv[i][0] == '-')
+                               return CMD_HELP;
+               }
+               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 {
+                       data.ioc_inllen3 = strlen(argv[2]) + 1;
+                       data.ioc_inlbuf3 = argv[2];
+               }
+       } else {
+               if (llog_cancel_parse_optional(argc, argv, &data) != 0)
+                       return CMD_HELP;
+       }
 
-        rc = l_ioctl(OBD_DEV_ID, OBD_IOC_LLOG_CANCEL, buf);
-        if (rc == 0)
-                fprintf(stdout, "index %s be canceled.\n", argv[3]);
-        else
-                fprintf(stderr, "OBD_IOC_LLOG_CANCEL failed: %s\n",
-                        strerror(errno));
+       memset(buf, 0, sizeof(rawbuf));
+       rc = obd_ioctl_pack(&data, &buf, sizeof(rawbuf));
+       if (rc) {
+               fprintf(stderr, "error: %s: invalid ioctl\n",
+                       jt_cmdname(argv[0]));
+               return rc;
+       }
 
-        return rc;
+       rc = l_ioctl(OBD_DEV_ID, OBD_IOC_LLOG_CANCEL, buf);
+       if (rc == 0)
+               fprintf(stdout, "index %s was canceled.\n",
+                       argc == 4 ? argv[3] : argv[2]);
+       else
+               fprintf(stderr, "OBD_IOC_LLOG_CANCEL failed: %s\n",
+                       strerror(errno));
 
+       return rc;
 }
+
 int jt_llog_check(int argc, char **argv)
 {
         struct obd_ioctl_data data;
@@ -2814,10 +2891,11 @@ int jt_llog_remove(int argc, char **argv)
 
         rc = l_ioctl(OBD_DEV_ID, OBD_IOC_LLOG_REMOVE, buf);
         if (rc == 0) {
-                if (argc == 3)
-                        fprintf(stdout, "log %s are removed.\n", argv[2]);
-                else
-                        fprintf(stdout, "the log in catalog %s are removed. \n", argv[1]);
+                if (argc == 2)
+                       fprintf(stdout, "log %s is removed.\n", argv[1]);
+               else
+                       fprintf(stdout, "the log in catalog %s is removed. \n",
+                               argv[1]);
         } else
                 fprintf(stderr, "OBD_IOC_LLOG_REMOVE failed: %s\n",
                         strerror(errno));