All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
@ 2014-03-08 18:47 Leandro Dorileo
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 1/2] qemu-arg: introduce a " Leandro Dorileo
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-08 18:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Leandro Dorileo,
	Stefan Weil, Michael Tokarev, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek, Peter Lieven

The following patchset introduces a general purpose argument parser and migrates
qemu-img to make use of it. qemu-img is just the first user of it, if we see a
good feedback here I move forward and migrate all the other possible users.

Leandro Dorileo (2):
  qemu-arg: introduce a general purpose argument parser
  qemu-img: migrate to use qemu-arg

 .gitignore              |    1 +
 Makefile                |   12 +-
 include/qemu/qemu-arg.h |  287 ++++++++++++
 qemu-img-cmds.hx        |   77 ---
 qemu-img-descs.h        |  128 +++++
 qemu-img.c              | 1184 ++++++++++++++++-------------------------------
 util/Makefile.objs      |    1 +
 util/qemu-arg.c         |  887 +++++++++++++++++++++++++++++++++++
 8 files changed, 1706 insertions(+), 871 deletions(-)
 create mode 100644 include/qemu/qemu-arg.h
 delete mode 100644 qemu-img-cmds.hx
 create mode 100644 qemu-img-descs.h
 create mode 100644 util/qemu-arg.c

-- 
1.9.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH RFC 1/2] qemu-arg: introduce a general purpose argument parser
  2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
