From: Yunkai Zhang <qiushu.zyk at taobao.com> V2: - update commit log, make it more descriptive - use 'options' name in inner struct instread of 'self_options' --------------------------------------------------------------- >8 Now, all collie's command share the same global collie_options, it will lead to option's name conflict among commands if they use the same options but with different description. By moving the private options into individual structure of each command, and make collie_options only contain the common part of them, we can solve this problem. Signed-off-by: Yunkai Zhang <qiushu.zyk at taobao.com> --- collie/cluster.c | 24 ++++++++++++++++++------ collie/collie.c | 49 +++++++++++++++++++++++++------------------------ collie/collie.h | 1 + collie/vdi.c | 56 ++++++++++++++++++++++++++++++++++++++++---------------- 4 files changed, 84 insertions(+), 46 deletions(-) diff --git a/collie/cluster.c b/collie/cluster.c index 9302b78..2cca3ec 100644 --- a/collie/cluster.c +++ b/collie/cluster.c @@ -16,6 +16,17 @@ #include "collie.h" +static struct sd_option cluster_options[] = { + {'b', "store", 1, "specify backend store"}, + {'c', "copies", 1, "specify the data redundancy (number of copies)"}, + {'m', "mode", 1, "mode (safe, quorum, unsafe)"}, + {'f', "force", 0, "do not prompt for confirmation"}, + {'R', "restore", 1, "restore the cluster"}, + {'l', "list", 0, "list the user epoch information"}, + + { 0, NULL, 0, NULL }, +}; + struct cluster_cmd_data { uint32_t epoch; int list; @@ -501,19 +512,20 @@ static int cluster_recover(int argc, char **argv) static struct subcommand cluster_cmd[] = { {"info", NULL, "aprh", "show cluster information", - NULL, SUBCMD_FLAG_NEED_NODELIST, cluster_info}, + NULL, SUBCMD_FLAG_NEED_NODELIST, cluster_info, cluster_options}, {"format", NULL, "bcmaph", "create a Sheepdog store", - NULL, 0, cluster_format}, + NULL, 0, cluster_format, cluster_options}, {"shutdown", NULL, "aph", "stop Sheepdog", - NULL, SUBCMD_FLAG_NEED_NODELIST, cluster_shutdown}, + NULL, SUBCMD_FLAG_NEED_NODELIST, cluster_shutdown, cluster_options}, {"snapshot", NULL, "aRlph", "snapshot/restore the cluster", - NULL, 0, cluster_snapshot}, + NULL, 0, cluster_snapshot, cluster_options}, {"cleanup", NULL, "aph", "cleanup the useless snapshot data from recovery", - NULL, 0, cluster_cleanup}, + NULL, 0, cluster_cleanup, cluster_options}, {"recover", NULL, "afph", "See 'collie cluster recover' for more information\n", - cluster_recover_cmd, SUBCMD_FLAG_NEED_THIRD_ARG, cluster_recover}, + cluster_recover_cmd, SUBCMD_FLAG_NEED_THIRD_ARG, + cluster_recover, cluster_options}, {NULL,}, }; diff --git a/collie/collie.c b/collie/collie.c index 32c044a..c1b1854 100644 --- a/collie/collie.c +++ b/collie/collie.c @@ -25,29 +25,13 @@ int raw_output = 0; static const struct sd_option collie_options[] = { - /* common options */ + /* common options for all collie commands */ {'a', "address", 1, "specify the daemon address (default: localhost)"}, {'p', "port", 1, "specify the daemon port"}, {'r', "raw", 0, "raw output mode: omit headers, separate fields with\n\ single spaces and print all sizes in decimal bytes"}, {'h', "help", 0, "display this help and exit"}, - /* VDI options */ - {'P', "prealloc", 0, "preallocate all the data objects"}, - {'i', "index", 1, "specify the index of data objects"}, - {'s', "snapshot", 1, "specify a snapshot id or tag name"}, - {'x', "exclusive", 0, "write in an exclusive mode"}, - {'d', "delete", 0, "delete a key"}, - {'C', "cache", 0, "enable object cache"}, - - /* cluster options */ - {'b', "store", 1, "specify backend store"}, - {'c', "copies", 1, "specify the data redundancy (number of copies)"}, - {'m', "mode", 1, "mode (safe, quorum, unsafe)"}, - {'f', "force", 0, "do not prompt for confirmation"}, - {'R', "restore", 1, "restore the cluster"}, - {'l', "list", 0, "list the user epoch information"}, - { 0, NULL, 0, NULL }, }; @@ -127,18 +111,34 @@ out: static int (*command_parser)(int, char *); static int (*command_fn)(int, char **); -static const char *command_options; +static const char *command_opts; static const char *command_arg; static const char *command_desc; +static struct sd_option *command_options; static const struct sd_option *find_opt(int ch) { int i; + struct sd_option *opt; + /* search for common options */ for (i = 0; i < ARRAY_SIZE(collie_options); i++) { if (collie_options[i].val == ch) return collie_options + i; } + + /* search for self options */ + if (!command_options) + goto out; + + opt = command_options; + while (opt->val) { + if (opt->val == ch) + return opt; + opt++; + } + +out: fprintf(stderr, "Internal error\n"); exit(EXIT_SYSFAIL); } @@ -246,9 +246,10 @@ static unsigned long setup_commands(const struct command *commands, for (s = commands[i].sub; s->name; s++) { if (!strcmp(s->name, subcmd)) { command_fn = s->fn; - command_options = s->opts; + command_opts = s->opts; command_arg = s->arg; command_desc = s->desc; + command_options = s->options; flags = s->flags; break; } @@ -293,7 +294,7 @@ static void usage(const struct command *commands, int status) void subcommand_usage(char *cmd, char *subcmd, int status) { - int i, n, len = strlen(command_options); + int i, n, len = strlen(command_opts); const struct sd_option *sd_opt; const struct subcommand *sub, *subsub; char name[64]; @@ -318,7 +319,7 @@ void subcommand_usage(char *cmd, char *subcmd, int status) } for (i = 0; i < len; i++) { - sd_opt = find_opt(command_options[i]); + sd_opt = find_opt(command_opts[i]); if (sd_opt->has_arg) printf(" [-%c %s]", sd_opt->val, sd_opt->name); else @@ -337,7 +338,7 @@ void subcommand_usage(char *cmd, char *subcmd, int status) printf("Options:\n"); for (i = 0; i < len; i++) { - sd_opt = find_opt(command_options[i]); + sd_opt = find_opt(command_opts[i]); sprintf(name, "-%c, --%s", sd_opt->val, sd_opt->name); printf(" %-24s%s\n", name, sd_opt->desc); } @@ -363,8 +364,8 @@ int main(int argc, char **argv) optind = 3; - long_options = build_long_options(command_options); - short_options = build_short_options(command_options); + long_options = build_long_options(command_opts); + short_options = build_short_options(command_opts); while ((ch = getopt_long(argc, argv, short_options, long_options, &longindex)) >= 0) { diff --git a/collie/collie.h b/collie/collie.h index ba2d859..7f93ded 100644 --- a/collie/collie.h +++ b/collie/collie.h @@ -49,6 +49,7 @@ struct subcommand { struct subcommand *sub; unsigned long flags; int (*fn)(int, char **); + struct sd_option *options; }; void subcommand_usage(char *cmd, char *subcmd, int status); diff --git a/collie/vdi.c b/collie/vdi.c index 5781e75..d27b5af 100644 --- a/collie/vdi.c +++ b/collie/vdi.c @@ -16,6 +16,17 @@ #include "collie.h" #include "treeview.h" +static struct sd_option vdi_options[] = { + {'P', "prealloc", 0, "preallocate all the data objects"}, + {'i', "index", 1, "specify the index of data objects"}, + {'s', "snapshot", 1, "specify a snapshot id or tag name"}, + {'x', "exclusive", 0, "write in an exclusive mode"}, + {'d', "delete", 0, "delete a key"}, + {'C', "cache", 0, "enable object cache"}, + + { 0, NULL, 0, NULL }, +}; + struct vdi_cmd_data { unsigned int index; int snapshot_id; @@ -1509,37 +1520,50 @@ out: static struct subcommand vdi_cmd[] = { {"check", "<vdiname>", "saph", "check and repair image's consistency", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_check}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_check, vdi_options}, {"create", "<vdiname> <size>", "Paph", "create an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_create}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_create, vdi_options}, {"snapshot", "<vdiname>", "saph", "create a snapshot", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_snapshot}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_snapshot, vdi_options}, {"clone", "<src vdi> <dst vdi>", "sPaph", "clone an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_clone}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_clone, vdi_options}, {"delete", "<vdiname>", "saph", "delete an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_delete}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_delete, vdi_options}, {"list", "[vdiname]", "aprh", "list images", - NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_list}, + NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_list, vdi_options}, {"tree", NULL, "aph", "show images in tree view format", - NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_tree}, + NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_tree, vdi_options}, {"graph", NULL, "aph", "show images in Graphviz dot format", - NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_graph}, + NULL, SUBCMD_FLAG_NEED_NODELIST, vdi_graph, vdi_options}, {"object", "<vdiname>", "isaph", "show object information in the image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_object}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_object, vdi_options}, {"track", "<vdiname>", "isaph", "show the object epoch trace in the image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_track}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_track, vdi_options}, {"setattr", "<vdiname> <key> [value]", "dxaph", "set a VDI attribute", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_setattr}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_setattr, vdi_options}, {"getattr", "<vdiname> <key>", "aph", "get a VDI attribute", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_getattr}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_getattr, vdi_options}, {"resize", "<vdiname> <new size>", "aph", "resize an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_resize}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_resize, vdi_options}, {"read", "<vdiname> [<offset> [<len>]]", "saph", "read data from an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_read}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_read, vdi_options}, {"write", "<vdiname> [<offset> [<len>]]", "apCh", "write data to an image", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_write}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_write, vdi_options}, {"flush", "<vdiname>", "aph", "flush data to cluster", - NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, vdi_flush}, + NULL, SUBCMD_FLAG_NEED_NODELIST|SUBCMD_FLAG_NEED_THIRD_ARG, + vdi_flush, vdi_options}, {NULL,}, }; -- 1.7.11.2 |