@ 2014-03-08 18:47 ` Leandro Dorileo
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg Leandro Dorileo
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-08 18:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Leandro Dorileo,
	Stefan Weil, Michael Tokarev, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek, Peter Lieven

qemu-arg defines a standardized API for argument parsing, help displaying and
texi generation/sync.

The implementation supports command + arguments form (i.e qemu-img requirements)
and a more general simple arguments parsing. So we can parse:

$ prog <command> --arg1 --arg2
$ prog --arg1 --arg2

We support the following:
   + basic arguments validation (i.e required arguments and required values);
   + basic arguments transformations (integer, bool values)
   + repeated/"cumullated" arguments (i.e -o opt1=val -o opt2=val2 will result the
     string "opt1=val,opt2=val2")
   + positional arguments;
     + identified positional for fixed/defined numbers of expected positional args;
     + listed positional for N expected positional args;
   + help messages generation;
   + texi generation;
   + default value setting;
   + mutually exclusive arguments;
   + display and parsing decorated arguments ("--argument value" and "--argument=value"
      are valid)

Signed-off-by: Leandro Dorileo <l@dorileo.org>
---
 include/qemu/qemu-arg.h | 287 ++++++++++++++++
 util/Makefile.objs      |   1 +
 util/qemu-arg.c         | 887 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1175 insertions(+)
 create mode 100644 include/qemu/qemu-arg.h
 create mode 100644 util/qemu-arg.c

diff --git a/include/qemu/qemu-arg.h b/include/qemu/qemu-arg.h
new file mode 100644
index 0000000..c8d8fb4
--- /dev/null
+++ b/include/qemu/qemu-arg.h
@@ -0,0 +1,287 @@
+/*
+ * QEMU argument helper
+ *
+ * Copyright (c) 2014 Leandro Dorileo <l@dorileo.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef _QEMU_ARG_H_
+#define _QEMU_ARG_H_
+
+#include <libintl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+typedef struct _QemuArgContext QemuArgContext;
+typedef struct _QemuArgCommand QemuArgCommand;
+
+typedef enum _QemuArgOptType {
+    QEMU_ARG_OPT_TYPE_INT,
+    QEMU_ARG_OPT_TYPE_BOOL,
+    QEMU_ARG_OPT_TYPE_STR,
+    QEMU_ARG_OPT_TYPE_POSITIONAL,
+    QEMU_ARG_OPT_TYPE_POSITIONAL_LIST,
+    QEMU_ARG_OPT_TYPE_DEPRECATED,
+    QEMU_ARG_OPT_TYPE_GROUP,
+    QEMU_ARG_OPT_TYPE_SENTINEL,
+} QemuArgOptType;
+
+typedef struct _QemuArgIntValue {
+    /** default value */
+    int def_val;
+
+    /** user value pointer */
+    int *value;
+} QemuArgIntValue;
+
+typedef struct _QemuArgBoolValue {
+    /** default value */
+    bool def_val;
+
+    /** user value pointer */
+    bool *value;
+} QemuArgBoolValue;
+
+typedef struct _QemuArgStrValue {
+    /** default value */
+    char *def_val;
+
+    /** user value pointer */
+    char **value;
+} QemuArgStrValue;
+
+typedef struct _QemuArgStrListValue {
+    /** default value */
+    char **def_val;
+
+    /** user value pointer */
+    char ***value;
+
+    /** list elements counter */
+    int list_cnt;
+} QemuArgStrListValue;
+
+typedef enum _QemuArgOptFlag {
+    ARG_FLAG_NONE       = 0,
+
+    /** provide many arguments instances, their value are concatenated in a
+        comman separated string */
+    ARG_FLAG_CUMULATE   = 1 << 0,
+
+    /** the argument is required */
+    ARG_FLAG_REQUIRED   = 1 << 1,
+
+    /** the argument requires a value like --foo bar where --foo requires bar */
+    ARG_FLAG_REQ_VALUE  = 1 << 2,
+} QemuArgOptFlag;
+
+typedef struct _QemuArgOpt {
+    /** argument type, bool, int, str, etc @see QemuArgOptType */
+    QemuArgOptType type;
+
+    /** the argument's short name i.e -c */
+    const char short_name;
+
+    /** argument's long name i.e --cache */
+    const char *long_name;
+
+    /** argument's description, used to display a hint about the argument's
+        value i.e -f fmt where fmt is the arg's desc */
+    const char *desc;
+
+    /** help message, describes the argument */
+    const char *help;
+
+    /** some behavior flags @see QemuArgOptFlag for possible modifiers */
+    int flags;
+
+    /** indicates the argument was set, for bool values it tells we got the
+        argument and the others we got the argument's value */
+    bool value_set;
+
+    /** value pointer, @see QemuArgIntValue @see QemuArgBoolValue
+        @see QemuArgStrValue @see QemuArgStrListValue */
+    const void *value;
+} QemuArgOpt;
+
+struct _QemuArgCommand {
+    /** the command itself i.e commit, create */
+    const char *cmd;
+
+    /** help msg displayed on main help msg - command listing msg */
+    const char *help;
+
+    /** the command's arguments */
+    QemuArgOpt *args;
+
+    /** number of arguments */
+    int args_cnt;
+
+    /** mutual exclusive groups - it's an array of QemuArgOpt arrays, each array
+        composes arguments mutually exclusive */
+    QemuArgOpt **mutual_groups;
+
+    int (* cb)(const QemuArgContext *ctx, const QemuArgCommand *cmd);
+};
+
+struct _QemuArgContext {
+    /** text displayed on top of help msg */
+    const char *prologue;
+
+    /** text displayed on bottom of help msg */
+    const char *epilogue;
+
+    /** the program name, usage on "usage" msg */
+    const char *prog_name;
+
+    /** command list */
+    const QemuArgCommand *commands;
+
+    /** non command usage or common arguments */
+    QemuArgOpt *args;
+
+    /** number of arguments (non command usage) */
+    int args_cnt;
+
+    /** non command usage */
+    QemuArgOpt **mutual_groups;
+
+    /** support decorated mode, like --arg=value. actually we'll always parse
+     this form, this flag will indicate we must show the long options on help
+    messages @see OPT_DECORATE_LONG and @see OPT_DECORATE_SHORT */
+    int decorate;
+};
+
+/** decoration modes, OPT_DECORATE_LONG | OPT_DECORATE_SHORT will decorate both
+    short and long arguments */
+#define OPT_DECORATE_LONG  (1 << 0)
+#define OPT_DECORATE_SHORT (1 << 1)
+
+/** type wrappers macros */
+#define INT_VALUE(_val, _default)                               \
+    &(QemuArgIntValue) {.value = _val, .def_val = _default}     \
+
+#define BOOL_VALUE(_val, _default)                                      \
+    &(QemuArgBoolValue) {.value = _val, .def_val = _default}            \
+
+#define STR_VALUE(_val, _default)                                       \
+    &(QemuArgStrValue) {.value = _val, .def_val = (char *)_default}     \
+
+#define STR_LIST_VALUE(_val, _default)                                  \
+    &(QemuArgStrListValue) {.value = _val, .def_val = _default}         \
+
+/** QemuArgOpt constructors */
+#define OPT_BOOL(s, l, h, p, df)                                        \
+    {QEMU_ARG_OPT_TYPE_BOOL, s, l, NULL, h, ARG_FLAG_NONE, false,       \
+            BOOL_VALUE(p, df)}                                          \
+
+#define OPT_STR(s, l, d, h, p, df)                                      \
+    {QEMU_ARG_OPT_TYPE_STR, s, l, d, h, ARG_FLAG_REQ_VALUE, false,      \
+            STR_VALUE(p, df)}                                           \
+
+#define OPT_INT(s, l, d, h, p, df)                                      \
+    {QEMU_ARG_OPT_TYPE_INT, s, l, d, h, ARG_FLAG_REQ_VALUE, false,      \
+            INT_VALUE(p, df)}                                           \
+
+#define OPT_CUMUL_STR(s, l, d, h, p, df)                                \
+    {QEMU_ARG_OPT_TYPE_STR, s, l, d, h,                                 \
+            ARG_FLAG_CUMULATE | ARG_FLAG_REQ_VALUE, false,              \
+            STR_VALUE(p, df)}                                           \
+
+#define OPT_CUMUL_STR_REQ(s, l, d, h, p, df)                            \
+    {QEMU_ARG_OPT_TYPE_STR, s, l, d, h,                                 \
+            ARG_FLAG_CUMULATE | ARG_FLAG_REQ_VALUE | ARG_FLAG_REQUIRED, \
+            false, STR_VALUE(p, df)}                                     \
+
+#define OPT_POS(d, h, p, df)                                            \
+    {QEMU_ARG_OPT_TYPE_POSITIONAL, 0, NULL, d, h, ARG_FLAG_NONE,        \
+            false, STR_VALUE(p, df)}                                    \
+
+#define OPT_POS_REQ(d, h, p, df)                                        \
+    {QEMU_ARG_OPT_TYPE_POSITIONAL, 0, NULL, d, h, ARG_FLAG_REQUIRED,    \
+            false, STR_VALUE(p, df)}                                    \
+
+#define OPT_POS_LIST(d, h, p, df)                                       \
+    {QEMU_ARG_OPT_TYPE_POSITIONAL_LIST, 0, NULL, d, h, ARG_FLAG_NONE,   \
+            false, STR_LIST_VALUE(p, df)}                               \
+
+#define OPT_POS_LIST_REQ(d, h, p, df)                                   \
+    {QEMU_ARG_OPT_TYPE_POSITIONAL_LIST, 0, NULL, d, h,                  \
+            ARG_FLAG_REQUIRED, false, STR_LIST_VALUE(p, df)}            \
+
+#define OPT_DEP(s, l, h)                                                \
+    {QEMU_ARG_OPT_TYPE_DEPRECATED, s, l, NULL, h, ARG_FLAG_NONE}        \
+
+#define OPT_GROUP(d)                                                    \
+    {QEMU_ARG_OPT_TYPE_GROUP, 0, NULL, d, NULL, ARG_FLAG_NONE}          \
+
+/** command  and context macro constructor */
+#define OPT_CMD(c, h, a, m, cb)                          \
+    {c, h, a, sizeof(a) / sizeof(QemuArgOpt), m, cb}     \
+
+#define OPT_CTX(n, p, e, pn, c, a, m, d)                                \
+    const QemuArgContext n = {p, e, pn, c, a,                           \
+                              sizeof(a) / sizeof(QemuArgOpt), m, d}     \
+
+#define OPT_MUTUAL_GROUP_SENTINEL NULL
+#define OPT_SENTINEL { QEMU_ARG_OPT_TYPE_SENTINEL }
+#define CMD_SENTINEL { NULL }
+
+/**
+ *
+ * @param argc The program arguments counter
+ * @param argv The program arguments vector
+ * @param ctx The qemu arg context of interest
+ *
+ * Parse commands.
+ *
+ * @return negative number on failure, 0 otherwise.
+ */
+int qemu_arg_parse(int argc, char *argv[], const QemuArgContext *ctx);
+
+/**
+ * @param ctx the context to be cleaned up
+ *
+ * Clean up the argument context.
+ */
+void qemu_arg_context_cleanup(const QemuArgContext *ctx);
+
+/**
+ * @param cmd The command of interest
+ *
+ * Query a command about its parsed positional arguments list.
+ *
+ * @return How many items a positional list has. @see OPT_POS_LIST and
+ * @see OPT_POS_LIST_REQ
+ */
+int qemu_arg_get_command_positional_list_cnt(const QemuArgCommand *cmd);
+
+/**
+ * @param ctx The qemu arg context of interest
+ * @param cmd The qemu command of interest
+ *
+ * Print a command help message.
+ */
+void qemu_arg_print_command_help(const QemuArgContext *ctx,
+                                 const QemuArgCommand *cmd);
+
+#endif /* _QEMU_ARG_H_ */
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 937376b..9d05ec3 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -14,3 +14,4 @@ util-obj-y += crc32c.o
 util-obj-y += throttle.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
+util-obj-y += qemu-arg.o
diff --git a/util/qemu-arg.c b/util/qemu-arg.c
new file mode 100644
index 0000000..e611d97
--- /dev/null
+++ b/util/qemu-arg.c
@@ -0,0 +1,887 @@
+/*
+ * QEMU argument helper
+ *
+ * Copyright (c) 2014 Leandro Dorileo <l@dorileo.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/qemu-arg.h"
+
+#define SUCCESS   0
+#define ERR      -1
+
+#define HELP_BLOCK_PADDING 25
+#define HELP_BLOCK_WIDTH 90
+#define HELP_DESC_BLOCK_WIDTH (HELP_BLOCK_WIDTH - HELP_BLOCK_PADDING)
+
+typedef void (*HelpFunc)(const QemuArgContext *ctx, const void *opaque);
+
+#define OPT_INT_VAL_GET(_opt)                        \
+    ((QemuArgIntValue *)_opt->value)                 \
+
+#define OPT_BOOL_VAL_GET(_opt)                        \
+    ((QemuArgBoolValue *)_opt->value)                 \
+
+#define OPT_STR_VAL_GET(_opt)                       \
+    ((QemuArgStrValue *)_opt->value)                \
+
+#define OPT_STR_LIST_VAL_GET(_opt)                       \
+    ((QemuArgStrListValue *)_opt->value)                 \
+
+#define OPT_HELP                                                        \
+    "show this help message. <command> -h will show the command "       \
+    "specific help message."                                            \
+
+static QemuArgOpt _internal_arg_opts[] = {
+    OPT_STR('h', "help", NULL, OPT_HELP, NULL, NULL),
+    OPT_SENTINEL
+};
+
+static void print_help_block(const char *help, int padding_len,
+                             int length)
+{
+    const char *c;
+    int i, j;
+
+    for (i = 0, c = help; *c != '\0'; c++, i++) {
+        /** should break the line? */
+        if (i >= length && *c == ' ') {
+            printf("\n");
+            for (j = 0; padding_len && j <= padding_len; j++) {
+                printf(" ");
+            }
+            i = 0;
+            continue;
+        }
+        printf("%c", *c);
+    }
+}
+
+static int format_short_opt(QemuArgOpt *opt, int decorate, bool texi)
+{
+    int cnt = 0;
+
+    if (!opt->short_name) {
+        return cnt;
+    }
+
+    cnt += printf("-%c", opt->short_name);
+
+    if (decorate & OPT_DECORATE_SHORT && opt->desc) {
+        if (texi) {
+            cnt += printf("=@var{%s}", opt->desc);
+        } else {
+            cnt += printf("=%s", opt->desc);
+        }
+    } else if (opt->desc) {
+        if (texi) {
+            cnt += printf(" @var{%s}", opt->desc);
+        } else {
+            cnt += printf(" %s", opt->desc);
+        }
+    }
+
+    if (opt->long_name) {
+        cnt += printf(", ");
+    }
+
+    return cnt;
+}
+
+static int format_long_opt(QemuArgOpt *opt, int decorate, bool texi)
+{
+    int cnt = 0;
+
+    if (!opt->long_name) {
+        return cnt;
+    }
+
+    cnt += printf("--%s", opt->long_name);
+
+    if (decorate & OPT_DECORATE_LONG && opt->desc) {
+        if (texi) {
+            cnt += printf("=@var{%s}", opt->desc);
+        } else {
+            cnt += printf("=%s", opt->desc);
+        }
+    } else if (opt->desc) {
+        if (texi) {
+            cnt += printf(" @var{%s}", opt->desc);
+        } else {
+            cnt += printf(" %s", opt->desc);
+        }
+    }
+
+    return cnt;
+}
+
+static int format_mutual_opt(QemuArgOpt **opt, int decorate, bool texi)
+{
+    QemuArgOpt **mutual, *itr;
+    int cnt = 0;
+
+    for (mutual = opt; *mutual; mutual++) {
+        cnt += printf(" [");
+
+        for (itr = *mutual; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+            cnt += format_short_opt(itr, decorate, texi);
+            cnt += format_long_opt(itr, decorate, texi);
+
+            if ((itr + 1)->type != QEMU_ARG_OPT_TYPE_SENTINEL) {
+                cnt += printf(" | ");
+            }
+        }
+
+        cnt += printf("]");
+    }
+
+    return cnt;
+}
+
+static void print_positional_arg_list(QemuArgOpt *list)
+{
+    QemuArgOpt *itr;
+
+    for (itr = list; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        int cnt = 0;
+
+        if (itr->type != QEMU_ARG_OPT_TYPE_POSITIONAL &&
+            itr->type != QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            continue;
+        }
+
+        cnt += printf("  %s", itr->desc);
+
+        if (cnt < HELP_BLOCK_PADDING) {
+            for (; cnt < HELP_BLOCK_PADDING; cnt++) {
+                printf(" ");
+            }
+        } else {
+            printf("\n");
+            for (cnt = 0; cnt < HELP_BLOCK_PADDING; cnt++) {
+                printf(" ");
+            }
+        }
+
+        print_help_block(itr->help, HELP_BLOCK_PADDING - 1,
+                         HELP_DESC_BLOCK_WIDTH);
+
+        printf("\n");
+    }
+}
+
+static void print_arg_list(QemuArgOpt *list, int decorate)
+{
+    QemuArgOpt *itr;
+
+    for (itr = list; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        int cnt = 0;
+
+        if (itr->type == QEMU_ARG_OPT_TYPE_GROUP) {
+            printf("\n%s options:\n", itr->desc);
+            continue;
+        }
+
+        cnt += printf("  ");
+        cnt += format_short_opt(itr, decorate, false);
+        cnt += format_long_opt(itr, decorate, false);
+
+        if (cnt < HELP_BLOCK_PADDING) {
+            for (; cnt < HELP_BLOCK_PADDING; cnt++) {
+                printf(" ");
+            }
+        } else {
+            printf("\n");
+            for (cnt = 0; cnt < HELP_BLOCK_PADDING; cnt++) {
+                printf(" ");
+            }
+        }
+
+        print_help_block(itr->help, HELP_BLOCK_PADDING - 1,
+                         HELP_DESC_BLOCK_WIDTH);
+        printf("\n");
+    }
+}
+
+static void print_help(const QemuArgContext *ctx)
+{
+    const QemuArgCommand *cmd;
+
+    if (ctx->prologue) {
+        printf("%s\n\n", ctx->prologue);
+    }
+
+    printf("%s\n", "Global options:");
+    print_arg_list(_internal_arg_opts, 0);
+
+    if (ctx->args) {
+        print_arg_list(ctx->args, ctx->decorate);
+        printf("\n");
+    }
+
+    if (ctx->commands) {
+        printf("%s\n", "Commands:");
+
+        for (cmd = ctx->commands; cmd->cmd; cmd++) {
+            printf("  %-10s %s\n", cmd->cmd, cmd->help);
+        }
+        printf("\n");
+    }
+
+    if (ctx->epilogue) {
+        print_help_block(ctx->epilogue, 0, HELP_BLOCK_WIDTH);
+        printf("\n");
+    }
+}
+
+static int format_positional_opt(QemuArgOpt *opt, bool texi)
+{
+    int cnt = 0;
+
+    if (!(opt->flags & ARG_FLAG_REQUIRED)) {
+        cnt += printf(" [");
+    } else {
+        cnt += printf(" ");
+    }
+
+    if (texi) {
+        cnt += printf("@var{%s}", opt->desc);
+    } else {
+        cnt += printf("%s", opt->desc);
+    }
+
+    if (!(opt->flags & ARG_FLAG_REQUIRED)) {
+        cnt += printf("]");
+    }
+
+    return cnt;
+}
+
+static void print_command_desc(const QemuArgContext *ctx,
+                               const QemuArgCommand *cmd, bool texi)
+{
+    QemuArgOpt *itr;
+
+    printf("%s", cmd->cmd);
+
+#define PRINT_OPT(_args)                                                \
+    for (itr = _args; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) { \
+        if (itr->type == QEMU_ARG_OPT_TYPE_DEPRECATED ||                \
+            itr->type == QEMU_ARG_OPT_TYPE_GROUP) {                     \
+            continue;                                                   \
+        }                                                               \
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL ||                \
+            itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {           \
+            format_positional_opt(itr, texi);                           \
+        } else {                                                        \
+            printf(" [");                                               \
+            format_short_opt(itr, ctx->decorate, texi);                 \
+            format_long_opt(itr, ctx->decorate, texi);                  \
+            printf("]");                                                \
+        }                                                               \
+    }                                                                   \
+
+    if (ctx->args) {
+        PRINT_OPT(ctx->args);
+    }
+
+    if (cmd->mutual_groups) {
+        format_mutual_opt(cmd->mutual_groups, ctx->decorate, texi);
+    }
+
+    if (cmd->args) {
+        PRINT_OPT(cmd->args);
+    }
+
+#undef PRINT_OPT
+}
+
+static void print_command_usage(const QemuArgContext *ctx,
+                                const QemuArgCommand *cmd)
+{
+    printf("usage:");
+
+    if (ctx->prog_name) {
+        printf(" %s ", ctx->prog_name);
+    }
+
+    print_command_desc(ctx, cmd, false);
+    printf("\n\n");
+}
+
+static bool command_has_positional(const QemuArgCommand *cmd)
+{
+    QemuArgOpt *itr;
+
+    if (!cmd->args) {
+        return false;
+    }
+
+    for (itr = cmd->args; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL ||
+            QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+void qemu_arg_print_command_help(const QemuArgContext *ctx,
+                                 const QemuArgCommand *cmd)
+{
+    QemuArgOpt **itr;
+    bool cmd_opts = false;
+
+    if (ctx->prologue) {
+        printf("%s\n\n", ctx->prologue);
+    }
+
+    print_command_usage(ctx, cmd);
+
+    printf("%s\n", "Common options:");
+    print_arg_list(_internal_arg_opts, 0);
+
+    if (ctx->args) {
+        print_arg_list(ctx->args, ctx->decorate);
+    }
+    printf("\n");
+
+    if (command_has_positional(cmd)) {
+        printf("%s\n", "Positional options:");
+        print_positional_arg_list(cmd->args);
+        printf("\n");
+    }
+
+    if (cmd->mutual_groups) {
+        printf("%s\n", "Command options:");
+
+        for (itr = cmd->mutual_groups; *itr; itr++) {
+            print_arg_list(*itr, ctx->decorate);
+        }
+        cmd_opts = true;
+    }
+
+    if (cmd->args) {
+        if (!cmd_opts) {
+            printf("%s\n", "Command options:");
+        }
+
+        if (cmd->args) {
+            print_arg_list(cmd->args, ctx->decorate);
+        }
+        cmd_opts = true;
+    }
+
+    if (cmd_opts) {
+        printf("\n");
+    }
+
+    if (ctx->epilogue) {
+        print_help_block(ctx->epilogue, 0, HELP_BLOCK_WIDTH);
+        printf("\n");
+    }
+}
+
+static void print_command_help_cb(const QemuArgContext *ctx, const void *opaque)
+{
+    qemu_arg_print_command_help(ctx, opaque);
+}
+
+static const QemuArgCommand *find_command(char *cmd, const QemuArgCommand *list)
+{
+    const QemuArgCommand *itr;
+
+    for (itr = list; itr->cmd; itr++) {
+        if (!strcmp(cmd, itr->cmd)) {
+            return itr;
+        }
+    }
+
+    return NULL;
+}
+
+static QemuArgOpt *find_long_opt(char *opt, char *end, QemuArgOpt *list)
+{
+    QemuArgOpt *itr;
+
+    if (!list) {
+        return NULL;
+    }
+
+    for (itr = list; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->long_name && !strncmp(itr->long_name, opt, end - opt)) {
+            return itr;
+        }
+    }
+
+    return NULL;
+}
+
+static QemuArgOpt *find_short_opt(char opt, QemuArgOpt *list)
+{
+    QemuArgOpt *itr;
+
+    if (!list) {
+        return NULL;
+    }
+
+    for (itr = list; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->short_name && itr->short_name == opt) {
+            return itr;
+        }
+    }
+
+    return NULL;
+}
+
+static QemuArgOpt *find_opt(char *opt, char *end, QemuArgOpt *list)
+{
+    char *long_opt, short_opt = 0;
+    size_t len;
+
+    len = strlen("--");
+    if (!strncmp(opt, "--", len)) {
+        long_opt = opt + len;
+        return find_long_opt(long_opt, end, list);
+    }
+
+    if (strlen(opt) == 2 && opt[0] == '-') {
+        short_opt = opt[1];
+        return find_short_opt(short_opt, list);
+    }
+
+    return NULL;
+}
+
+static QemuArgOpt *opt_get_next_positional(char *opt, QemuArgOpt *args)
+{
+    QemuArgOpt *itr;
+
+    for (itr = args; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            return itr;
+        }
+
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL && itr->value &&
+            !itr->value_set) {
+            return itr;
+        }
+    }
+
+    return NULL;
+}
+
+static bool opt_set_value(char *curr, char *next, int *cnt, QemuArgOpt *opt)
+{
+    if (opt->flags & ARG_FLAG_REQ_VALUE && !next) {
+        printf("argument %s requires a value\n", curr);
+        return false;
+    }
+
+    if (opt->value && opt->type == QEMU_ARG_OPT_TYPE_BOOL) {
+        *OPT_BOOL_VAL_GET(opt)->value = true;
+        opt->value_set = true;
+        return true;
+    }
+
+    if (opt->value && opt->type == QEMU_ARG_OPT_TYPE_INT) {
+        *OPT_INT_VAL_GET(opt)->value = atoi(next);
+        opt->value_set = true;
+        return true;
+    }
+
+    if (opt->value && opt->type == QEMU_ARG_OPT_TYPE_STR) {
+        if (opt->flags & ARG_FLAG_CUMULATE) {
+            char **str = OPT_STR_VAL_GET(opt)->value;
+            char *cp;
+
+            if (*str) {
+                cp = strdup(*str);
+                free(*str);
+
+                *str = calloc(1, strlen(cp) + strlen(next) + 1);
+                sprintf(*str, "%s,%s", cp, next);
+
+                free(cp);
+            } else {
+                *str = calloc(1, strlen(next) + 1);
+                sprintf(*str, "%s", next);
+            }
+        } else {
+            *OPT_STR_VAL_GET(opt)->value = next;
+        }
+
+        *cnt += 1;
+        opt->value_set = true;
+        return true;
+    }
+
+    if (opt->value && opt->type == QEMU_ARG_OPT_TYPE_POSITIONAL) {
+        *OPT_STR_VAL_GET(opt)->value = curr;
+        opt->value_set = true;
+        return true;
+    }
+
+    if (opt->value && opt->type == QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+        char **val;
+        size_t value_size;
+        int list_cnt;
+
+        val = *OPT_STR_LIST_VAL_GET(opt)->value;
+
+        list_cnt = OPT_STR_LIST_VAL_GET(opt)->list_cnt;
+        if (!list_cnt) {
+            val = realloc(val, sizeof(char *));
+        }
+
+        value_size = sizeof(val) + sizeof(char *);
+        val = realloc(val, value_size);
+
+        val[list_cnt] = (char *)curr;
+
+        list_cnt++;
+        *OPT_STR_LIST_VAL_GET(opt)->value = val;
+        OPT_STR_LIST_VAL_GET(opt)->list_cnt = list_cnt;
+        val[list_cnt] = NULL;
+
+        opt->value_set = true;
+        return true;
+    }
+
+    return true;
+}
+
+static QemuArgOpt *find_opt_set(QemuArgOpt *list, QemuArgOpt *opt)
+{
+    QemuArgOpt *itr;
+
+    for (itr = list; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->value_set && itr != opt) {
+            return itr;
+        }
+    }
+
+    return NULL;
+}
+
+static bool validate_required(const QemuArgContext *ctx, QemuArgOpt *opt)
+{
+    QemuArgOpt *itr;
+
+    for (itr = opt; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->flags & ARG_FLAG_REQUIRED && !itr->value_set) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+static void set_default_values(QemuArgOpt *opt)
+{
+    QemuArgOpt *itr;
+
+    for (itr = opt; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->value_set || !itr->value) {
+            continue;
+        }
+
+        if (itr->type == QEMU_ARG_OPT_TYPE_INT) {
+            *OPT_INT_VAL_GET(itr)->value = OPT_INT_VAL_GET(itr)->def_val;
+        } else if (itr->type == QEMU_ARG_OPT_TYPE_STR) {
+            *OPT_STR_VAL_GET(itr)->value = OPT_STR_VAL_GET(itr)->def_val;
+        } else if (itr->type == QEMU_ARG_OPT_TYPE_BOOL) {
+            OPT_BOOL_VAL_GET(itr)->value = &OPT_BOOL_VAL_GET(itr)->def_val;
+        }
+    }
+}
+
+static int arg_parse(int argc, char *argv[], int begin,
+                     const QemuArgContext *ctx, QemuArgOpt *args,
+                     QemuArgOpt **mutual_groups, HelpFunc cb,
+                     const void *opaque)
+{
+    int i;
+
+    for (i = begin; i < argc; i++) {
+        QemuArgOpt *opt, **itr, *mutual;
+        char *curr, *next, *curr_end, size_char;
+        bool positional, size;
+
+        size = false;
+        opt = NULL;
+        curr = argv[i];
+        curr_end = strchr(curr, '=');
+
+        if (!curr_end) {
+            curr_end = curr + strlen(curr);
+            next = argv[i + 1];
+        } else {
+            next = curr_end + 1;
+        }
+
+        if (!strncmp(curr, "-h", curr_end - curr) ||
+            !strncmp(curr, "--help", curr_end - curr)) {
+            cb(ctx, opaque);
+            return SUCCESS;
+        }
+
+        opt = find_opt(curr, curr_end, args);
+        if (!opt && mutual_groups) {
+            for (itr = mutual_groups; *itr; itr++) {
+                opt = find_opt(curr, curr_end, *itr);
+
+                if (opt) {
+                    mutual = find_opt_set(*itr, opt);
+                    if (mutual) {
+                        printf("the following arguments are mutually exclusive "
+                               "and should not be used in conjunction:\n");
+                        format_mutual_opt(itr, ctx->decorate, false);
+                        printf("\n");
+
+                        return ERR;
+                    }
+                }
+            }
+        }
+
+        if (opt && opt->type == QEMU_ARG_OPT_TYPE_DEPRECATED) {
+            printf("%s\n", opt->help);
+            return ERR;
+        }
+
+        size_char = curr[strlen(curr) - 1];
+
+        if (size_char == 'k' || size_char == 'K' || size_char == 'M' ||
+            size_char == 'G' || size_char == 'T' || size_char == 'P' ||
+            size_char == 'E') {
+            size = true;
+        }
+
+        /** not a valid --argument or not a size specifier (like qemu-img does
+            for resize i.e +1G or -1G) **/
+        positional = curr[0] != '-' || size;
+        if (!opt && !positional) {
+            printf("unknown option %s\n", curr);
+            return ERR;
+        } else if (positional) {
+            opt = opt_get_next_positional(curr, args);
+            if (!opt) {
+                continue;
+            }
+        }
+
+        if (opt->value_set && !(opt->flags & ARG_FLAG_CUMULATE) &&
+            opt->type != QEMU_ARG_OPT_TYPE_BOOL &&
+            opt->type != QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            printf("repeated argument, you can provide %s argument just"
+                   " once.\n", curr);
+            return ERR;
+        }
+
+        if (!opt_set_value(curr, next, &i, opt)) {
+            return ERR;
+        }
+    }
+
+    return 0;
+}
+
+static QemuArgOpt *merge_args(const QemuArgOpt *list, int list_cnt,
+                              const QemuArgOpt *list2, int list2_cnt)
+{
+  QemuArgOpt *tmp, *lst;
+  size_t list_size, list2_size;
+
+  if (list_cnt && list2_cnt) {
+    list_cnt--;
+  }
+
+  list_size = list_cnt * sizeof(QemuArgOpt);
+  list2_size = list2_cnt * sizeof(QemuArgOpt);
+
+  lst = calloc(1, list_size + list2_size);
+  lst = memcpy(lst, list, list_size);
+
+  tmp = lst + list_cnt;
+  tmp = memcpy(tmp, list2, list2_size);
+
+  return lst;
+}
+
+static void generate_hx(const QemuArgContext *ctx)
+{
+    const QemuArgCommand *itr;
+
+    printf("HXCOMM generated with %s generate-hx\n\n",
+           ctx->prog_name);
+
+    printf("STEXI\n"
+           "@table @option\n"
+           "ETEXI\n\n");
+
+    for (itr = ctx->commands; itr->cmd; itr++) {
+        printf("STEXI\n");
+        printf("@item ");
+        print_command_desc(ctx, itr, true);
+
+        if (!(itr + 1)->cmd) {
+            printf("\n@end table");
+        }
+
+        printf("\nETEXI\n\n");
+    }
+}
+
+static int command_parse(int argc, char *argv[], const QemuArgContext *ctx)
+{
+    const QemuArgCommand *cmd;
+    QemuArgOpt *args;
+    int ret;
+
+    ret = 0;
+
+    if (!strcmp(argv[1], "generate-hx")) {
+        generate_hx(ctx);
+        return SUCCESS;
+    }
+
+    cmd = find_command(argv[1], ctx->commands);
+    if (!cmd) {
+        print_help(ctx);
+        return ERR;
+    }
+
+    if (!cmd->args) {
+        return ERR;
+    }
+
+    args = merge_args(ctx->args, ctx->args_cnt, cmd->args, cmd->args_cnt);
+    ret = arg_parse(argc, argv, 2, ctx, args, cmd->mutual_groups,
+                    print_command_help_cb, cmd);
+
+    if (!validate_required(ctx, args)) {
+        qemu_arg_print_command_help(ctx, cmd);
+        goto err;
+    }
+
+    set_default_values(args);
+
+    if (cmd->cb) {
+        ret = cmd->cb(ctx, cmd);
+    }
+
+err:
+    free(args);
+    return ret;
+}
+
+static void print_help_cb(const QemuArgContext *ctx, const void *opaque)
+{
+    print_help(ctx);
+}
+
+static int arg_list_parse(int argc, char *argv[], const QemuArgContext *ctx)
+{
+    int ret = 0;
+
+    ret = arg_parse(argc, argv, 1, ctx, ctx->args, ctx->mutual_groups,
+                    print_help_cb, NULL);
+
+    if (!validate_required(ctx, ctx->args)) {
+        print_help(ctx);
+        return ERR;
+    }
+
+    set_default_values(ctx->args);
+
+    return ret;
+}
+
+int qemu_arg_parse(int argc, char *argv[], const QemuArgContext *ctx)
+{
+    if (argc == 1) {
+        print_help(ctx);
+        return ERR;
+    }
+
+    if (ctx->commands) {
+        return command_parse(argc, argv, ctx);
+    } else {
+        return arg_list_parse(argc, argv, ctx);
+    }
+}
+
+static void opt_cleanup(QemuArgOpt *opt)
+{
+    QemuArgOpt *itr;
+
+    for (itr = opt; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (!itr->value) {
+            continue;
+        }
+
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            free(*OPT_STR_LIST_VAL_GET(itr)->value);
+        } else if (itr->flags & ARG_FLAG_CUMULATE) {
+            free(*OPT_STR_VAL_GET(itr)->value);
+        }
+    }
+}
+
+void qemu_arg_context_cleanup(const QemuArgContext *ctx)
+{
+    const QemuArgCommand *itr;
+    QemuArgOpt **opt;
+
+    for (itr = ctx->commands; itr->cmd; itr++) {
+        if (!itr->args || !itr->mutual_groups) {
+            continue;
+        }
+
+        opt_cleanup(itr->args);
+
+        if (!itr->mutual_groups) {
+            continue;
+        }
+
+        for (opt = itr->mutual_groups; *opt; opt++) {
+            opt_cleanup(*opt);
+        }
+    }
+}
+
+int qemu_arg_get_command_positional_list_cnt(const QemuArgCommand *cmd)
+{
+    QemuArgOpt *itr;
+
+    if (!cmd) {
+        return 0;
+    }
+
+    for (itr = cmd->args; itr->type != QEMU_ARG_OPT_TYPE_SENTINEL; itr++) {
+        if (itr->type == QEMU_ARG_OPT_TYPE_POSITIONAL_LIST) {
+            return OPT_STR_LIST_VAL_GET(itr)->list_cnt;
+        }
+    }
+
+    return 0;
+}
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg
  2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 1/2] qemu-arg: introduce a " Leandro Dorileo
@ 2014-03-08 18:47 ` Leandro Dorileo
  2014-03-09  7:30   ` Paolo Bonzini
  2014-03-08 18:55 ` [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Peter Maydell
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-08 18:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Leandro Dorileo,
	Stefan Weil, Michael Tokarev, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek, Peter Lieven

Remove the arg parsing implementations using getopt and use qemu-arg.
Also remove the qemu-img-cmds.hx since it's now generated on building time,
adapted the build system to generate the .hx file using the qemu-img itself
using the qemu-arg internal command generate-hx.

Signed-off-by: Leandro Dorileo <l@dorileo.org>
---
 .gitignore       |    1 +
 Makefile         |   12 +-
 qemu-img-cmds.hx |   77 ----
 qemu-img-descs.h |  128 ++++++
 qemu-img.c       | 1184 ++++++++++++++++++------------------------------------
 5 files changed, 531 insertions(+), 871 deletions(-)
 delete mode 100644 qemu-img-cmds.hx
 create mode 100644 qemu-img-descs.h

diff --git a/.gitignore b/.gitignore
index ef7019f..700e92d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -116,3 +116,4 @@ cscope.*
 tags
 TAGS
 *~
+qemu-img-cmds.hx
diff --git a/Makefile b/Makefile
index bd9cd4f..0f2f102 100644
--- a/Makefile
+++ b/Makefile
@@ -214,8 +214,6 @@ util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
 
 ######################################################################
 
-qemu-img.o: qemu-img-cmds.h
-
 qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
@@ -225,9 +223,6 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
-qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
-	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
-
 qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
 qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 
@@ -272,7 +267,7 @@ clean:
 	rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
 	rm -f fsdev/*.pod
 	rm -rf .libs */.libs
-	rm -f qemu-img-cmds.h
+	rm -f qemu-img-cmds.hx
 	@# May not be present in GENERATED_HEADERS
 	rm -f trace/generated-tracers-dtrace.dtrace*
 	rm -f trace/generated-tracers-dtrace.h*
@@ -442,7 +437,10 @@ qemu-monitor.texi: $(SRC_PATH)/hmp-commands.hx
 qmp-commands.txt: $(SRC_PATH)/qmp-commands.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -q < $< > $@,"  GEN   $@")
 
-qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
+qemu-img-cmds.hx: qemu-img$(EXESUF)
+	$(call quiet-command,$(SRC_PATH)/qemu-img generate-hx -h > $(SRC_PATH)/$@,"  GEN   $@")
+
+qemu-img-cmds.texi: qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"  GEN   $@")
 
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
deleted file mode 100644
index d029609..0000000
--- a/qemu-img-cmds.hx
+++ /dev/null
@@ -1,77 +0,0 @@
-HXCOMM Use DEFHEADING() to define headings in both help text and texi
-HXCOMM Text between STEXI and ETEXI are copied to texi version and
-HXCOMM discarded from C version
-HXCOMM DEF(command, callback, arg_string) is used to construct
-HXCOMM command structures and help message.
-HXCOMM HXCOMM can be used for comments, discarded from both texi and C
-
-STEXI
-@table @option
-ETEXI
-
-DEF("check", img_check,
-    "check [-q] [-f fmt] [--output=ofmt]  [-r [leaks | all]] filename")
-STEXI
-@item check [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename}
-ETEXI
-
-DEF("create", img_create,
-    "create [-q] [-f fmt] [-o options] filename [size]")
-STEXI
-@item create [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
-ETEXI
-
-DEF("commit", img_commit,
-    "commit [-q] [-f fmt] [-t cache] filename")
-STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
-ETEXI
-
-DEF("compare", img_compare,
-    "compare [-f fmt] [-F fmt] [-p] [-q] [-s] filename1 filename2")
-STEXI
-@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-q] [-s] @var{filename1} @var{filename2}
-ETEXI
-
-DEF("convert", img_convert,
-    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
-STEXI
-@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
-ETEXI
-
-DEF("info", img_info,
-    "info [-f fmt] [--output=ofmt] [--backing-chain] filename")
-STEXI
-@item info [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
-ETEXI
-
-DEF("map", img_map,
-    "map [-f fmt] [--output=ofmt] filename")
-STEXI
-@item map [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
-ETEXI
-
-DEF("snapshot", img_snapshot,
-    "snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
-STEXI
-@item snapshot [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
-ETEXI
-
-DEF("rebase", img_rebase,
-    "rebase [-q] [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
-STEXI
-@item rebase [-q] [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
-ETEXI
-
-DEF("resize", img_resize,
-    "resize [-q] filename [+ | -]size")
-STEXI
-@item resize [-q] @var{filename} [+ | -]@var{size}
-ETEXI
-
-DEF("amend", img_amend,
-    "amend [-q] [-f fmt] -o options filename")
-STEXI
-@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename}
-@end table
-ETEXI
diff --git a/qemu-img-descs.h b/qemu-img-descs.h
new file mode 100644
index 0000000..5725161
--- /dev/null
+++ b/qemu-img-descs.h
@@ -0,0 +1,128 @@
+#ifndef _QEMU_IMG_DESCS_H_
+#define _QEMU_IMG_DESCS_H_
+
+#define OPT_POS_FILENAME_HELP                   \
+    "the specified disk image file name"        \
+
+#define OPT_POS_SIZE_HELP                                               \
+    "the disk image size in bytes. Optional suffixes 'k' or 'K' "       \
+    "(kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M), "  \
+    "'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E'"              \
+    " (exabyte, 1024P)  are supported. 'b' is ignored"                 \
+
+#define OPT_INC_SHIK_HELP                                               \
+    "grow or shrink the image size i.e +1G to increase it 1G or -1G "   \
+    "to shirink it 1G"                                                  \
+
+#define OPT_BACKING_FILE_HELP                   \
+    "the backing file name"                     \
+
+#define OPT_BACKING_FMT_HELP "the backing file format"
+
+#define OPT_FMT_HELP                                                    \
+    "the disk image format. It is automatically guessed in most cases"  \
+
+#define OPT_SPARSE_SIZE_HELP                                            \
+    "indicates the consecutive number of bytes that must contain only " \
+    "zeros for qemu-img to create a sparse image during conversion. "   \
+    "This value is rounded down to the nearest 512 bytes. You may"      \
+    "use the common size suffixes like \"k\" for kilobytes."            \
+
+#define OPT_SNAPSHOT_ID_NAME_HELP "a snapshot id or name"
+
+#define OPT_OPTIONS                                                     \
+    "a comma separated list of format specific options "                \
+    "in a name=value format. Use -o ? for an overview of the options "  \
+    "supported by the used format"                                      \
+
+#define OPT_CACHE_HELP                                                  \
+    "the cache mode used to write the output disk image, the valid "    \
+    "options are: 'none', 'writeback' (default, except for convert), "  \
+    "'writethrough', 'directsync' and 'unsafe' (default for convert)"   \
+
+
+#define OPT_COMPRESS_HELP                                               \
+    "indicates that target image must be compressed (qcow format only)" \
+
+#define OPT_OFMT_HELP                           \
+    "the output format"                         \
+
+#define OPT_OUT_BASE_IMG_HELP                   \
+    "the output base image"                     \
+
+#define OPT_SNAPSHOT_OPT_HELP                   \
+    "snapshot option like snapshot.*"           \
+
+#define OPT_OUTPUT_HELP                                                 \
+    "takes the format in which the output must be "                     \
+    "done (human or json)"                                              \
+
+#define OPT_BACKING_CHAIN_HELP                                          \
+    "will enumerate information about backing files in a disk"          \
+    "image chain"                                                       \
+
+#define OPT_QUIET_HELP                                                  \
+    "use Quiet mode - do not print any output (except errors)"          \
+
+#define OPT_REPAIR_HELP                                                 \
+    "tries to repair any inconsistencies that are found during "        \
+    "the check. '-r leaks' repairs only cluster leaks, whereas "        \
+    "'-r all' fixes all kinds of errors, with a higher risk of "        \
+    "choosing the wrong fix or hiding corruption that has "             \
+    "already occurred."                                                 \
+
+#define OPT_FMT2_HELP                           \
+    "second image format"                       \
+
+#define OPT_PROGRESS_HELP                       \
+    "show progress of command"                  \
+
+#define OPT_STRICT_HELP                                                 \
+    "run in Strict mode - fail on different image size or "             \
+    "sector allocation"                                                 \
+
+#define OPT_SKIP_HELP                                                 \
+    "skips the target volume creation (useful if the volume "         \
+    "is created prior to running qemu-img)"                           \
+
+#define OPT_SNAPSHOT_PARAM_HELP                 \
+    "the snapshot params"                       \
+
+#define OPT_SNAPSHOT_LIST_HELP                  \
+    "lists all snapshots in the given image"    \
+
+#define OPT_SNAPSHOT_APPLY_HELP                         \
+    "applies a snapshot (revert disk to saved state)"   \
+
+#define OPT_SNAPSHOT_CREATE_HELP                \
+    "create a snapshot"                         \
+
+#define OPT_SNAPSHOT_DELETE_HELP                \
+    "delete a snapshot"                         \
+
+#define OPT_UNSAFE_HELP                                                 \
+    "enables unsafe rebasing. It is assumed that old "                  \
+    "and new backing file match exactly. The image doesn't "            \
+    "need a working backing file before rebasing in this case "         \
+    "(useful for renaming the backing file) "                           \
+
+#define OPT_CREATE_E_DEP_HELP                                           \
+    "option -e is deprecated, please use \'-o encryption\' instead"     \
+
+#define OPT_CREATE_6_DEP_HELP                                           \
+    "option -6 is deprecated, please use \'-o compat6\' instead"        \
+
+#define QEMU_IMG_CHECK_HELP "check image integrity/consistency"
+#define QEMU_IMG_CREATE_HELP "create a new image file"
+#define QEMU_IMG_COMMIT_HELP "commit COW file into raw image"
+#define QEMU_IMG_COMPARE_HELP "compare 2 image files"
+#define QEMU_IMG_CONVERT_HELP "convert N image files or snapshot to"    \
+    " a new format"                                                     \
+#define QEMU_IMG_INFO_HELP "show image file informations"
+#define QEMU_IMG_MAP_HELP "dump the image file metadata"
+#define QEMU_IMG_SNAPSHOT_HELP "list, apply, create or delete snapshots"
+#define QEMU_IMG_REBASE_HELP "changes the backing file of an image"
+#define QEMU_IMG_RESIZE_HELP "resize an image file"
+#define QEMU_IMG_AMEND_HELP "amend the image format"
+
+#endif /* _QEMU_IMG_DESCS_H_ */
diff --git a/qemu-img.c b/qemu-img.c
index 2e40cc1..bafe439 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -31,6 +31,8 @@
 #include "sysemu/sysemu.h"
 #include "block/block_int.h"
 #include "block/qapi.h"
+#include "qemu/qemu-arg.h"
+#include "qemu-img-descs.h"
 #include <getopt.h>
 
 typedef struct img_cmd_t {
@@ -52,85 +54,151 @@ typedef enum OutputFormat {
 #define BDRV_O_FLAGS BDRV_O_CACHE_WB
 #define BDRV_DEFAULT_CACHE "writeback"
 
-static void format_print(void *opaque, const char *name)
-{
-    printf(" %s", name);
-}
+static bool _quiet, _progress, _strict, _skip_create, _backing_chain;
+static bool _snapshot_list, _unsafe, _compress;
+
+static char *_filename, *_filename2, *_fmt, *_ofmt, *_fmt2;
+static char *_rep, *_options, *_size, *_cache, *_sparse_size;
+static char *_snapshot_name, *_snapshot_apply, *_snapshot_create;
+static char *_snapshot_delete;
+static char *_backing_file, *_backing_fmt, *_output, *_out_baseimg;
+static char *_out_basefmt, **_filename_list;
+
+#define PROLOGUE                                                        \
+    "qemu-img version "QEMU_VERSION", Copyright (c) 2004-2008 "         \
+    "Fabrice Bellard\nQEMU disk image utility"                          \
+
+static QemuArgOpt _check_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_STR('f', "format", "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR(0, "output", "ofmt", OPT_OUTPUT_HELP, &_output, "human"),
+    OPT_STR('r', "repair", "[leaks | all]", OPT_REPAIR_HELP, &_rep, "all"),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
 
-/* Please keep in synch with qemu-img.texi */
-static void help(void)
-{
-    const char *help_msg =
-           "qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice Bellard\n"
-           "usage: qemu-img command [command options]\n"
-           "QEMU disk image utility\n"
-           "\n"
-           "Command syntax:\n"
-#define DEF(option, callback, arg_string)        \
-           "  " arg_string "\n"
-#include "qemu-img-cmds.h"
-#undef DEF
-#undef GEN_DOCS
-           "\n"
-           "Command parameters:\n"
-           "  'filename' is a disk image filename\n"
-           "  'fmt' is the disk image format. It is guessed automatically in most cases\n"
-           "  'cache' is the cache mode used to write the output disk image, the valid\n"
-           "    options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n"
-           "    'directsync' and 'unsafe' (default for convert)\n"
-           "  'size' is the disk image size in bytes. Optional suffixes\n"
-           "    'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n"
-           "    'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P)  are\n"
-           "    supported. 'b' is ignored.\n"
-           "  'output_filename' is the destination disk image filename\n"
-           "  'output_fmt' is the destination format\n"
-           "  'options' is a comma separated list of format specific options in a\n"
-           "    name=value format. Use -o ? for an overview of the options supported by the\n"
-           "    used format\n"
-           "  'snapshot_param' is param used for internal snapshot, format\n"
-           "    is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
-           "    '[ID_OR_NAME]'\n"
-           "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
-           "    instead\n"
-           "  '-c' indicates that target image must be compressed (qcow format only)\n"
-           "  '-u' enables unsafe rebasing. It is assumed that old and new backing file\n"
-           "       match exactly. The image doesn't need a working backing file before\n"
-           "       rebasing in this case (useful for renaming the backing file)\n"
-           "  '-h' with or without a command shows this help and lists the supported formats\n"
-           "  '-p' show progress of command (only certain commands)\n"
-           "  '-q' use Quiet mode - do not print any output (except errors)\n"
-           "  '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n"
-           "       contain only zeros for qemu-img to create a sparse image during\n"
-           "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
-           "       unallocated or zero sectors, and the destination image will always be\n"
-           "       fully allocated\n"
-           "  '--output' takes the format in which the output must be done (human or json)\n"
-           "  '-n' skips the target volume creation (useful if the volume is created\n"
-           "       prior to running qemu-img)\n"
-           "\n"
-           "Parameters to check subcommand:\n"
-           "  '-r' tries to repair any inconsistencies that are found during the check.\n"
-           "       '-r leaks' repairs only cluster leaks, whereas '-r all' fixes all\n"
-           "       kinds of errors, with a higher risk of choosing the wrong fix or\n"
-           "       hiding corruption that has already occurred.\n"
-           "\n"
-           "Parameters to snapshot subcommand:\n"
-           "  'snapshot' is the name of the snapshot to create, apply or delete\n"
-           "  '-a' applies a snapshot (revert disk to saved state)\n"
-           "  '-c' creates a snapshot\n"
-           "  '-d' deletes a snapshot\n"
-           "  '-l' lists all snapshots in the given image\n"
-           "\n"
-           "Parameters to compare subcommand:\n"
-           "  '-f' first image format\n"
-           "  '-F' second image format\n"
-           "  '-s' run in Strict mode - fail on different image size or sector allocation\n";
-
-    printf("%s\nSupported formats:", help_msg);
-    bdrv_iterate_format(format_print, NULL);
-    printf("\n");
-    exit(1);
-}
+static QemuArgOpt _create_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, NULL),
+    OPT_STR('F', NULL, "fmt", OPT_BACKING_FMT_HELP, &_backing_fmt, NULL),
+    OPT_STR('b', NULL, "filename", OPT_BACKING_FILE_HELP, &_backing_file, NULL),
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, "raw"),
+    OPT_CUMUL_STR('o', NULL, "options", OPT_OPTIONS, &_options, NULL),
+    OPT_DEP('e', NULL, OPT_CREATE_E_DEP_HELP),
+    OPT_DEP('6', NULL, OPT_CREATE_6_DEP_HELP),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_POS_REQ("size", OPT_POS_SIZE_HELP, &_size, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _commit_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR('t', NULL, "cache", OPT_CACHE_HELP, &_cache, BDRV_DEFAULT_CACHE),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _compare_args[] = {
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR('F', NULL, "fmt", OPT_FMT2_HELP, &_fmt2, NULL),
+    OPT_BOOL('p', NULL, OPT_PROGRESS_HELP, &_progress, false),
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_BOOL('s', NULL, OPT_STRICT_HELP, &_strict, false),
+    OPT_POS_REQ("filename1", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_POS_REQ("filename2", OPT_POS_FILENAME_HELP, &_filename2, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _convert_args[] = {
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR('O', NULL, "output_fmt", OPT_OFMT_HELP, &_ofmt, "raw"),
+    OPT_STR('B', NULL, "output_base_img", OPT_OUT_BASE_IMG_HELP,
+            &_out_baseimg, NULL),
+    OPT_BOOL('c', NULL, OPT_COMPRESS_HELP, &_compress, false),
+    OPT_DEP('e', NULL, OPT_CREATE_E_DEP_HELP),
+    OPT_DEP('6', NULL, OPT_CREATE_6_DEP_HELP),
+    OPT_CUMUL_STR('o', NULL, "options", OPT_OPTIONS, &_options, NULL),
+    OPT_CUMUL_STR('s', NULL, "snapshot_name", OPT_SNAPSHOT_ID_NAME_HELP,
+                  &_snapshot_name, NULL),
+    OPT_CUMUL_STR('l', NULL, "snapshot_opt", OPT_SNAPSHOT_OPT_HELP,
+                  &_snapshot_name, NULL),
+    OPT_CUMUL_STR('S', NULL, "sparse_size", OPT_SPARSE_SIZE_HELP, &_sparse_size,
+                  NULL),
+    OPT_BOOL('p', NULL, OPT_PROGRESS_HELP, &_progress, false),
+    OPT_STR('t', NULL, "cache", OPT_CACHE_HELP, &_cache, "unsafe"),
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_BOOL('n', NULL, OPT_SKIP_HELP, &_skip_create, false),
+    OPT_POS_LIST("filename [filename2 [...]] output_filename",
+                 OPT_POS_FILENAME_HELP, &_filename_list, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _info_args[] = {
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR(0, "output", "ofmt", OPT_OUTPUT_HELP, &_ofmt, NULL),
+    OPT_BOOL(0, "backing-chain", OPT_BACKING_CHAIN_HELP, &_backing_chain,
+             false),
+    OPT_POS("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _map_args[] = {
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR(0, "output", "ofmt", OPT_OUTPUT_HELP, &_ofmt, NULL),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _snapshot_action_args[] = {
+    OPT_BOOL('l', NULL, OPT_SNAPSHOT_LIST_HELP, &_snapshot_list, false),
+    OPT_STR('a', NULL, "snapshot", OPT_SNAPSHOT_APPLY_HELP,
+            &_snapshot_apply, NULL),
+    OPT_STR('c', NULL, "snapshot", OPT_SNAPSHOT_CREATE_HELP, &_snapshot_create,
+            NULL),
+    OPT_STR('d', NULL, "snapshot", OPT_SNAPSHOT_DELETE_HELP, &_snapshot_delete,
+            NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _snapshot_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt *_snapshot_mutual_groups[] = {
+    _snapshot_action_args,
+    OPT_MUTUAL_GROUP_SENTINEL
+};
+
+static QemuArgOpt _rebase_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_STR('t', NULL, "cache", OPT_CACHE_HELP, &_cache, BDRV_DEFAULT_CACHE),
+    OPT_BOOL('p', NULL, OPT_PROGRESS_HELP, &_progress, false),
+    OPT_BOOL('u', NULL, OPT_UNSAFE_HELP, &_unsafe, false),
+    OPT_STR('b', NULL, "backing_file", OPT_BACKING_FILE_HELP, &_backing_file,
+            NULL),
+    OPT_STR('F', NULL, "backing_fmt", OPT_BACKING_FMT_HELP, &_out_basefmt,
+            NULL),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _resize_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_POS_REQ("[+ | -]size", OPT_INC_SHIK_HELP, &_size, NULL),
+    OPT_SENTINEL
+};
+
+static QemuArgOpt _amend_args[] = {
+    OPT_BOOL('q', NULL, OPT_QUIET_HELP, &_quiet, false),
+    OPT_STR('f', NULL, "fmt", OPT_FMT_HELP, &_fmt, NULL),
+    OPT_CUMUL_STR_REQ('o', NULL, "options", OPT_OPTIONS, &_options, NULL),
+    OPT_POS_REQ("filename", OPT_POS_FILENAME_HELP, &_filename, NULL),
+    OPT_SENTINEL
+};
 
 static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
 {
@@ -332,81 +400,19 @@ static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
     return 0;
 }
 
-static int img_create(int argc, char **argv)
+static int img_create(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c;
     uint64_t img_size = -1;
-    const char *fmt = "raw";
-    const char *base_fmt = NULL;
-    const char *filename;
-    const char *base_filename = NULL;
-    char *options = NULL;
     Error *local_err = NULL;
-    bool quiet = false;
 
-    for(;;) {
-        c = getopt(argc, argv, "F:b:f:he6o:q");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'F':
-            base_fmt = optarg;
-            break;
-        case 'b':
-            base_filename = optarg;
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 'e':
-            error_report("option -e is deprecated, please use \'-o "
-                  "encryption\' instead!");
-            goto fail;
-        case '6':
-            error_report("option -6 is deprecated, please use \'-o "
-                  "compat6\' instead!");
-            goto fail;
-        case 'o':
-            if (!is_valid_option_list(optarg)) {
-                error_report("Invalid option list: %s", optarg);
-                goto fail;
-            }
-            if (!options) {
-                options = g_strdup(optarg);
-            } else {
-                char *old_options = options;
-                options = g_strdup_printf("%s,%s", options, optarg);
-                g_free(old_options);
-            }
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        }
-    }
-
-    /* Get the filename */
-    filename = (optind < argc) ? argv[optind] : NULL;
-    if (options && has_help_option(options)) {
-        g_free(options);
-        return print_block_option_help(filename, fmt);
+    if (_options && has_help_option(_options)) {
+        return print_block_option_help(_filename, _fmt);
     }
 
-    if (optind >= argc) {
-        help();
-    }
-    optind++;
-
-    /* Get image size, if specified */
-    if (optind < argc) {
+    if (_size) {
         int64_t sval;
         char *end;
-        sval = strtosz_suffix(argv[optind++], &end, STRTOSZ_DEFSUFFIX_B);
+        sval = strtosz_suffix(_size, &end, STRTOSZ_DEFSUFFIX_B);
         if (sval < 0 || *end) {
             if (sval == -ERANGE) {
                 error_report("Image size must be less than 8 EiB!");
@@ -420,23 +426,18 @@ static int img_create(int argc, char **argv)
         }
         img_size = (uint64_t)sval;
     }
-    if (optind != argc) {
-        help();
-    }
 
-    bdrv_img_create(filename, fmt, base_filename, base_fmt,
-                    options, img_size, BDRV_O_FLAGS, &local_err, quiet);
+    bdrv_img_create(_filename, _fmt, _backing_file, _backing_fmt,
+                    _options, img_size, BDRV_O_FLAGS, &local_err, _quiet);
     if (local_err) {
-        error_report("%s: %s", filename, error_get_pretty(local_err));
+        error_report("%s: %s", _filename, error_get_pretty(local_err));
         error_free(local_err);
         goto fail;
     }
 
-    g_free(options);
     return 0;
 
 fail:
-    g_free(options);
     return 1;
 }
 
@@ -547,81 +548,44 @@ static int collect_image_check(BlockDriverState *bs,
  * 2 - Check completed, image is corrupted
  * 3 - Check completed, image has leaked clusters, but is good otherwise
  */
-static int img_check(int argc, char **argv)
+static int img_check(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c, ret;
+    int ret;
     OutputFormat output_format = OFORMAT_HUMAN;
-    const char *filename, *fmt, *output;
     BlockDriverState *bs;
     int fix = 0;
     int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
     ImageCheck *check;
-    bool quiet = false;
 
-    fmt = NULL;
-    output = NULL;
-    for(;;) {
-        int option_index = 0;
-        static const struct option long_options[] = {
-            {"help", no_argument, 0, 'h'},
-            {"format", required_argument, 0, 'f'},
-            {"repair", no_argument, 0, 'r'},
-            {"output", required_argument, 0, OPTION_OUTPUT},
-            {0, 0, 0, 0}
-        };
-        c = getopt_long(argc, argv, "f:hr:q",
-                        long_options, &option_index);
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 'r':
-            flags |= BDRV_O_RDWR;
-
-            if (!strcmp(optarg, "leaks")) {
-                fix = BDRV_FIX_LEAKS;
-            } else if (!strcmp(optarg, "all")) {
-                fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
-            } else {
-                help();
-            }
-            break;
-        case OPTION_OUTPUT:
-            output = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
+    if (_rep) {
+        flags |= BDRV_O_RDWR;
+        if (!strcmp(_rep, "leaks")) {
+            fix = BDRV_FIX_LEAKS;
+        } else if (!strcmp(_rep, "all")) {
+            fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS;
+        } else {
+            error_report("[-r | --repair] must be used with leak or all as "
+                         "argument.");
+            return -1;
         }
     }
-    if (optind != argc - 1) {
-        help();
-    }
-    filename = argv[optind++];
 
-    if (output && !strcmp(output, "json")) {
+    if (!strcmp(_output, "json")) {
         output_format = OFORMAT_JSON;
-    } else if (output && !strcmp(output, "human")) {
+    } else if (!strcmp(_output, "human")) {
         output_format = OFORMAT_HUMAN;
-    } else if (output) {
+    } else if (_output) {
         error_report("--output must be used with human or json as argument.");
-        return 1;
+        return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+    bs = bdrv_new_open(_filename, _fmt, flags, true, _quiet);
     if (!bs) {
-        return 1;
+        return -1;
     }
 
     check = g_new0(ImageCheck, 1);
-    ret = collect_image_check(bs, check, filename, fmt, fix);
+    ret = collect_image_check(bs, check, _filename, _fmt, fix);
 
     if (ret == -ENOTSUP) {
         if (output_format == OFORMAT_HUMAN) {
@@ -638,7 +602,7 @@ static int img_check(int argc, char **argv)
         corruptions_fixed   = check->corruptions_fixed;
 
         if (output_format == OFORMAT_HUMAN) {
-            qprintf(quiet,
+            qprintf(_quiet,
                     "The following inconsistencies were found and repaired:\n\n"
                     "    %" PRId64 " leaked clusters\n"
                     "    %" PRId64 " corruptions\n\n"
@@ -647,7 +611,7 @@ static int img_check(int argc, char **argv)
                     check->corruptions_fixed);
         }
 
-        ret = collect_image_check(bs, check, filename, fmt, 0);
+        ret = collect_image_check(bs, check, _filename, _fmt, 0);
 
         check->leaks_fixed          = leaks_fixed;
         check->corruptions_fixed    = corruptions_fixed;
@@ -655,10 +619,10 @@ static int img_check(int argc, char **argv)
 
     switch (output_format) {
     case OFORMAT_HUMAN:
-        dump_human_image_check(check, quiet);
+        dump_human_image_check(check, _quiet);
         break;
     case OFORMAT_JSON:
-        dump_json_image_check(check, quiet);
+        dump_json_image_check(check, _quiet);
         break;
     }
 
@@ -682,56 +646,26 @@ fail:
     return ret;
 }
 
-static int img_commit(int argc, char **argv)
+static int img_commit(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c, ret, flags;
-    const char *filename, *fmt, *cache;
+    int ret, flags;
     BlockDriverState *bs;
-    bool quiet = false;
-
-    fmt = NULL;
-    cache = BDRV_DEFAULT_CACHE;
-    for(;;) {
-        c = getopt(argc, argv, "f:ht:q");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 't':
-            cache = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        }
-    }
-    if (optind != argc - 1) {
-        help();
-    }
-    filename = argv[optind++];
 
     flags = BDRV_O_RDWR;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    ret = bdrv_parse_cache_flags(_cache, &flags);
     if (ret < 0) {
-        error_report("Invalid cache option: %s", cache);
+        error_report("Invalid cache option: %s", _cache);
         return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+    bs = bdrv_new_open(_filename, _fmt, flags, true, _quiet);
     if (!bs) {
         return 1;
     }
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
-        qprintf(quiet, "Image committed.\n");
+        qprintf(_quiet, "Image committed.\n");
         break;
     case -ENOENT:
         error_report("No disk inserted");
@@ -907,76 +841,39 @@ static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
  * 1 - Images differ
  * >1 - Error occurred
  */
-static int img_compare(int argc, char **argv)
+static int img_compare(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    const char *fmt1 = NULL, *fmt2 = NULL, *filename1, *filename2;
     BlockDriverState *bs1, *bs2;
     int64_t total_sectors1, total_sectors2;
     uint8_t *buf1 = NULL, *buf2 = NULL;
     int pnum1, pnum2;
     int allocated1, allocated2;
     int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */
-    bool progress = false, quiet = false, strict = false;
     int64_t total_sectors;
     int64_t sector_num = 0;
     int64_t nb_sectors;
-    int c, pnum;
+    int pnum;
     uint64_t bs_sectors;
     uint64_t progress_base;
 
-    for (;;) {
-        c = getopt(argc, argv, "hpf:F:sq");
-        if (c == -1) {
-            break;
-        }
-        switch (c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt1 = optarg;
-            break;
-        case 'F':
-            fmt2 = optarg;
-            break;
-        case 'p':
-            progress = true;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        case 's':
-            strict = true;
-            break;
-        }
-    }
-
     /* Progress is not shown in Quiet mode */
-    if (quiet) {
-        progress = false;
-    }
-
-
-    if (optind != argc - 2) {
-        help();
+    if (_quiet) {
+        _progress = false;
     }
-    filename1 = argv[optind++];
-    filename2 = argv[optind++];
 
     /* Initialize before goto out */
-    qemu_progress_init(progress, 2.0);
+    qemu_progress_init(_progress, 2.0);
 
-    bs1 = bdrv_new_open(filename1, fmt1, BDRV_O_FLAGS, true, quiet);
+    bs1 = bdrv_new_open(_filename, _fmt, BDRV_O_FLAGS, true, _quiet);
     if (!bs1) {
-        error_report("Can't open file %s", filename1);
+        error_report("Can't open file %s", _filename);
         ret = 2;
         goto out3;
     }
 
-    bs2 = bdrv_new_open(filename2, fmt2, BDRV_O_FLAGS, true, quiet);
+    bs2 = bdrv_new_open(_filename2, _fmt2, BDRV_O_FLAGS, true, _quiet);
     if (!bs2) {
-        error_report("Can't open file %s", filename2);
+        error_report("Can't open file %s", _filename2);
         ret = 2;
         goto out2;
     }
@@ -992,9 +889,9 @@ static int img_compare(int argc, char **argv)
 
     qemu_progress_print(0, 100);
 
-    if (strict && total_sectors1 != total_sectors2) {
+    if (_strict && total_sectors1 != total_sectors2) {
         ret = 1;
-        qprintf(quiet, "Strict mode: Image size mismatch!\n");
+        qprintf(_quiet, "Strict mode: Image size mismatch!\n");
         goto out;
     }
 
@@ -1007,7 +904,7 @@ static int img_compare(int argc, char **argv)
                                              &pnum1);
         if (allocated1 < 0) {
             ret = 3;
-            error_report("Sector allocation test failed for %s", filename1);
+            error_report("Sector allocation test failed for %s", _filename);
             goto out;
         }
 
@@ -1015,7 +912,7 @@ static int img_compare(int argc, char **argv)
                                              &pnum2);
         if (allocated2 < 0) {
             ret = 3;
-            error_report("Sector allocation test failed for %s", filename2);
+            error_report("Sector allocation test failed for %s", _filename2);
             goto out;
         }
         nb_sectors = MIN(pnum1, pnum2);
@@ -1025,7 +922,7 @@ static int img_compare(int argc, char **argv)
                 ret = bdrv_read(bs1, sector_num, buf1, nb_sectors);
                 if (ret < 0) {
                     error_report("Error while reading offset %" PRId64 " of %s:"
-                                 " %s", sectors_to_bytes(sector_num), filename1,
+                                 " %s", sectors_to_bytes(sector_num), _filename,
                                  strerror(-ret));
                     ret = 4;
                     goto out;
@@ -1034,13 +931,13 @@ static int img_compare(int argc, char **argv)
                 if (ret < 0) {
                     error_report("Error while reading offset %" PRId64
                                  " of %s: %s", sectors_to_bytes(sector_num),
-                                 filename2, strerror(-ret));
+                                 _filename2, strerror(-ret));
                     ret = 4;
                     goto out;
                 }
                 ret = compare_sectors(buf1, buf2, nb_sectors, &pnum);
                 if (ret || pnum != nb_sectors) {
-                    qprintf(quiet, "Content mismatch at offset %" PRId64 "!\n",
+                    qprintf(_quiet, "Content mismatch at offset %" PRId64 "!\n",
                             sectors_to_bytes(
                                 ret ? sector_num : sector_num + pnum));
                     ret = 1;
@@ -1048,9 +945,9 @@ static int img_compare(int argc, char **argv)
                 }
             }
         } else {
-            if (strict) {
+            if (_strict) {
                 ret = 1;
-                qprintf(quiet, "Strict mode: Offset %" PRId64
+                qprintf(_quiet, "Strict mode: Offset %" PRId64
                         " allocation mismatch!\n",
                         sectors_to_bytes(sector_num));
                 goto out;
@@ -1058,10 +955,10 @@ static int img_compare(int argc, char **argv)
 
             if (allocated1) {
                 ret = check_empty_sectors(bs1, sector_num, nb_sectors,
-                                          filename1, buf1, quiet);
+                                          _filename, buf1, _quiet);
             } else {
                 ret = check_empty_sectors(bs2, sector_num, nb_sectors,
-                                          filename2, buf1, quiet);
+                                          _filename2, buf1, _quiet);
             }
             if (ret) {
                 if (ret < 0) {
@@ -1081,15 +978,15 @@ static int img_compare(int argc, char **argv)
         int64_t total_sectors_over;
         const char *filename_over;
 
-        qprintf(quiet, "Warning: Image size mismatch!\n");
+        qprintf(_quiet, "Warning: Image size mismatch!\n");
         if (total_sectors1 > total_sectors2) {
             total_sectors_over = total_sectors1;
             bs_over = bs1;
-            filename_over = filename1;
+            filename_over = _filename;
         } else {
             total_sectors_over = total_sectors2;
             bs_over = bs2;
-            filename_over = filename2;
+            filename_over = _filename2;
         }
 
         for (;;) {
@@ -1109,7 +1006,7 @@ static int img_compare(int argc, char **argv)
             nb_sectors = pnum;
             if (ret) {
                 ret = check_empty_sectors(bs_over, sector_num, nb_sectors,
-                                          filename_over, buf1, quiet);
+                                          filename_over, buf1, _quiet);
                 if (ret) {
                     if (ret < 0) {
                         error_report("Error while reading offset %" PRId64
@@ -1125,7 +1022,7 @@ static int img_compare(int argc, char **argv)
         }
     }
 
-    qprintf(quiet, "Images are identical.\n");
+    qprintf(_quiet, "Images are identical.\n");
     ret = 0;
 
 out:
@@ -1139,12 +1036,13 @@ out3:
     return ret;
 }
 
-static int img_convert(int argc, char **argv)
+static int img_convert(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c, n, n1, bs_n, bs_i, compress, cluster_sectors, skip_create;
+    int n, n1, bs_n, bs_i, cluster_sectors;
     int64_t ret = 0;
-    int progress = 0, flags;
-    const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
+    int flags;
+    char **curr;
+    const char *out_filename;
     BlockDriver *drv, *proto_drv;
     BlockDriverState **bs = NULL, *out_bs = NULL;
     int64_t total_sectors, nb_sectors, sector_num, bs_offset;
@@ -1155,131 +1053,57 @@ static int img_convert(int argc, char **argv)
     BlockDriverInfo bdi;
     QEMUOptionParameter *param = NULL, *create_options = NULL;
     QEMUOptionParameter *out_baseimg_param;
-    char *options = NULL;
-    const char *snapshot_name = NULL;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
-    bool quiet = false;
     Error *local_err = NULL;
     QemuOpts *sn_opts = NULL;
 
-    fmt = NULL;
-    out_fmt = "raw";
-    cache = "unsafe";
-    out_baseimg = NULL;
-    compress = 0;
-    skip_create = 0;
-    for(;;) {
-        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnl:");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 'O':
-            out_fmt = optarg;
-            break;
-        case 'B':
-            out_baseimg = optarg;
-            break;
-        case 'c':
-            compress = 1;
-            break;
-        case 'e':
-            error_report("option -e is deprecated, please use \'-o "
-                  "encryption\' instead!");
-            ret = -1;
-            goto fail_getopt;
-        case '6':
-            error_report("option -6 is deprecated, please use \'-o "
-                  "compat6\' instead!");
-            ret = -1;
-            goto fail_getopt;
-        case 'o':
-            if (!is_valid_option_list(optarg)) {
-                error_report("Invalid option list: %s", optarg);
-                ret = -1;
-                goto fail_getopt;
-            }
-            if (!options) {
-                options = g_strdup(optarg);
-            } else {
-                char *old_options = options;
-                options = g_strdup_printf("%s,%s", options, optarg);
-                g_free(old_options);
-            }
-            break;
-        case 's':
-            snapshot_name = optarg;
-            break;
-        case 'l':
-            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
-                sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
-                if (!sn_opts) {
-                    error_report("Failed in parsing snapshot param '%s'",
-                                 optarg);
-                    ret = -1;
-                    goto fail_getopt;
-                }
-            } else {
-                snapshot_name = optarg;
-            }
-            break;
-        case 'S':
-        {
-            int64_t sval;
-            char *end;
-            sval = strtosz_suffix(optarg, &end, STRTOSZ_DEFSUFFIX_B);
-            if (sval < 0 || *end) {
-                error_report("Invalid minimum zero buffer size for sparse output specified");
-                ret = -1;
-                goto fail_getopt;
-            }
+    bs_n = qemu_arg_get_command_positional_list_cnt(cmd) - 1;
 
-            min_sparse = sval / BDRV_SECTOR_SIZE;
-            break;
-        }
-        case 'p':
-            progress = 1;
-            break;
-        case 't':
-            cache = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        case 'n':
-            skip_create = 1;
-            break;
-        }
+    if (bs_n < 1) {
+        error_report("At least an image and an output image file name must be"
+                     " provided");
+        return -1;
     }
 
-    /* Initialize before goto out */
-    if (quiet) {
-        progress = 0;
+    out_filename = _filename_list[bs_n];
+    if (_options && has_help_option(_options)) {
+        ret = print_block_option_help(out_filename, _ofmt);
+        goto out;
     }
-    qemu_progress_init(progress, 1.0);
 
+    if (_snapshot_name) {
+        if (strstart(_snapshot_name, SNAPSHOT_OPT_BASE, NULL)) {
+            sn_opts = qemu_opts_parse(&internal_snapshot_opts,
+                                      _snapshot_name, 0);
+            if (!sn_opts) {
+                error_report("Failed in parsing snapshot param '%s'",
+                             optarg);
+                return -1;
+            }
+        }
+    }
 
-    bs_n = argc - optind - 1;
-    out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;
+    if (_sparse_size) {
+        int64_t sval;
+        char *end;
+        sval = strtosz_suffix(_sparse_size, &end, STRTOSZ_DEFSUFFIX_B);
+        if (sval < 0 || *end) {
+            error_report("Invalid minimum zero buffer size for sparse output "
+                         "specified");
+            return -1;
+        }
 
-    if (options && has_help_option(options)) {
-        ret = print_block_option_help(out_filename, out_fmt);
-        goto out;
+        min_sparse = sval / BDRV_SECTOR_SIZE;
     }
 
-    if (bs_n < 1) {
-        help();
+    if (_quiet) {
+        _progress = 0;
     }
 
+    /* Initialize before goto out */
+    qemu_progress_init(_progress, 1.0);
 
-    if (bs_n > 1 && out_baseimg) {
+    if (bs_n > 1 && _out_baseimg) {
         error_report("-B makes no sense when concatenating multiple input "
                      "images");
         ret = -1;
@@ -1291,11 +1115,11 @@ static int img_convert(int argc, char **argv)
     bs = g_malloc0(bs_n * sizeof(BlockDriverState *));
 
     total_sectors = 0;
-    for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true,
-                                 quiet);
+    for (bs_i = 0, curr = _filename_list; *curr && *curr != out_filename;
+         curr++, bs_i++) {
+        bs[bs_i] = bdrv_new_open(*curr, _fmt, BDRV_O_FLAGS, true, _quiet);
         if (!bs[bs_i]) {
-            error_report("Could not open '%s'", argv[optind + bs_i]);
+            error_report("Could not open '%s'", *curr);
             ret = -1;
             goto out;
         }
@@ -1308,14 +1132,14 @@ static int img_convert(int argc, char **argv)
                                      qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
                                      qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
                                      &local_err);
-    } else if (snapshot_name != NULL) {
+    } else if (_snapshot_name != NULL) {
         if (bs_n > 1) {
             error_report("No support for concatenating multiple snapshot");
             ret = -1;
             goto out;
         }
 
-        bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
+        bdrv_snapshot_load_tmp_by_id_or_name(bs[0], _snapshot_name, &local_err);
     }
     if (local_err) {
         error_report("Failed to load snapshot: %s",
@@ -1326,9 +1150,9 @@ static int img_convert(int argc, char **argv)
     }
 
     /* Find driver and parse its options */
-    drv = bdrv_find_format(out_fmt);
+    drv = bdrv_find_format(_ofmt);
     if (!drv) {
-        error_report("Unknown file format '%s'", out_fmt);
+        error_report("Unknown file format '%s'", _ofmt);
         ret = -1;
         goto out;
     }
@@ -1345,10 +1169,10 @@ static int img_convert(int argc, char **argv)
     create_options = append_option_parameters(create_options,
                                               proto_drv->create_options);
 
-    if (options) {
-        param = parse_option_parameters(options, create_options, param);
+    if (_options) {
+        param = parse_option_parameters(_options, create_options, param);
         if (param == NULL) {
-            error_report("Invalid options for file format '%s'.", out_fmt);
+            error_report("Invalid options for file format '%s'.", _ofmt);
             ret = -1;
             goto out;
         }
@@ -1357,7 +1181,7 @@ static int img_convert(int argc, char **argv)
     }
 
     set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
-    ret = add_old_style_options(out_fmt, param, out_baseimg, NULL);
+    ret = add_old_style_options(_ofmt, param, _out_baseimg, NULL);
     if (ret < 0) {
         goto out;
     }
@@ -1365,11 +1189,11 @@ static int img_convert(int argc, char **argv)
     /* Get backing file name if -o backing_file was used */
     out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
     if (out_baseimg_param) {
-        out_baseimg = out_baseimg_param->value.s;
+        _out_baseimg = out_baseimg_param->value.s;
     }
 
     /* Check if compression is supported */
-    if (compress) {
+    if (_compress) {
         QEMUOptionParameter *encryption =
             get_option_parameter(param, BLOCK_OPT_ENCRYPT);
         QEMUOptionParameter *preallocation =
@@ -1398,25 +1222,25 @@ static int img_convert(int argc, char **argv)
         }
     }
 
-    if (!skip_create) {
+    if (!_skip_create) {
         /* Create the new image */
         ret = bdrv_create(drv, out_filename, param, &local_err);
         if (ret < 0) {
             error_report("%s: error while converting %s: %s",
-                         out_filename, out_fmt, error_get_pretty(local_err));
+                         out_filename, _ofmt, error_get_pretty(local_err));
             error_free(local_err);
             goto out;
         }
     }
 
     flags = min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    ret = bdrv_parse_cache_flags(_cache, &flags);
     if (ret < 0) {
-        error_report("Invalid cache option: %s", cache);
+        error_report("Invalid cache option: %s", _cache);
         return -1;
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, flags, true, quiet);
+    out_bs = bdrv_new_open(out_filename, _ofmt, flags, true, _quiet);
     if (!out_bs) {
         ret = -1;
         goto out;
@@ -1436,7 +1260,7 @@ static int img_convert(int argc, char **argv)
 
     buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
 
-    if (skip_create) {
+    if (_skip_create) {
         int64_t output_length = bdrv_getlength(out_bs);
         if (output_length < 0) {
             error_report("unable to get output image length: %s\n",
@@ -1453,7 +1277,7 @@ static int img_convert(int argc, char **argv)
     cluster_sectors = 0;
     ret = bdrv_get_info(out_bs, &bdi);
     if (ret < 0) {
-        if (compress) {
+        if (_compress) {
             error_report("could not get block driver info");
             goto out;
         }
@@ -1461,7 +1285,7 @@ static int img_convert(int argc, char **argv)
         cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
     }
 
-    if (compress) {
+    if (_compress) {
         if (cluster_sectors <= 0 || cluster_sectors > bufsectors) {
             error_report("invalid cluster size");
             ret = -1;
@@ -1545,7 +1369,7 @@ static int img_convert(int argc, char **argv)
         }
 
         sectors_to_read = total_sectors;
-        count_allocated_sectors = progress && (out_baseimg || has_zero_init);
+        count_allocated_sectors = _progress && (_out_baseimg || has_zero_init);
 restart:
         sector_num = 0; // total number of sectors converted so far
         sectors_read = 0;
@@ -1573,7 +1397,7 @@ restart:
                    sector_num, bs_i, bs_offset, bs_sectors); */
             }
 
-            if ((out_baseimg || has_zero_init) &&
+            if ((_out_baseimg || has_zero_init) &&
                 sector_num >= sector_num_next_status) {
                 n = nb_sectors > INT_MAX ? INT_MAX : nb_sectors;
                 ret = bdrv_get_block_status(bs[bs_i], sector_num - bs_offset,
@@ -1587,7 +1411,7 @@ restart:
                 /* If the output image is zero initialized, we are not working
                  * on a shared base and the input is zero we can skip the next
                  * n1 sectors */
-                if (has_zero_init && !out_baseimg && (ret & BDRV_BLOCK_ZERO)) {
+                if (has_zero_init && !_out_baseimg && (ret & BDRV_BLOCK_ZERO)) {
                     sector_num += n1;
                     continue;
                 }
@@ -1595,7 +1419,7 @@ restart:
                  * image, assume that sectors which are unallocated in the
                  * input image are present in both the output's and input's
                  * base images (no need to copy them). */
-                if (out_baseimg) {
+                if (_out_baseimg) {
                     if (!(ret & BDRV_BLOCK_DATA)) {
                         sector_num += n1;
                         continue;
@@ -1681,8 +1505,6 @@ out:
         }
         g_free(bs);
     }
-fail_getopt:
-    g_free(options);
 
     if (ret) {
         return 1;
@@ -1840,61 +1662,21 @@ err:
     return NULL;
 }
 
-static int img_info(int argc, char **argv)
+static int img_info(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c;
     OutputFormat output_format = OFORMAT_HUMAN;
-    bool chain = false;
-    const char *filename, *fmt, *output;
     ImageInfoList *list;
 
-    fmt = NULL;
-    output = NULL;
-    for(;;) {
-        int option_index = 0;
-        static const struct option long_options[] = {
-            {"help", no_argument, 0, 'h'},
-            {"format", required_argument, 0, 'f'},
-            {"output", required_argument, 0, OPTION_OUTPUT},
-            {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
-            {0, 0, 0, 0}
-        };
-        c = getopt_long(argc, argv, "f:h",
-                        long_options, &option_index);
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case OPTION_OUTPUT:
-            output = optarg;
-            break;
-        case OPTION_BACKING_CHAIN:
-            chain = true;
-            break;
-        }
-    }
-    if (optind != argc - 1) {
-        help();
-    }
-    filename = argv[optind++];
-
-    if (output && !strcmp(output, "json")) {
+    if (_ofmt && !strcmp(_ofmt, "json")) {
         output_format = OFORMAT_JSON;
-    } else if (output && !strcmp(output, "human")) {
+    } else if (_ofmt && !strcmp(_ofmt, "human")) {
         output_format = OFORMAT_HUMAN;
-    } else if (output) {
+    } else if (_ofmt) {
         error_report("--output must be used with human or json as argument.");
         return 1;
     }
 
-    list = collect_image_info_list(filename, fmt, chain);
+    list = collect_image_info_list(_filename, _fmt, _backing_chain);
     if (!list) {
         return 1;
     }
@@ -1904,7 +1686,7 @@ static int img_info(int argc, char **argv)
         dump_human_image_info_list(list);
         break;
     case OFORMAT_JSON:
-        if (chain) {
+        if (_backing_chain) {
             dump_json_image_info_list(list);
         } else {
             dump_json_image_info(list->value);
@@ -1916,7 +1698,6 @@ static int img_info(int argc, char **argv)
     return 0;
 }
 
-
 typedef struct MapEntry {
     int flags;
     int depth;
@@ -2007,59 +1788,24 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
     return 0;
 }
 
-static int img_map(int argc, char **argv)
+static int img_map(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c;
     OutputFormat output_format = OFORMAT_HUMAN;
     BlockDriverState *bs;
-    const char *filename, *fmt, *output;
     int64_t length;
     MapEntry curr = { .length = 0 }, next;
     int ret = 0;
 
-    fmt = NULL;
-    output = NULL;
-    for (;;) {
-        int option_index = 0;
-        static const struct option long_options[] = {
-            {"help", no_argument, 0, 'h'},
-            {"format", required_argument, 0, 'f'},
-            {"output", required_argument, 0, OPTION_OUTPUT},
-            {0, 0, 0, 0}
-        };
-        c = getopt_long(argc, argv, "f:h",
-                        long_options, &option_index);
-        if (c == -1) {
-            break;
-        }
-        switch (c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case OPTION_OUTPUT:
-            output = optarg;
-            break;
-        }
-    }
-    if (optind >= argc) {
-        help();
-    }
-    filename = argv[optind++];
-
-    if (output && !strcmp(output, "json")) {
+    if (_ofmt && !strcmp(_ofmt, "json")) {
         output_format = OFORMAT_JSON;
-    } else if (output && !strcmp(output, "human")) {
+    } else if (_ofmt && !strcmp(_ofmt, "human")) {
         output_format = OFORMAT_HUMAN;
-    } else if (output) {
+    } else if (_ofmt) {
         error_report("--output must be used with human or json as argument.");
         return 1;
     }
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS, true, false);
+    bs = bdrv_new_open(_filename, _fmt, BDRV_O_FLAGS, true, false);
     if (!bs) {
         return 1;
     }
@@ -2112,74 +1858,34 @@ out:
 #define SNAPSHOT_APPLY  3
 #define SNAPSHOT_DELETE 4
 
-static int img_snapshot(int argc, char **argv)
+static int img_snapshot(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
     BlockDriverState *bs;
     QEMUSnapshotInfo sn;
-    char *filename, *snapshot_name = NULL;
-    int c, ret = 0, bdrv_oflags;
+    char *snapshot_name = NULL;
+    int ret = 0, bdrv_oflags;
     int action = 0;
     qemu_timeval tv;
-    bool quiet = false;
     Error *err = NULL;
 
     bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
-    /* Parse commandline parameters */
-    for(;;) {
-        c = getopt(argc, argv, "la:c:d:hq");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            return 0;
-        case 'l':
-            if (action) {
-                help();
-                return 0;
-            }
-            action = SNAPSHOT_LIST;
-            bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
-            break;
-        case 'a':
-            if (action) {
-                help();
-                return 0;
-            }
-            action = SNAPSHOT_APPLY;
-            snapshot_name = optarg;
-            break;
-        case 'c':
-            if (action) {
-                help();
-                return 0;
-            }
-            action = SNAPSHOT_CREATE;
-            snapshot_name = optarg;
-            break;
-        case 'd':
-            if (action) {
-                help();
-                return 0;
-            }
-            action = SNAPSHOT_DELETE;
-            snapshot_name = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        }
-    }
 
-    if (optind != argc - 1) {
-        help();
+    if (_snapshot_list) {
+        action = SNAPSHOT_LIST;
+        bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
+    } else if (_snapshot_apply) {
+        action = SNAPSHOT_APPLY;
+        snapshot_name = _snapshot_apply;
+    } else if (_snapshot_create) {
+        action = SNAPSHOT_CREATE;
+        snapshot_name = _snapshot_create;
+    } else if (_snapshot_delete) {
+        action = SNAPSHOT_DELETE;
+        snapshot_name = _snapshot_delete;
     }
-    filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new_open(filename, NULL, bdrv_oflags, true, quiet);
+    bs = bdrv_new_open(_filename, NULL, bdrv_oflags, true, _quiet);
     if (!bs) {
         return 1;
     }
@@ -2232,73 +1938,29 @@ static int img_snapshot(int argc, char **argv)
     return 0;
 }
 
-static int img_rebase(int argc, char **argv)
+static int img_rebase(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
     BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL;
     BlockDriver *old_backing_drv, *new_backing_drv;
-    char *filename;
-    const char *fmt, *cache, *out_basefmt, *out_baseimg;
-    int c, flags, ret;
-    int unsafe = 0;
-    int progress = 0;
-    bool quiet = false;
+    int flags, ret;
     Error *local_err = NULL;
 
-    /* Parse commandline parameters */
-    fmt = NULL;
-    cache = BDRV_DEFAULT_CACHE;
-    out_baseimg = NULL;
-    out_basefmt = NULL;
-    for(;;) {
-        c = getopt(argc, argv, "uhf:F:b:pt:q");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            return 0;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 'F':
-            out_basefmt = optarg;
-            break;
-        case 'b':
-            out_baseimg = optarg;
-            break;
-        case 'u':
-            unsafe = 1;
-            break;
-        case 'p':
-            progress = 1;
-            break;
-        case 't':
-            cache = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        }
-    }
-
-    if (quiet) {
-        progress = 0;
+    if (_quiet) {
+        _progress = 0;
     }
 
-    if ((optind != argc - 1) || (!unsafe && !out_baseimg)) {
-        help();
+    if (!_unsafe && !_out_baseimg) {
+        qemu_arg_print_command_help(ctx, cmd);
+        return -1;
     }
-    filename = argv[optind++];
 
-    qemu_progress_init(progress, 2.0);
+    qemu_progress_init(_progress, 2.0);
     qemu_progress_print(0, 100);
 
-    flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    flags = BDRV_O_RDWR | (_unsafe ? BDRV_O_NO_BACKING : 0);
+    ret = bdrv_parse_cache_flags(_cache, &flags);
     if (ret < 0) {
-        error_report("Invalid cache option: %s", cache);
+        error_report("Invalid cache option: %s", _cache);
         return -1;
     }
 
@@ -2308,7 +1970,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new_open(filename, fmt, flags, true, quiet);
+    bs = bdrv_new_open(_filename, _fmt, flags, true, _quiet);
     if (!bs) {
         return 1;
     }
@@ -2317,7 +1979,7 @@ static int img_rebase(int argc, char **argv)
     old_backing_drv = NULL;
     new_backing_drv = NULL;
 
-    if (!unsafe && bs->backing_format[0] != '\0') {
+    if (!_unsafe && bs->backing_format[0] != '\0') {
         old_backing_drv = bdrv_find_format(bs->backing_format);
         if (old_backing_drv == NULL) {
             error_report("Invalid format name: '%s'", bs->backing_format);
@@ -2326,17 +1988,17 @@ static int img_rebase(int argc, char **argv)
         }
     }
 
-    if (out_basefmt != NULL) {
-        new_backing_drv = bdrv_find_format(out_basefmt);
+    if (_out_basefmt != NULL) {
+        new_backing_drv = bdrv_find_format(_out_basefmt);
         if (new_backing_drv == NULL) {
-            error_report("Invalid format name: '%s'", out_basefmt);
+            error_report("Invalid format name: '%s'", _out_basefmt);
             ret = -1;
             goto out;
         }
     }
 
     /* For safe rebasing we need to compare old and new backing file */
-    if (unsafe) {
+    if (_unsafe) {
         /* Make the compiler happy */
         bs_old_backing = NULL;
         bs_new_backing = NULL;
@@ -2353,13 +2015,13 @@ static int img_rebase(int argc, char **argv)
             error_free(local_err);
             goto out;
         }
-        if (out_baseimg[0]) {
+        if (_out_baseimg[0]) {
             bs_new_backing = bdrv_new("new_backing");
-            ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL,
+            ret = bdrv_open(&bs_new_backing, _out_baseimg, NULL, NULL,
                             BDRV_O_FLAGS, new_backing_drv, &local_err);
             if (ret) {
                 error_report("Could not open new backing file '%s': %s",
-                             out_baseimg, error_get_pretty(local_err));
+                             _out_baseimg, error_get_pretty(local_err));
                 error_free(local_err);
                 goto out;
             }
@@ -2375,7 +2037,7 @@ static int img_rebase(int argc, char **argv)
      * If qemu-img crashes during this step, no harm is done. The content of
      * the image is the same as the original one at any time.
      */
-    if (!unsafe) {
+    if (!_unsafe) {
         uint64_t num_sectors;
         uint64_t old_backing_num_sectors;
         uint64_t new_backing_num_sectors = 0;
@@ -2483,18 +2145,18 @@ static int img_rebase(int argc, char **argv)
      * backing file are overwritten in the COW file now, so the visible content
      * doesn't change when we switch the backing file.
      */
-    if (out_baseimg && *out_baseimg) {
-        ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
+    if (_out_baseimg && *_out_baseimg) {
+        ret = bdrv_change_backing_file(bs, _out_baseimg, _out_basefmt);
     } else {
         ret = bdrv_change_backing_file(bs, NULL, NULL);
     }
 
     if (ret == -ENOSPC) {
         error_report("Could not change the backing file to '%s': No "
-                     "space left in the file header", out_baseimg);
+                     "space left in the file header", _out_baseimg);
     } else if (ret < 0) {
         error_report("Could not change the backing file to '%s': %s",
-            out_baseimg, strerror(-ret));
+            _out_baseimg, strerror(-ret));
     }
 
     qemu_progress_print(100, 0);
@@ -2507,7 +2169,7 @@ static int img_rebase(int argc, char **argv)
 out:
     qemu_progress_end();
     /* Cleanup */
-    if (!unsafe) {
+    if (!_unsafe) {
         if (bs_old_backing != NULL) {
             bdrv_unref(bs_old_backing);
         }
@@ -2523,10 +2185,9 @@ out:
     return 0;
 }
 
-static int img_resize(int argc, char **argv)
+static int img_resize(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c, ret, relative;
-    const char *filename, *fmt, *size;
+    int ret, relative;
     int64_t n, total_size;
     bool quiet = false;
     BlockDriverState *bs = NULL;
@@ -2545,49 +2206,15 @@ static int img_resize(int argc, char **argv)
         },
     };
 
-    /* Remove size from argv manually so that negative numbers are not treated
-     * as options by getopt. */
-    if (argc < 3) {
-        help();
-        return 1;
-    }
-
-    size = argv[--argc];
-
-    /* Parse getopt arguments */
-    fmt = NULL;
-    for(;;) {
-        c = getopt(argc, argv, "f:hq");
-        if (c == -1) {
-            break;
-        }
-        switch(c) {
-        case '?':
-        case 'h':
-            help();
-            break;
-        case 'f':
-            fmt = optarg;
-            break;
-        case 'q':
-            quiet = true;
-            break;
-        }
-    }
-    if (optind != argc - 1) {
-        help();
-    }
-    filename = argv[optind++];
-
     /* Choose grow, shrink, or absolute resize mode */
-    switch (size[0]) {
+    switch (_size[0]) {
     case '+':
         relative = 1;
-        size++;
+        _size++;
         break;
     case '-':
         relative = -1;
-        size++;
+        _size++;
         break;
     default:
         relative = 0;
@@ -2596,7 +2223,7 @@ static int img_resize(int argc, char **argv)
 
     /* Parse size */
     param = qemu_opts_create(&resize_options, NULL, 0, &error_abort);
-    if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) {
+    if (qemu_opt_set(param, BLOCK_OPT_SIZE, _size)) {
         /* Error message already printed when size parsing fails */
         ret = -1;
         qemu_opts_del(param);
@@ -2605,7 +2232,8 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+    bs = bdrv_new_open(_filename, _fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true,
+                       _quiet);
     if (!bs) {
         ret = -1;
         goto out;
@@ -2647,86 +2275,41 @@ out:
     return 0;
 }
 
-static int img_amend(int argc, char **argv)
+static int img_amend(const QemuArgContext *ctx, const QemuArgCommand *cmd)
 {
-    int c, ret = 0;
-    char *options = NULL;
+    int ret = 0;
     QEMUOptionParameter *create_options = NULL, *options_param = NULL;
-    const char *fmt = NULL, *filename;
-    bool quiet = false;
     BlockDriverState *bs = NULL;
 
-    for (;;) {
-        c = getopt(argc, argv, "hqf:o:");
-        if (c == -1) {
-            break;
-        }
-
-        switch (c) {
-            case 'h':
-            case '?':
-                help();
-                break;
-            case 'o':
-                if (!is_valid_option_list(optarg)) {
-                    error_report("Invalid option list: %s", optarg);
-                    ret = -1;
-                    goto out;
-                }
-                if (!options) {
-                    options = g_strdup(optarg);
-                } else {
-                    char *old_options = options;
-                    options = g_strdup_printf("%s,%s", options, optarg);
-                    g_free(old_options);
-                }
-                break;
-            case 'f':
-                fmt = optarg;
-                break;
-            case 'q':
-                quiet = true;
-                break;
-        }
-    }
-
-    if (!options) {
-        help();
-    }
-
-    filename = (optind == argc - 1) ? argv[argc - 1] : NULL;
-    if (fmt && has_help_option(options)) {
+    if (_fmt && has_help_option(_options)) {
         /* If a format is explicitly specified (and possibly no filename is
          * given), print option help here */
-        ret = print_block_option_help(filename, fmt);
+        ret = print_block_option_help(_filename, _fmt);
         goto out;
     }
 
-    if (optind != argc - 1) {
-        help();
-    }
-
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+    bs = bdrv_new_open(_filename, _fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true,
+                       _quiet);
     if (!bs) {
-        error_report("Could not open image '%s'", filename);
+        error_report("Could not open image '%s'", _filename);
         ret = -1;
         goto out;
     }
 
-    fmt = bs->drv->format_name;
+    _fmt = (char *)bs->drv->format_name;
 
-    if (has_help_option(options)) {
+    if (has_help_option(_options)) {
         /* If the format was auto-detected, print option help here */
-        ret = print_block_option_help(filename, fmt);
+        ret = print_block_option_help(_filename, _fmt);
         goto out;
     }
 
     create_options = append_option_parameters(create_options,
             bs->drv->create_options);
-    options_param = parse_option_parameters(options, create_options,
+    options_param = parse_option_parameters(_options, create_options,
             options_param);
     if (options_param == NULL) {
-        error_report("Invalid options for file format '%s'", fmt);
+        error_report("Invalid options for file format '%s'", _fmt);
         ret = -1;
         goto out;
     }
@@ -2743,7 +2326,6 @@ out:
     }
     free_option_parameters(create_options);
     free_option_parameters(options_param);
-    g_free(options);
 
     if (ret) {
         return 1;
@@ -2751,19 +2333,46 @@ out:
     return 0;
 }
 
-static const img_cmd_t img_cmds[] = {
-#define DEF(option, callback, arg_string)        \
-    { option, callback },
-#include "qemu-img-cmds.h"
-#undef DEF
-#undef GEN_DOCS
-    { NULL, NULL, },
+static const QemuArgCommand _cmds[] = {
+    OPT_CMD("check", QEMU_IMG_CHECK_HELP, _check_args, NULL, img_check),
+    OPT_CMD("create", QEMU_IMG_CREATE_HELP, _create_args, NULL, img_create),
+    OPT_CMD("commit", QEMU_IMG_COMMIT_HELP, _commit_args, NULL, img_commit),
+    OPT_CMD("compare", QEMU_IMG_COMPARE_HELP, _compare_args, NULL, img_compare),
+    OPT_CMD("convert", QEMU_IMG_CONVERT_HELP, _convert_args, NULL, img_convert),
+    OPT_CMD("info", QEMU_IMG_INFO_HELP, _info_args, NULL, img_info),
+    OPT_CMD("map", QEMU_IMG_MAP_HELP, _map_args, NULL, img_map),
+    OPT_CMD("snapshot", QEMU_IMG_SNAPSHOT_HELP, _snapshot_args,
+            _snapshot_mutual_groups, img_snapshot),
+    OPT_CMD("rebase", QEMU_IMG_REBASE_HELP, _rebase_args, NULL, img_rebase),
+    OPT_CMD("resize", QEMU_IMG_RESIZE_HELP, _resize_args, NULL, img_resize),
+    OPT_CMD("amend", QEMU_IMG_AMEND_HELP, _amend_args, NULL, img_amend),
+    CMD_SENTINEL
 };
 
+static void format_print(void *opaque, const char *name)
+{
+    char **str = opaque;
+    char *cp;
+
+    if (*str) {
+        cp = g_strdup(*str);
+        free(*str);
+
+        *str = g_malloc0(strlen(cp) + strlen(name) + 1);
+        sprintf(*str, "%s %s", cp, name);
+
+        free(cp);
+    } else {
+        *str = g_malloc0(strlen(name) + 1);
+        sprintf(*str, "%s", name);
+    }
+}
+
 int main(int argc, char **argv)
 {
-    const img_cmd_t *cmd;
-    const char *cmdname;
+    int ret;
+    char *formats = NULL, *epilogue = NULL;
+    const char *pref = "Supported formats: ";
 
 #ifdef CONFIG_POSIX
     signal(SIGPIPE, SIG_IGN);
@@ -2774,19 +2383,20 @@ int main(int argc, char **argv)
 
     qemu_init_main_loop();
     bdrv_init();
-    if (argc < 2)
-        help();
-    cmdname = argv[1];
-    argc--; argv++;
 
-    /* find the command */
-    for(cmd = img_cmds; cmd->name != NULL; cmd++) {
-        if (!strcmp(cmdname, cmd->name)) {
-            return cmd->handler(argc, argv);
-        }
-    }
+    bdrv_iterate_format(format_print, &formats);
 
-    /* not found */
-    help();
-    return 0;
+    epilogue = g_malloc0(strlen(formats) + strlen(pref));
+    sprintf(epilogue, "%s%s", pref, formats);
+
+    OPT_CTX(ctx, PROLOGUE, epilogue, "qemu-img", _cmds, NULL, NULL,
+            OPT_DECORATE_LONG);
+
+    ret = qemu_arg_parse(argc, argv, &ctx);
+    qemu_arg_context_cleanup(&ctx);
+
+    free(formats);
+    free(epilogue);
+
+    return ret;
 }
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 1/2] qemu-arg: introduce a " Leandro Dorileo
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg Leandro Dorileo
@ 2014-03-08 18:55 ` Peter Maydell
  2014-03-08 20:28   ` Leandro Dorileo
  2014-03-09 16:32 ` Andreas Färber
  2014-03-11 11:06 ` Kevin Wolf
  4 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2014-03-08 18:55 UTC (permalink / raw)
  To: Leandro Dorileo
  Cc: Kevin Wolf, Fam Zheng, Stefan Weil, Michael Tokarev,
	QEMU Developers, Stefan Hajnoczi, Paolo Bonzini, Laszlo Ersek,
	Peter Lieven

On 8 March 2014 18:47, Leandro Dorileo <l@dorileo.org> wrote:
> The following patchset introduces a general purpose argument parser and migrates
> qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> good feedback here I move forward and migrate all the other possible users.

Can you describe what the QEMU-specific features are that
mean we must roll our own argument-parsing infrastructure
rather than using (say) the glib option parsing routines?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-08 18:55 ` [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Peter Maydell
@ 2014-03-08 20:28   ` Leandro Dorileo
  0 siblings, 0 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-08 20:28 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Kevin Wolf, Fam Zheng, Stefan Weil, Michael Tokarev,
	QEMU Developers, Stefan Hajnoczi, Paolo Bonzini, Laszlo Ersek,
	Peter Lieven

Hi Peter,

On Sat, Mar 08, 2014 at 06:55:50PM +0000, Peter Maydell wrote:
> On 8 March 2014 18:47, Leandro Dorileo <l@dorileo.org> wrote:
> > The following patchset introduces a general purpose argument parser and migrates
> > qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> > good feedback here I move forward and migrate all the other possible users.
> 
> Can you describe what the QEMU-specific features are that
> mean we must roll our own argument-parsing infrastructure
> rather than using (say) the glib option parsing routines?


I don't think GOption will do the output I want and think to be the ideal for
qemu or specially to qemu-img.

GOption knows nothing about command, since qemu-img was my first target I
wanted something to handle its command schema. GOption will not show the users
the list of available commands neither show commands specific options, I wanted
the user to run qemu-img create -h and show the create command arguments (similar
to git output). GOption will not do that by default, of course we can wrap that
and have something similar.

GOption will not be able to list commands and their arguments so we can generate
the .hx file (see patch 02 in my series) or maybe the texi output and keep the
sync between the implemented commands, arguments and the textinfo stuffs.
GOption also doesn't know about the "cumulative" stuff - well I agree this last
one is not somethig to justify qemu-arg per se.

The command callbacks flow is also something GOption will not give us for free,
we would still need to know ourself about the available commands and their
callees.

As I said, my first target was qemu-img, but I ended up writing something more
generic to be used elsewhere.

Of course I could wrap GOption or getopt and handle all the corner cases but
parsing the arguments myself gave me more flexibility.

Regards...

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg
  2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg Leandro Dorileo
@ 2014-03-09  7:30   ` Paolo Bonzini
  2014-03-09 12:37     ` Leandro Dorileo
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2014-03-09  7:30 UTC (permalink / raw)
  To: Leandro Dorileo, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, Stefan Hajnoczi, Laszlo Ersek, Peter Lieven

Il 08/03/2014 19:47, Leandro Dorileo ha scritto:
> Remove the arg parsing implementations using getopt and use qemu-arg.
> Also remove the qemu-img-cmds.hx since it's now generated on building time,
> adapted the build system to generate the .hx file using the qemu-img itself
> using the qemu-arg internal command generate-hx.
>
> Signed-off-by: Leandro Dorileo <l@dorileo.org>

This makes it much harder to cross-compile QEMU.  Also, I wonder how 
hard it would be to apply the same approach to the main QEMU binary 
which already uses QemuOpts for its more complex arguments; for sure you 
risk that accumulating multiple layers of abstractions makes the code 
even harder to read than it is now.

Paolo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg
  2014-03-09  7:30   ` Paolo Bonzini
@ 2014-03-09 12:37     ` Leandro Dorileo
  2014-03-09 13:03       ` Peter Maydell
  0 siblings, 1 reply; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-09 12:37 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Laszlo Ersek,
	Peter Lieven

Hi Paolo,

On Sun, Mar 09, 2014 at 08:30:28AM +0100, Paolo Bonzini wrote:
> Il 08/03/2014 19:47, Leandro Dorileo ha scritto:
> >Remove the arg parsing implementations using getopt and use qemu-arg.
> >Also remove the qemu-img-cmds.hx since it's now generated on building time,
> >adapted the build system to generate the .hx file using the qemu-img itself
> >using the qemu-arg internal command generate-hx.
> >
> >Signed-off-by: Leandro Dorileo <l@dorileo.org>
> 
> This makes it much harder to cross-compile QEMU.

What's non-portable in this case? what would limit the QEMU cross-compile?

>  Also, I wonder how hard it
> would be to apply the same approach to the main QEMU binary which already
> uses QemuOpts for its more complex arguments;

Yeah, you're right, QEMU binary is much more complex, In that case I think we
should put QemuOpts and QemuArg together or so, I still need to better understand
the current vl.c + QemuOpt source code to come up with a good solution.

> for sure you risk that
> accumulating multiple layers of abstractions makes the code even harder to
> read than it is now.

The idea is to keep things simple not the other way round. I think it's possible
to accommodate both cases without imposing more complexity.


Regards...

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg
  2014-03-09 12:37     ` Leandro Dorileo
@ 2014-03-09 13:03       ` Peter Maydell
  2014-03-09 13:35         ` Leandro Dorileo
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2014-03-09 13:03 UTC (permalink / raw)
  To: Leandro Dorileo
  Cc: Kevin Wolf, Fam Zheng, Stefan Weil, Michael Tokarev,
	QEMU Developers, Stefan Hajnoczi, Paolo Bonzini, Laszlo Ersek,
	Peter Lieven

On 9 March 2014 12:37, Leandro Dorileo <l@dorileo.org> wrote:
> Hi Paolo,
>
> On Sun, Mar 09, 2014 at 08:30:28AM +0100, Paolo Bonzini wrote:
>> Il 08/03/2014 19:47, Leandro Dorileo ha scritto:
>> >Remove the arg parsing implementations using getopt and use qemu-arg.
>> >Also remove the qemu-img-cmds.hx since it's now generated on building time,
>> >adapted the build system to generate the .hx file using the qemu-img itself
>> >using the qemu-arg internal command generate-hx.
>> >
>> >Signed-off-by: Leandro Dorileo <l@dorileo.org>
>>
>> This makes it much harder to cross-compile QEMU.
>
> What's non-portable in this case? what would limit the QEMU cross-compile?

qemu-img is a binary for the target system; it can't run at all
on the host system, so you can't run it during compilation.
You need to keep "things we do during build" completely
separate from the runtime binaries.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg
  2014-03-09 13:03       ` Peter Maydell
@ 2014-03-09 13:35         ` Leandro Dorileo
  0 siblings, 0 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-09 13:35 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Kevin Wolf, Fam Zheng, Stefan Weil, Michael Tokarev,
	QEMU Developers, Stefan Hajnoczi, Paolo Bonzini, Laszlo Ersek,
	Peter Lieven

On Sun, Mar 09, 2014 at 01:03:12PM +0000, Peter Maydell wrote:
> On 9 March 2014 12:37, Leandro Dorileo <l@dorileo.org> wrote:
> > Hi Paolo,
> >
> > On Sun, Mar 09, 2014 at 08:30:28AM +0100, Paolo Bonzini wrote:
> >> Il 08/03/2014 19:47, Leandro Dorileo ha scritto:
> >> >Remove the arg parsing implementations using getopt and use qemu-arg.
> >> >Also remove the qemu-img-cmds.hx since it's now generated on building time,
> >> >adapted the build system to generate the .hx file using the qemu-img itself
> >> >using the qemu-arg internal command generate-hx.
> >> >
> >> >Signed-off-by: Leandro Dorileo <l@dorileo.org>
> >>
> >> This makes it much harder to cross-compile QEMU.
> >
> > What's non-portable in this case? what would limit the QEMU cross-compile?
> 
> qemu-img is a binary for the target system; it can't run at all
> on the host system, so you can't run it during compilation.
> You need to keep "things we do during build" completely
> separate from the runtime binaries.

Yep, true.

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
                   ` (2 preceding siblings ...)
  2014-03-08 18:55 ` [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Peter Maydell
@ 2014-03-09 16:32 ` Andreas Färber
  2014-03-09 21:47   ` Leandro Dorileo
  2014-03-11 11:06 ` Kevin Wolf
  4 siblings, 1 reply; 15+ messages in thread
From: Andreas Färber @ 2014-03-09 16:32 UTC (permalink / raw)
  To: Leandro Dorileo, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, Stefan Hajnoczi, Paolo Bonzini, Laszlo Ersek,
	Peter Lieven

Am 08.03.2014 19:47, schrieb Leandro Dorileo:
> The following patchset introduces a general purpose argument parser and migrates
> qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> good feedback here I move forward and migrate all the other possible users.

Why? :) You forgot to describe what's wrong with the current
infrastructure, how your approach is different and what the benefit is.

Regards,
Andreas

> Leandro Dorileo (2):
>   qemu-arg: introduce a general purpose argument parser
>   qemu-img: migrate to use qemu-arg
> 
>  .gitignore              |    1 +
>  Makefile                |   12 +-
>  include/qemu/qemu-arg.h |  287 ++++++++++++
>  qemu-img-cmds.hx        |   77 ---
>  qemu-img-descs.h        |  128 +++++
>  qemu-img.c              | 1184 ++++++++++++++++-------------------------------
>  util/Makefile.objs      |    1 +
>  util/qemu-arg.c         |  887 +++++++++++++++++++++++++++++++++++
>  8 files changed, 1706 insertions(+), 871 deletions(-)
>  create mode 100644 include/qemu/qemu-arg.h
>  delete mode 100644 qemu-img-cmds.hx
>  create mode 100644 qemu-img-descs.h
>  create mode 100644 util/qemu-arg.c
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-09 16:32 ` Andreas Färber
@ 2014-03-09 21:47   ` Leandro Dorileo
  0 siblings, 0 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-09 21:47 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek, Peter Lieven

Hi Andreas,


On Sun, Mar 09, 2014 at 05:32:17PM +0100, Andreas Färber wrote:
> Am 08.03.2014 19:47, schrieb Leandro Dorileo:
> > The following patchset introduces a general purpose argument parser and migrates
> > qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> > good feedback here I move forward and migrate all the other possible users.
> 
> Why? :) You forgot to describe what's wrong with the current
> infrastructure, how your approach is different and what the benefit is.
>

Indead, I failed miserably on that (describing the problem intended to be solved).
The general sense is to unify the argument parsing strategies with some benefits
like help generation. If you see, every qemu binary has its own strategy - where
some (most?) use getopt and the QEMU main binary uses QemuOpt, they lack a runtine
help message generation, the sync between the help message and expected/implemented
arguments are done manually.

But I'm not sure if we really want/need it, that's the why I sent an early RFC. For
me it was just few weekend hours putting all together. :)

I see a good benefit on that - having a simple, clean, consistent and common API
throughout the source code. But again, do we want it? is it worth it?

Regards...

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
                   ` (3 preceding siblings ...)
  2014-03-09 16:32 ` Andreas Färber
@ 2014-03-11 11:06 ` Kevin Wolf
  2014-03-11 14:09   ` Leandro Dorileo
  4 siblings, 1 reply; 15+ messages in thread
From: Kevin Wolf @ 2014-03-11 11:06 UTC (permalink / raw)
  To: Leandro Dorileo
  Cc: Peter Lieven, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek

Am 08.03.2014 um 19:47 hat Leandro Dorileo geschrieben:
> The following patchset introduces a general purpose argument parser and migrates
> qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> good feedback here I move forward and migrate all the other possible users.

I was planning to reply to this in more detail, but it doesn't look like
I can find the time to do so, so let me just summarise my thoughts
briefly.

I do like the idea of simplifying qemu-img's argument parsing, but we
shouldn't make the mistake of introducing another option parsing
infrastructure and end up with three different coexisting models. If we
were to introduce a new framework, we must make sure that all code is
converted to it and QemuOpts can be dropped.

Now replacing QemuOpts and all of its users is not something that seems
to be very productive - it works quite well in those places. So I think
we should rather extend QemuOpts to work for qemu-img as well.

We would probably need to add a new parser to QemuOpts that parses
command line options into a QemuOpts, and extend the definition of them
with a couple of new features that are required there (sub-QemuOpts for
-o, perhaps enumerations for things like --output=human/json, positional
parameters).

I see that you also fill the values in (global) variables. The existing
code that converts QemuOpts into C variables are the QAPI visitors. So I
could imagine that we define all qemu-img options in a JSON schema like
qapi-schema.json and generate the struct definitions from it. We would
then end up with something like this, where the code to create a
QemuImgCreateOptions struct from the QemuOpts would be QAPI generated:

void img_create(QemuImgCreateOptions *options, Error **errp);

Not sure if we really want to go that far, but perhaps it's some food
for thought.

Kevin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-11 11:06 ` Kevin Wolf
@ 2014-03-11 14:09   ` Leandro Dorileo
  2014-03-11 15:22     ` Eric Blake
  0 siblings, 1 reply; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-11 14:09 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Peter Lieven, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek

Hi Kevin,

On Tue, Mar 11, 2014 at 12:06:16PM +0100, Kevin Wolf wrote:
> Am 08.03.2014 um 19:47 hat Leandro Dorileo geschrieben:
> > The following patchset introduces a general purpose argument parser and migrates
> > qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> > good feedback here I move forward and migrate all the other possible users.
> 
> I was planning to reply to this in more detail, but it doesn't look like
> I can find the time to do so, so let me just summarise my thoughts
> briefly.

Ok.

> 
> I do like the idea of simplifying qemu-img's argument parsing, but we
> shouldn't make the mistake of introducing another option parsing
> infrastructure and end up with three different coexisting models. If we
> were to introduce a new framework, we must make sure that all code is
> converted to it and QemuOpts can be dropped.

Agreed.

> 
> Now replacing QemuOpts and all of its users is not something that seems
> to be very productive - it works quite well in those places. So I think
> we should rather extend QemuOpts to work for qemu-img as well.

Indeed, I took some time yesterday to take a deeper look at QemuOpt users,
I saw that converting it all to an entirely new framework would be a massive
task and maybe not worth it. Extending QemuOpts to new needs seems to be a more
reasonable thing.

> 
> We would probably need to add a new parser to QemuOpts that parses
> command line options into a QemuOpts, and extend the definition of them
> with a couple of new features that are required there (sub-QemuOpts for
> -o, perhaps enumerations for things like --output=human/json, positional
> parameters).

Ok.

> 
> I see that you also fill the values in (global) variables. The existing
> code that converts QemuOpts into C variables are the QAPI visitors. So I
> could imagine that we define all qemu-img options in a JSON schema like
> qapi-schema.json and generate the struct definitions from it. We would
> then end up with something like this, where the code to create a
> QemuImgCreateOptions struct from the QemuOpts would be QAPI generated:
> 
> void img_create(QemuImgCreateOptions *options, Error **errp);

Using json descriptors was something I considered after sending the patches
and the feedbacks I already got. That's something that would be useful on
cases of generating different outputs from it, like manpages or even
fragments of it, it would be more flexible as far as I can see.

But yeah, I need to take a look at qapi and see how things work.

> 
> Not sure if we really want to go that far, but perhaps it's some food
> for thought.

Yeah, sure.

Regards...

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-11 14:09   ` Leandro Dorileo
@ 2014-03-11 15:22     ` Eric Blake
  2014-03-16 21:23       ` Leandro Dorileo
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2014-03-11 15:22 UTC (permalink / raw)
  To: Leandro Dorileo, Kevin Wolf
  Cc: Peter Lieven, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

On 03/11/2014 08:09 AM, Leandro Dorileo wrote:
> Hi Kevin,
> 
> On Tue, Mar 11, 2014 at 12:06:16PM +0100, Kevin Wolf wrote:
>> Am 08.03.2014 um 19:47 hat Leandro Dorileo geschrieben:
>>> The following patchset introduces a general purpose argument parser and migrates
>>> qemu-img to make use of it. qemu-img is just the first user of it, if we see a
>>> good feedback here I move forward and migrate all the other possible users.
>>
>> I was planning to reply to this in more detail, but it doesn't look like
>> I can find the time to do so, so let me just summarise my thoughts
>> briefly.
> 
> Ok.
> 
>>
>> I do like the idea of simplifying qemu-img's argument parsing, but we
>> shouldn't make the mistake of introducing another option parsing
>> infrastructure and end up with three different coexisting models. If we
>> were to introduce a new framework, we must make sure that all code is
>> converted to it and QemuOpts can be dropped.
> 
> Agreed.

For that matter, if you want to help the current conversion efforts
going on, we are trying to get rid of QEMUOptionParameters to _just_ use
QemuOpts:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg01728.html


>>
>> We would probably need to add a new parser to QemuOpts that parses
>> command line options into a QemuOpts, and extend the definition of them
>> with a couple of new features that are required there (sub-QemuOpts for
>> -o, perhaps enumerations for things like --output=human/json, positional
>> parameters).
> 
> Ok.

We also need to think how to expose those improvements via the QMP
command query-command-line-arguments; lots of details in this thread:
https://lists.gnu.org/archive/html/qemu-devel/2014-03/threads.html#00934

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser
  2014-03-11 15:22     ` Eric Blake
@ 2014-03-16 21:23       ` Leandro Dorileo
  0 siblings, 0 replies; 15+ messages in thread
From: Leandro Dorileo @ 2014-03-16 21:23 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, Peter Maydell, Fam Zheng, Stefan Weil,
	Michael Tokarev, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Laszlo Ersek, Peter Lieven

Hi Erick,

On Tue, Mar 11, 2014 at 09:22:54AM -0600, Eric Blake wrote:
> On 03/11/2014 08:09 AM, Leandro Dorileo wrote:
> > Hi Kevin,
> > 
> > On Tue, Mar 11, 2014 at 12:06:16PM +0100, Kevin Wolf wrote:
> >> Am 08.03.2014 um 19:47 hat Leandro Dorileo geschrieben:
> >>> The following patchset introduces a general purpose argument parser and migrates
> >>> qemu-img to make use of it. qemu-img is just the first user of it, if we see a
> >>> good feedback here I move forward and migrate all the other possible users.
> >>
> >> I was planning to reply to this in more detail, but it doesn't look like
> >> I can find the time to do so, so let me just summarise my thoughts
> >> briefly.
> > 
> > Ok.
> > 
> >>
> >> I do like the idea of simplifying qemu-img's argument parsing, but we
> >> shouldn't make the mistake of introducing another option parsing
> >> infrastructure and end up with three different coexisting models. If we
> >> were to introduce a new framework, we must make sure that all code is
> >> converted to it and QemuOpts can be dropped.
> > 
> > Agreed.
> 
> For that matter, if you want to help the current conversion efforts
> going on, we are trying to get rid of QEMUOptionParameters to _just_ use
> QemuOpts:
> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg01728.html

Yep sure, I have followed the current patches and reviews, I have just
posted a patch with an initial testsuite for QemuOpt in the sense to help to have it
integrated.

> 
> 
> >>
> >> We would probably need to add a new parser to QemuOpts that parses
> >> command line options into a QemuOpts, and extend the definition of them
> >> with a couple of new features that are required there (sub-QemuOpts for
> >> -o, perhaps enumerations for things like --output=human/json, positional
> >> parameters).
> > 
> > Ok.
> 
> We also need to think how to expose those improvements via the QMP
> command query-command-line-arguments; lots of details in this thread:
> https://lists.gnu.org/archive/html/qemu-devel/2014-03/threads.html#00934

Will take a look at it.

Thanks...

-- 
Leandro Dorileo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-03-16 21:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-08 18:47 [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Leandro Dorileo
2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 1/2] qemu-arg: introduce a " Leandro Dorileo
2014-03-08 18:47 ` [Qemu-devel] [PATCH RFC 2/2] qemu-img: migrate to use qemu-arg Leandro Dorileo
2014-03-09  7:30   ` Paolo Bonzini
2014-03-09 12:37     ` Leandro Dorileo
2014-03-09 13:03       ` Peter Maydell
2014-03-09 13:35         ` Leandro Dorileo
2014-03-08 18:55 ` [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser Peter Maydell
2014-03-08 20:28   ` Leandro Dorileo
2014-03-09 16:32 ` Andreas Färber
2014-03-09 21:47   ` Leandro Dorileo
2014-03-11 11:06 ` Kevin Wolf
2014-03-11 14:09   ` Leandro Dorileo
2014-03-11 15:22     ` Eric Blake
2014-03-16 21:23       ` Leandro Dorileo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.