qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
@ 2014-05-07  9:58 Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling Chunyan Liu
                   ` (25 more replies)
  0 siblings, 26 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

This patch series is to replace QEMUOptionParameter with QemuOpts, so that only
one Qemu Option structure is kept in QEMU code.

---
Changes to v26:
  * Following Eric's comment, backward split 2/33, 3/33.
    (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc later).
  * Fix memory free in qemu_opts_append to solve iotest issue. 10/33
  * Following Eric's comment, remove the end '.' in error message. And update
    qemu-iotests .out file. 12/33
  * Following Eric's comment, fix memory free in vvfat.c 13/33
  * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33:
    export qemu_opt_find first, add qcow2 driver patch later.
  * rebase to git master

All patches are also available from:
https://github.com/chunyanliu/qemu/commits/QemuOpts


Chunyan Liu (33):
  QemuOpts: move find_desc_by_name ahead for later calling
  QemuOpts: repurpose qemu_opts_print to replace
    print_option_parameters
  QemuOpts: add def_value_str to QemuOptDesc
  qapi: output def_value_str when query command line options
  QemuOpts: change opt->name|str from (const char *) to (char *)
  QemuOpts: move qemu_opt_del ahead for later calling
  QemuOpts: add qemu_opt_get_*_del functions for replace work
  QemuOpts: add qemu_opts_print_help to replace print_option_help
  QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
  QemuOpts: add qemu_opts_append to replace append_option_parameters
  QemuOpts: check NULL input for qemu_opts_del
  change block layer to support both QemuOpts and QEMUOptionParamter
  vvfat.c: handle cross_driver's create_options and create_opts
  cow.c: replace QEMUOptionParameter with QemuOpts
  gluster.c: replace QEMUOptionParameter with QemuOpts
  iscsi.c: replace QEMUOptionParameter with QemuOpts
  nfs.c: replace QEMUOptionParameter with QemuOpts
  qcow.c: replace QEMUOptionParameter with QemuOpts
  QemuOpts: export qemu_opt_find
  qcow2.c: replace QEMUOptionParameter with QemuOpts
  qed.c: replace QEMUOptionParameter with QemuOpts
  raw-posix.c: replace QEMUOptionParameter with QemuOpts
  raw-win32.c: replace QEMUOptionParameter with QemuOpts
  raw_bsd.c: replace QEMUOptionParameter with QemuOpts
  rbd.c: replace QEMUOptionParameter with QemuOpts
  sheepdog.c: replace QEMUOptionParameter with QemuOpts
  ssh.c: replace QEMUOptionParameter with QemuOpts
  vdi.c: replace QEMUOptionParameter with QemuOpts
  vhdx.c: replace QEMUOptionParameter with QemuOpts
  vmdk.c: replace QEMUOptionParameter with QemuOpts
  vpc.c: replace QEMUOptionParameter with QemuOpts
  cleanup QEMUOptionParameter
  QemuOpts: cleanup tmp 'allocated' member from QemuOptsList

 block.c                    |  99 ++++----
 block/cow.c                |  52 ++--
 block/gluster.c            |  73 +++---
 block/iscsi.c              |  32 ++-
 block/nfs.c                |  10 +-
 block/qcow.c               |  72 +++---
 block/qcow2.c              | 259 ++++++++++----------
 block/qed.c                | 112 +++++----
 block/qed.h                |   3 +-
 block/raw-posix.c          |  55 ++---
 block/raw-win32.c          |  38 +--
 block/raw_bsd.c            |  25 +-
 block/rbd.c                |  61 +++--
 block/sheepdog.c           | 102 ++++----
 block/ssh.c                |  30 ++-
 block/vdi.c                |  71 +++---
 block/vhdx.c               |  97 ++++----
 block/vhdx.h               |   1 +
 block/vmdk.c               | 121 +++++-----
 block/vpc.c                |  60 ++---
 block/vvfat.c              |  13 +-
 include/block/block.h      |   7 +-
 include/block/block_int.h  |   9 +-
 include/qemu/option.h      |  53 +---
 include/qemu/option_int.h  |   4 +-
 qapi-schema.json           |   5 +-
 qapi/opts-visitor.c        |  10 +-
 qemu-img.c                 |  89 +++----
 qmp-commands.hx            |   2 +
 tests/qemu-iotests/049.out |   2 +-
 tests/qemu-iotests/061.out |   2 +-
 util/qemu-config.c         |   4 +
 util/qemu-option.c         | 588 ++++++++++++++++++++-------------------------
 33 files changed, 1033 insertions(+), 1128 deletions(-)

-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters Chunyan Liu
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 util/qemu-option.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8bbc3ad..808aef4 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -173,6 +173,20 @@ static void parse_option_number(const char *name, const char *value,
     }
 }
 
+static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
+                                            const char *name)
+{
+    int i;
+
+    for (i = 0; desc[i].name != NULL; i++) {
+        if (strcmp(desc[i].name, name) == 0) {
+            return &desc[i];
+        }
+    }
+
+    return NULL;
+}
+
 void parse_option_size(const char *name, const char *value,
                        uint64_t *ret, Error **errp)
 {
@@ -637,20 +651,6 @@ static bool opts_accepts_any(const QemuOpts *opts)
     return opts->list->desc[0].name == NULL;
 }
 
-static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
-                                            const char *name)
-{
-    int i;
-
-    for (i = 0; desc[i].name != NULL; i++) {
-        if (strcmp(desc[i].name, name) == 0) {
-            return &desc[i];
-        }
-    }
-
-    return NULL;
-}
-
 int qemu_opt_unset(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc Chunyan Liu
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Currently this function is not used anywhere. In later patches, it will
replace print_option_parameters. To avoid print info changes, change
qemu_opts_print from fprintf stderr to printf, and remove last printf.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * backward split than v26 (2/33, 3/33).

 include/qemu/option.h |  2 +-
 util/qemu-option.c    | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..1077b69 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -156,7 +156,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
 void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
 
 typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
-int qemu_opts_print(QemuOpts *opts, void *dummy);
+void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
                       int abort_on_failure);
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 808aef4..290ed54 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -895,17 +895,15 @@ void qemu_opts_del(QemuOpts *opts)
     g_free(opts);
 }
 
-int qemu_opts_print(QemuOpts *opts, void *dummy)
+void qemu_opts_print(QemuOpts *opts)
 {
     QemuOpt *opt;
 
-    fprintf(stderr, "%s: %s:", opts->list->name,
-            opts->id ? opts->id : "<noid>");
+    printf("%s: %s:", opts->list->name,
+           opts->id ? opts->id : "<noid>");
     QTAILQ_FOREACH(opt, &opts->head, next) {
-        fprintf(stderr, " %s=\"%s\"", opt->name, opt->str);
+        printf(" %s=\"%s\"", opt->name, opt->str);
     }
-    fprintf(stderr, "\n");
-    return 0;
 }
 
 static int opts_do_parse(QemuOpts *opts, const char *params,
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options Chunyan Liu
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Add def_value_str (default value) to QemuOptDesc, to replace function of the
default value in QEMUOptionParameter.

Improve qemu_opts_get_* functions: if find opt, return opt->str; otherwise,
if desc->def_value_str is set, return desc->def_value_str; otherwise, return
input defval.

Improve qemu_opts_print: if option is set, print opt->str; otherwise, if
desc->def_value_str is set, print it.

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * backward split than v26. (2/33, 3/33)

 include/qemu/option.h |  1 +
 util/qemu-option.c    | 56 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1077b69..c3b0a91 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -99,6 +99,7 @@ typedef struct QemuOptDesc {
     const char *name;
     enum QemuOptType type;
     const char *help;
+    const char *def_value_str;
 } QemuOptDesc;
 
 struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 290ed54..2be6995 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
+
+    if (!opt) {
+        const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+        if (desc && desc->def_value_str) {
+            return desc->def_value_str;
+        }
+    }
     return opt ? opt->str : NULL;
 }
 
@@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
 
-    if (opt == NULL)
+    if (opt == NULL) {
+        const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+        if (desc && desc->def_value_str) {
+            parse_option_bool(name, desc->def_value_str, &defval, &error_abort);
+        }
         return defval;
+    }
     assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
     return opt->value.boolean;
 }
@@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
 
-    if (opt == NULL)
+    if (opt == NULL) {
+        const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+        if (desc && desc->def_value_str) {
+            parse_option_number(name, desc->def_value_str, &defval,
+                                &error_abort);
+        }
         return defval;
+    }
     assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER);
     return opt->value.uint;
 }
@@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
 
-    if (opt == NULL)
+    if (opt == NULL) {
+        const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
+        if (desc && desc->def_value_str) {
+            parse_option_size(name, desc->def_value_str, &defval, &error_abort);
+        }
         return defval;
+    }
     assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE);
     return opt->value.uint;
 }
@@ -898,11 +921,30 @@ void qemu_opts_del(QemuOpts *opts)
 void qemu_opts_print(QemuOpts *opts)
 {
     QemuOpt *opt;
+    QemuOptDesc *desc = opts->list->desc;
 
-    printf("%s: %s:", opts->list->name,
-           opts->id ? opts->id : "<noid>");
-    QTAILQ_FOREACH(opt, &opts->head, next) {
-        printf(" %s=\"%s\"", opt->name, opt->str);
+    if (desc[0].name == NULL) {
+        QTAILQ_FOREACH(opt, &opts->head, next) {
+            printf("%s=\"%s\" ", opt->name, opt->str);
+        }
+        return;
+    }
+    for (; desc && desc->name; desc++) {
+        const char *value;
+        QemuOpt *opt = qemu_opt_find(opts, desc->name);
+
+        value = opt ? opt->str : desc->def_value_str;
+        if (!value) {
+            continue;
+        }
+        if (desc->type == QEMU_OPT_STRING) {
+            printf("%s='%s' ", desc->name, value);
+        } else if ((desc->type == QEMU_OPT_SIZE ||
+                    desc->type == QEMU_OPT_NUMBER) && opt) {
+            printf("%s=%" PRId64 " ", desc->name, opt->value.uint);
+        } else {
+            printf("%s=%s ", desc->name, value);
+        }
     }
 }
 
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (2 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *) Chunyan Liu
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Change qapi interfaces to output the newly added def_value_str when querying
command line options.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 qapi-schema.json   | 5 ++++-
 qmp-commands.hx    | 2 ++
 util/qemu-config.c | 4 ++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0b00427..880829d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4088,12 +4088,15 @@
 #
 # @help: #optional human readable text string, not suitable for parsing.
 #
+# @default: #optional default value string (since 2.1)
+#
 # Since 1.5
 ##
 { 'type': 'CommandLineParameterInfo',
   'data': { 'name': 'str',
             'type': 'CommandLineParameterType',
-            '*help': 'str' } }
+            '*help': 'str',
+            '*default': 'str' } }
 
 ##
 # @CommandLineOptionInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ed3ab92..1271332 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2895,6 +2895,8 @@ Each array entry contains the following:
               or 'size')
     - "help": human readable description of the parameter
               (json-string, optional)
+    - "default": default value string for the parameter
+                 (json-string, optional)
 
 Example:
 
diff --git a/util/qemu-config.c b/util/qemu-config.c
index f4e4f38..ba375c0 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -82,6 +82,10 @@ static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc)
             info->has_help = true;
             info->help = g_strdup(desc[i].help);
         }
+        if (desc[i].def_value_str) {
+            info->has_q_default = true;
+            info->q_default = g_strdup(desc[i].def_value_str);
+        }
 
         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *)
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (3 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling Chunyan Liu
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead.  By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 include/qemu/option_int.h |  4 ++--
 qapi/opts-visitor.c       | 10 +++++++---
 util/qemu-option.c        |  4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 8212fa4..6432c1a 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -30,8 +30,8 @@
 #include "qemu/error-report.h"
 
 struct QemuOpt {
-    const char   *name;
-    const char   *str;
+    char *name;
+    char *str;
 
     const QemuOptDesc *desc;
     union {
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5d830a2..7a64f4e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -143,8 +143,8 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
     if (ov->opts_root->id != NULL) {
         ov->fake_id_opt = g_malloc0(sizeof *ov->fake_id_opt);
 
-        ov->fake_id_opt->name = "id";
-        ov->fake_id_opt->str = ov->opts_root->id;
+        ov->fake_id_opt->name = g_strdup("id");
+        ov->fake_id_opt->str = g_strdup(ov->opts_root->id);
         opts_visitor_insert(ov->unprocessed_opts, ov->fake_id_opt);
     }
 }
@@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp)
     }
     g_hash_table_destroy(ov->unprocessed_opts);
     ov->unprocessed_opts = NULL;
-    g_free(ov->fake_id_opt);
+    if (ov->fake_id_opt) {
+        g_free(ov->fake_id_opt->name);
+        g_free(ov->fake_id_opt->str);
+        g_free(ov->fake_id_opt);
+    }
     ov->fake_id_opt = NULL;
 }
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2be6995..69cdf3f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
 static void qemu_opt_del(QemuOpt *opt)
 {
     QTAILQ_REMOVE(&opt->opts->head, opt, next);
-    g_free((/* !const */ char*)opt->name);
-    g_free((/* !const */ char*)opt->str);
+    g_free(opt->name);
+    g_free(opt->str);
     g_free(opt);
 }
 
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (4 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *) Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work Chunyan Liu
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

In later patch, qemu_opt_get_del functions will be added, they will
first get the option value, then call qemu_opt_del to remove the option
from opt list. To prepare for that purpose, move qemu_opt_del ahead first.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 util/qemu-option.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 69cdf3f..4d2d4d1 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -567,6 +567,14 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
     return NULL;
 }
 
+static void qemu_opt_del(QemuOpt *opt)
+{
+    QTAILQ_REMOVE(&opt->opts->head, opt, next);
+    g_free(opt->name);
+    g_free(opt->str);
+    g_free(opt);
+}
+
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
@@ -661,14 +669,6 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
     }
 }
 
-static void qemu_opt_del(QemuOpt *opt)
-{
-    QTAILQ_REMOVE(&opt->opts->head, opt, next);
-    g_free(opt->name);
-    g_free(opt->str);
-    g_free(opt);
-}
-
 static bool opts_accepts_any(const QemuOpts *opts)
 {
     return opts->list->desc[0].name == NULL;
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (5 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-06-04  8:54   ` Stefan Hajnoczi
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help Chunyan Liu
                   ` (18 subsequent siblings)
  25 siblings, 1 reply; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Add qemu_opt_get_del, qemu_opt_get_bool_del, qemu_opt_get_number_del and
qemu_opt_get_size_del to replace the same handling of QEMUOptionParameter
(get and delete).

Several drivers are coded to parse a known subset of options, then
remove them from the list before handing all remaining options to a
second driver for further option processing.  get_*_del makes it easier
to retrieve a known option (or its default) and remove it from the list
all in one action.

Share common helper function:

For qemu_opt_get_bool/size/number, they and their get_*_del counterpart
could share most of the code except whether or not deleting the opt from
option list, so generate common helper functions.

For qemu_opt_get and qemu_opt_get_del, keep code duplication, since
1. qemu_opt_get_del returns malloc'd memory while qemu_opt_get returns
in-place memory
2. qemu_opt_get_del returns (char *), qemu_opt_get returns (const char *),
and could not change to (char *), since in one case, it will return
desc->def_value_str, which is (const char *).

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 include/qemu/option.h |   6 +++
 util/qemu-option.c    | 116 ++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 109 insertions(+), 13 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index c3b0a91..6653e43 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -111,6 +111,7 @@ struct QemuOptsList {
 };
 
 const char *qemu_opt_get(QemuOpts *opts, const char *name);
+char *qemu_opt_get_del(QemuOpts *opts, const char *name);
 /**
  * qemu_opt_has_help_opt:
  * @opts: options to search for a help request
@@ -126,6 +127,11 @@ bool qemu_opt_has_help_opt(QemuOpts *opts);
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
+bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval);
+uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
+                                 uint64_t defval);
+uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
+                               uint64_t defval);
 int qemu_opt_unset(QemuOpts *opts, const char *name);
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
 void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 4d2d4d1..32e1d50 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -575,6 +575,19 @@ static void qemu_opt_del(QemuOpt *opt)
     g_free(opt);
 }
 
+/* qemu_opt_set allows many settings for the same option.
+ * This function deletes all settings for an option.
+ */
+static void qemu_opt_del_all(QemuOpts *opts, const char *name)
+{
+    QemuOpt *opt, *next_opt;
+
+    QTAILQ_FOREACH_SAFE(opt, &opts->head, next, next_opt) {
+        if (!strcmp(opt->name, name))
+            qemu_opt_del(opt);
+    }
+}
+
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
@@ -588,6 +601,34 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
     return opt ? opt->str : NULL;
 }
 
+/* Get a known option (or its default) and remove it from the list
+ * all in one action. Return a malloced string of the option value.
+ * Result must be freed by caller with g_free().
+ */
+char *qemu_opt_get_del(QemuOpts *opts, const char *name)
+{
+    QemuOpt *opt;
+    const QemuOptDesc *desc;
+    char *str = NULL;
+
+    if (opts == NULL) {
+        return NULL;
+    }
+
+    opt = qemu_opt_find(opts, name);
+    if (!opt) {
+        desc = find_desc_by_name(opts->list->desc, name);
+        if (desc && desc->def_value_str) {
+            str = g_strdup(desc->def_value_str);
+        }
+        return str;
+    }
+    str = opt->str;
+    opt->str = NULL;
+    qemu_opt_del_all(opts, name);
+    return str;
+}
+
 bool qemu_opt_has_help_opt(QemuOpts *opts)
 {
     QemuOpt *opt;
@@ -600,50 +641,99 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
     return false;
 }
 
-bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
+static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
+                                     bool defval, bool del)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
+    bool ret = defval;
 
     if (opt == NULL) {
         const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
         if (desc && desc->def_value_str) {
-            parse_option_bool(name, desc->def_value_str, &defval, &error_abort);
+            parse_option_bool(name, desc->def_value_str, &ret, &error_abort);
         }
-        return defval;
+        return ret;
     }
     assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL);
-    return opt->value.boolean;
+    ret = opt->value.boolean;
+    if (del) {
+        qemu_opt_del_all(opts, name);
+    }
+    return ret;
 }
 
-uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
+bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
+{
+    return qemu_opt_get_bool_helper(opts, name, defval, false);
+}
+
+bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval)
+{
+    return qemu_opt_get_bool_helper(opts, name, defval, true);
+}
+
+static uint64_t qemu_opt_get_number_helper(QemuOpts *opts, const char *name,
+                                           uint64_t defval, bool del)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
+    uint64_t ret = defval;
 
     if (opt == NULL) {
         const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
         if (desc && desc->def_value_str) {
-            parse_option_number(name, desc->def_value_str, &defval,
-                                &error_abort);
+            parse_option_number(name, desc->def_value_str, &ret, &error_abort);
         }
-        return defval;
+        return ret;
     }
     assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER);
-    return opt->value.uint;
+    ret = opt->value.uint;
+    if (del) {
+        qemu_opt_del_all(opts, name);
+    }
+    return ret;
 }
 
-uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
+uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval)
+{
+    return qemu_opt_get_number_helper(opts, name, defval, false);
+}
+
+uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
+                                 uint64_t defval)
+{
+    return qemu_opt_get_number_helper(opts, name, defval, true);
+}
+
+static uint64_t qemu_opt_get_size_helper(QemuOpts *opts, const char *name,
+                                         uint64_t defval, bool del)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
+    uint64_t ret = defval;
 
     if (opt == NULL) {
         const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);
         if (desc && desc->def_value_str) {
-            parse_option_size(name, desc->def_value_str, &defval, &error_abort);
+            parse_option_size(name, desc->def_value_str, &ret, &error_abort);
         }
-        return defval;
+        return ret;
     }
     assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE);
-    return opt->value.uint;
+    ret = opt->value.uint;
+    if (del) {
+        qemu_opt_del_all(opts, name);
+    }
+    return ret;
+}
+
+uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
+{
+    return qemu_opt_get_size_helper(opts, name, defval, false);
+}
+
+uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
+                               uint64_t defval)
+{
+    return qemu_opt_get_size_helper(opts, name, defval, true);
 }
 
 static void qemu_opt_parse(QemuOpt *opt, Error **errp)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (6 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts Chunyan Liu
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

print_option_help takes QEMUOptionParameter as parameter, add
qemu_opts_print_help to take QemuOptsList as parameter for later
replace work.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 include/qemu/option.h |  1 +
 util/qemu-option.c    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 6653e43..fbf5dc2 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -166,5 +166,6 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
 void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
                       int abort_on_failure);
+void qemu_opts_print_help(QemuOptsList *list);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 32e1d50..adb7c3c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -553,6 +553,19 @@ void print_option_help(QEMUOptionParameter *list)
     }
 }
 
+void qemu_opts_print_help(QemuOptsList *list)
+{
+    QemuOptDesc *desc;
+
+    assert(list);
+    desc = list->desc;
+    printf("Supported options:\n");
+    while (desc && desc->name) {
+        printf("%-16s %s\n", desc->name,
+               desc->help ? desc->help : "No description available");
+        desc++;
+    }
+}
 /* ------------------------------------------------------------------ */
 
 static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (7 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters Chunyan Liu
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Add two temp conversion functions between QEMUOptionParameter to QemuOpts,
so that next patch can use it. It will simplify later patch for easier
review. And will be finally removed after all backend drivers switch to
QemuOpts.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 include/qemu/option.h |   9 +++
 util/qemu-option.c    | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index fbf5dc2..e98e8ef 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,6 +103,12 @@ typedef struct QemuOptDesc {
 } QemuOptDesc;
 
 struct QemuOptsList {
+    /* FIXME: Temp used for QEMUOptionParamter->QemuOpts conversion to
+     * indicate the need to free memory. Will remove after all drivers
+     * switch to QemuOpts.
+     */
+    bool allocated;
+
     const char *name;
     const char *implied_opt_name;
     bool merge_lists;  /* Merge multiple uses of option into a single list? */
@@ -167,5 +173,8 @@ void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
                       int abort_on_failure);
 void qemu_opts_print_help(QemuOptsList *list);
+void qemu_opts_free(QemuOptsList *list);
+QEMUOptionParameter *opts_to_params(QemuOpts *opts);
+QemuOptsList *params_to_opts(QEMUOptionParameter *list);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index adb7c3c..93ffcd5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1345,3 +1345,156 @@ int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
     loc_pop(&loc);
     return rc;
 }
+
+static size_t count_opts_list(QemuOptsList *list)
+{
+    QemuOptDesc *desc = NULL;
+    size_t num_opts = 0;
+
+    if (!list) {
+        return 0;
+    }
+
+    desc = list->desc;
+    while (desc && desc->name) {
+        num_opts++;
+        desc++;
+    }
+
+    return num_opts;
+}
+
+/* Convert QEMUOptionParameter to QemuOpts
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QemuOptsList *params_to_opts(QEMUOptionParameter *list)
+{
+    QemuOptsList *opts = NULL;
+    size_t num_opts, i = 0;
+
+    if (!list) {
+        return NULL;
+    }
+
+    num_opts = count_option_parameters(list);
+    opts = g_malloc0(sizeof(QemuOptsList) +
+                     (num_opts + 1) * sizeof(QemuOptDesc));
+    QTAILQ_INIT(&opts->head);
+    /* (const char *) members will point to malloced space and need to free */
+    opts->allocated = true;
+
+    while (list && list->name) {
+        opts->desc[i].name = g_strdup(list->name);
+        opts->desc[i].help = g_strdup(list->help);
+        switch (list->type) {
+        case OPT_FLAG:
+            opts->desc[i].type = QEMU_OPT_BOOL;
+            opts->desc[i].def_value_str =
+                g_strdup(list->value.n ? "on" : "off");
+            break;
+
+        case OPT_NUMBER:
+            opts->desc[i].type = QEMU_OPT_NUMBER;
+            if (list->value.n) {
+                opts->desc[i].def_value_str =
+                    g_strdup_printf("%" PRIu64, list->value.n);
+            }
+            break;
+
+        case OPT_SIZE:
+            opts->desc[i].type = QEMU_OPT_SIZE;
+            if (list->value.n) {
+                opts->desc[i].def_value_str =
+                    g_strdup_printf("%" PRIu64, list->value.n);
+            }
+            break;
+
+        case OPT_STRING:
+            opts->desc[i].type = QEMU_OPT_STRING;
+            opts->desc[i].def_value_str = g_strdup(list->value.s);
+            break;
+        }
+
+        i++;
+        list++;
+    }
+
+    return opts;
+}
+
+/* convert QemuOpts to QEMUOptionParameter
+ * Note: result QEMUOptionParameter has shorter lifetime than
+ * input QemuOpts.
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QEMUOptionParameter *opts_to_params(QemuOpts *opts)
+{
+    QEMUOptionParameter *dest = NULL;
+    QemuOptDesc *desc;
+    size_t num_opts, i = 0;
+    const char *tmp;
+
+    if (!opts || !opts->list || !opts->list->desc) {
+        return NULL;
+    }
+    assert(!opts_accepts_any(opts));
+
+    num_opts = count_opts_list(opts->list);
+    dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
+
+    desc = opts->list->desc;
+    while (desc && desc->name) {
+        dest[i].name = desc->name;
+        dest[i].help = desc->help;
+        dest[i].assigned = qemu_opt_find(opts, desc->name) ? true : false;
+        switch (desc->type) {
+        case QEMU_OPT_STRING:
+            dest[i].type = OPT_STRING;
+            tmp = qemu_opt_get(opts, desc->name);
+            dest[i].value.s = g_strdup(tmp);
+            break;
+
+        case QEMU_OPT_BOOL:
+            dest[i].type = OPT_FLAG;
+            dest[i].value.n = qemu_opt_get_bool(opts, desc->name, 0) ? 1 : 0;
+            break;
+
+        case QEMU_OPT_NUMBER:
+            dest[i].type = OPT_NUMBER;
+            dest[i].value.n = qemu_opt_get_number(opts, desc->name, 0);
+            break;
+
+        case QEMU_OPT_SIZE:
+            dest[i].type = OPT_SIZE;
+            dest[i].value.n = qemu_opt_get_size(opts, desc->name, 0);
+            break;
+        }
+
+        i++;
+        desc++;
+    }
+
+    return dest;
+}
+
+void qemu_opts_free(QemuOptsList *list)
+{
+    /* List members point to new malloced space and need to be freed.
+     * FIXME:
+     * Introduced for QEMUOptionParamter->QemuOpts conversion.
+     * Will remove after all drivers switch to QemuOpts.
+     */
+    if (list && list->allocated) {
+        QemuOptDesc *desc = list->desc;
+        while (desc && desc->name) {
+            g_free((char *)desc->name);
+            g_free((char *)desc->help);
+            g_free((char *)desc->def_value_str);
+            desc++;
+        }
+    }
+
+    g_free(list);
+}
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (8 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del Chunyan Liu
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

For later merge .create_opts of drv and proto_drv in qemu-img commands.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * fix memory free:
     if (param) {
   -     g_free(list);
   +     qemu_opts_free(list); //free allocated memory too
     }


 include/qemu/option.h |  5 ++++
 util/qemu-option.c    | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index e98e8ef..44d9961 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -176,5 +176,10 @@ void qemu_opts_print_help(QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
 QEMUOptionParameter *opts_to_params(QemuOpts *opts);
 QemuOptsList *params_to_opts(QEMUOptionParameter *list);
+/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+                               QemuOptsList *list,
+                               QEMUOptionParameter *param);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 93ffcd5..e15723a 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1498,3 +1498,70 @@ void qemu_opts_free(QemuOptsList *list)
 
     g_free(list);
 }
+
+/* Realloc dst option list and append options either from an option list (list)
+ * or a QEMUOptionParameter (param) to it. dst could be NULL or a malloced list.
+ * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+                               QemuOptsList *list,
+                               QEMUOptionParameter *param)
+{
+    size_t num_opts, num_dst_opts;
+    QemuOptDesc *desc;
+    bool need_init = false;
+
+    assert(!(list && param));
+    if (!param && !list) {
+        return dst;
+    }
+
+    if (param) {
+        list = params_to_opts(param);
+    }
+
+    /* If dst is NULL, after realloc, some area of dst should be initialized
+     * before adding options to it.
+     */
+    if (!dst) {
+        need_init = true;
+    }
+
+    num_opts = count_opts_list(dst);
+    num_dst_opts = num_opts;
+    num_opts += count_opts_list(list);
+    dst = g_realloc(dst, sizeof(QemuOptsList) +
+                    (num_opts + 1) * sizeof(QemuOptDesc));
+    if (need_init) {
+        dst->name = NULL;
+        dst->implied_opt_name = NULL;
+        QTAILQ_INIT(&dst->head);
+        dst->allocated = true;
+        dst->merge_lists = false;
+    }
+    dst->desc[num_dst_opts].name = NULL;
+
+    /* (const char *) members of result dst are malloced, need free. */
+    assert(dst->allocated);
+    /* append list->desc to dst->desc */
+    if (list) {
+        desc = list->desc;
+        while (desc && desc->name) {
+            if (find_desc_by_name(dst->desc, desc->name) == NULL) {
+                dst->desc[num_dst_opts].name = g_strdup(desc->name);
+                dst->desc[num_dst_opts].type = desc->type;
+                dst->desc[num_dst_opts].help = g_strdup(desc->help);
+                dst->desc[num_dst_opts].def_value_str =
+                                         g_strdup(desc->def_value_str);
+                num_dst_opts++;
+                dst->desc[num_dst_opts].name = NULL;
+            }
+            desc++;
+        }
+    }
+
+    if (param) {
+        qemu_opts_free(list);
+    }
+    return dst;
+}
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (9 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 12/33] change block layer to support both QemuOpts and QEMUOptionParamter Chunyan Liu
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

To simplify later using of qemu_opts_del, accept NULL input.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 util/qemu-option.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index e15723a..71415ee 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1010,6 +1010,10 @@ void qemu_opts_del(QemuOpts *opts)
 {
     QemuOpt *opt;
 
+    if (opts == NULL) {
+        return;
+    }
+
     for (;;) {
         opt = QTAILQ_FIRST(&opts->head);
         if (opt == NULL)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 12/33] change block layer to support both QemuOpts and QEMUOptionParamter
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (10 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 13/33] vvfat.c: handle cross_driver's create_options and create_opts Chunyan Liu
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Change block layer to support both QemuOpts and QEMUOptionParameter.
After this patch, it will change backend drivers one by one. At the end,
QEMUOptionParameter will be removed and only QemuOpts is kept.

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * Fix Eric's comments:
    - remove the end '.' in two error messages.
    - update related iotests .out file.

 block.c                    | 160 +++++++++++++++++++++++++++++++--------------
 block/cow.c                |   2 +-
 block/qcow.c               |   2 +-
 block/qcow2.c              |   2 +-
 block/qed.c                |   2 +-
 block/raw_bsd.c            |   2 +-
 block/vhdx.c               |   2 +-
 block/vmdk.c               |   4 +-
 block/vvfat.c              |   2 +-
 include/block/block.h      |   7 +-
 include/block/block_int.h  |  13 +++-
 qemu-img.c                 |  94 +++++++++++++-------------
 tests/qemu-iotests/049.out |   2 +-
 tests/qemu-iotests/061.out |   2 +-
 14 files changed, 183 insertions(+), 113 deletions(-)

diff --git a/block.c b/block.c
index b749d31..d417190 100644
--- a/block.c
+++ b/block.c
@@ -328,6 +328,13 @@ void bdrv_register(BlockDriver *bdrv)
         }
     }
 
+    if (bdrv->bdrv_create) {
+        assert(!bdrv->bdrv_create2 && !bdrv->create_opts);
+        assert(!bdrv->bdrv_amend_options2);
+    } else if (bdrv->bdrv_create2) {
+        assert(!bdrv->bdrv_create && !bdrv->create_options);
+        assert(!bdrv->bdrv_amend_options);
+    }
     QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
 }
 
@@ -419,6 +426,7 @@ typedef struct CreateCo {
     BlockDriver *drv;
     char *filename;
     QEMUOptionParameter *options;
+    QemuOpts *opts;
     int ret;
     Error *err;
 } CreateCo;
@@ -430,8 +438,28 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 
     CreateCo *cco = opaque;
     assert(cco->drv);
+    assert(!(cco->options && cco->opts));
 
-    ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+    if (cco->drv->bdrv_create2) {
+        QemuOptsList *opts_list = NULL;
+        if (cco->options) {
+            opts_list = params_to_opts(cco->options);
+            cco->opts = qemu_opts_create(opts_list, NULL, 0, &error_abort);
+        }
+        ret = cco->drv->bdrv_create2(cco->filename, cco->opts, &local_err);
+        if (cco->options) {
+            qemu_opts_del(cco->opts);
+            qemu_opts_free(opts_list);
+        }
+    } else {
+        if (cco->opts) {
+            cco->options = opts_to_params(cco->opts);
+        }
+        ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+        if (cco->opts) {
+            free_option_parameters(cco->options);
+        }
+    }
     if (local_err) {
         error_propagate(&cco->err, local_err);
     }
@@ -439,7 +467,8 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options, Error **errp)
+                QEMUOptionParameter *options,
+                QemuOpts *opts, Error **errp)
 {
     int ret;
 
@@ -448,11 +477,12 @@ int bdrv_create(BlockDriver *drv, const char* filename,
         .drv = drv,
         .filename = g_strdup(filename),
         .options = options,
+        .opts = opts,
         .ret = NOT_DONE,
         .err = NULL,
     };
 
-    if (!drv->bdrv_create) {
+    if (!drv->bdrv_create && !drv->bdrv_create2) {
         error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
         ret = -ENOTSUP;
         goto out;
@@ -484,7 +514,7 @@ out:
 }
 
 int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
-                     Error **errp)
+                     QemuOpts *opts, Error **errp)
 {
     BlockDriver *drv;
     Error *local_err = NULL;
@@ -496,7 +526,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
         return -ENOENT;
     }
 
-    ret = bdrv_create(drv, filename, options, &local_err);
+    ret = bdrv_create(drv, filename, options, opts, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
     }
@@ -1212,7 +1242,8 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
     char *tmp_filename = g_malloc0(PATH_MAX + 1);
     int64_t total_size;
     BlockDriver *bdrv_qcow2;
-    QEMUOptionParameter *create_options;
+    QemuOptsList *create_opts = NULL;
+    QemuOpts *opts = NULL;
     QDict *snapshot_options;
     BlockDriverState *bs_snapshot;
     Error *local_err;
@@ -1237,13 +1268,20 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
     }
 
     bdrv_qcow2 = bdrv_find_format("qcow2");
-    create_options = parse_option_parameters("", bdrv_qcow2->create_options,
-                                             NULL);
-
-    set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
 
-    ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
-    free_option_parameters(create_options);
+    assert(!(bdrv_qcow2->create_options && bdrv_qcow2->create_opts));
+    if (bdrv_qcow2->create_options) {
+        create_opts = params_to_opts(bdrv_qcow2->create_options);
+    } else {
+        create_opts = bdrv_qcow2->create_opts;
+    }
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
+    ret = bdrv_create(bdrv_qcow2, tmp_filename, NULL, opts, &local_err);
+    qemu_opts_del(opts);
+    if (bdrv_qcow2->create_options) {
+        qemu_opts_free(create_opts);
+    }
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not create temporary overlay "
                          "'%s': %s", tmp_filename,
@@ -5346,8 +5384,10 @@ void bdrv_img_create(const char *filename, const char *fmt,
                      char *options, uint64_t img_size, int flags,
                      Error **errp, bool quiet)
 {
-    QEMUOptionParameter *param = NULL, *create_options = NULL;
-    QEMUOptionParameter *backing_fmt, *backing_file, *size;
+    QemuOptsList *create_opts = NULL;
+    QemuOpts *opts = NULL;
+    const char *backing_fmt, *backing_file;
+    int64_t size;
     BlockDriver *drv, *proto_drv;
     BlockDriver *backing_drv = NULL;
     Error *local_err = NULL;
@@ -5366,28 +5406,25 @@ void bdrv_img_create(const char *filename, const char *fmt,
         return;
     }
 
-    create_options = append_option_parameters(create_options,
-                                              drv->create_options);
-    create_options = append_option_parameters(create_options,
-                                              proto_drv->create_options);
+    create_opts = qemu_opts_append(create_opts, drv->create_opts,
+                                   drv->create_options);
+    create_opts = qemu_opts_append(create_opts, proto_drv->create_opts,
+                                   proto_drv->create_options);
 
     /* Create parameter list with default values */
-    param = parse_option_parameters("", create_options, param);
-
-    set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size);
 
     /* Parse -o options */
     if (options) {
-        param = parse_option_parameters(options, create_options, param);
-        if (param == NULL) {
-            error_setg(errp, "Invalid options for file format '%s'.", fmt);
+        if (qemu_opts_do_parse(opts, options, NULL) != 0) {
+            error_setg(errp, "Invalid options for file format '%s'", fmt);
             goto out;
         }
     }
 
     if (base_filename) {
-        if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
-                                 base_filename)) {
+        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) {
             error_setg(errp, "Backing file not supported for file format '%s'",
                        fmt);
             goto out;
@@ -5395,37 +5432,37 @@ void bdrv_img_create(const char *filename, const char *fmt,
     }
 
     if (base_fmt) {
-        if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) {
             error_setg(errp, "Backing file format not supported for file "
                              "format '%s'", fmt);
             goto out;
         }
     }
 
-    backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
-    if (backing_file && backing_file->value.s) {
-        if (!strcmp(filename, backing_file->value.s)) {
+    backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
+    if (backing_file) {
+        if (!strcmp(filename, backing_file)) {
             error_setg(errp, "Error: Trying to create an image with the "
                              "same filename as the backing file");
             goto out;
         }
     }
 
-    backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
-    if (backing_fmt && backing_fmt->value.s) {
-        backing_drv = bdrv_find_format(backing_fmt->value.s);
+    backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
+    if (backing_fmt) {
+        backing_drv = bdrv_find_format(backing_fmt);
         if (!backing_drv) {
             error_setg(errp, "Unknown backing file format '%s'",
-                       backing_fmt->value.s);
+                       backing_fmt);
             goto out;
         }
     }
 
     // The size for the image must always be specified, with one exception:
     // If we are using a backing file, we can obtain the size from there
-    size = get_option_parameter(param, BLOCK_OPT_SIZE);
-    if (size && size->value.n == -1) {
-        if (backing_file && backing_file->value.s) {
+    size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+    if (size == -1) {
+        if (backing_file) {
             BlockDriverState *bs;
             uint64_t size;
             char buf[32];
@@ -5436,11 +5473,11 @@ void bdrv_img_create(const char *filename, const char *fmt,
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
             bs = NULL;
-            ret = bdrv_open(&bs, backing_file->value.s, NULL, NULL, back_flags,
+            ret = bdrv_open(&bs, backing_file, NULL, NULL, back_flags,
                             backing_drv, &local_err);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Could not open '%s': %s",
-                                 backing_file->value.s,
+                                 backing_file,
                                  error_get_pretty(local_err));
                 error_free(local_err);
                 local_err = NULL;
@@ -5450,7 +5487,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
             size *= 512;
 
             snprintf(buf, sizeof(buf), "%" PRId64, size);
-            set_option_parameter(param, BLOCK_OPT_SIZE, buf);
+            qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size);
 
             bdrv_unref(bs);
         } else {
@@ -5461,16 +5498,18 @@ void bdrv_img_create(const char *filename, const char *fmt,
 
     if (!quiet) {
         printf("Formatting '%s', fmt=%s ", filename, fmt);
-        print_option_parameters(param);
+        qemu_opts_print(opts);
         puts("");
     }
-    ret = bdrv_create(drv, filename, param, &local_err);
+
+    ret = bdrv_create(drv, filename, NULL, opts, &local_err);
+
     if (ret == -EFBIG) {
         /* This is generally a better message than whatever the driver would
          * deliver (especially because of the cluster_size_hint), since that
          * is most probably not much different from "image too large". */
         const char *cluster_size_hint = "";
-        if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+        if (qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, 0)) {
             cluster_size_hint = " (try using a larger cluster size)";
         }
         error_setg(errp, "The image size is too large for file format '%s'"
@@ -5480,9 +5519,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
     }
 
 out:
-    free_option_parameters(create_options);
-    free_option_parameters(param);
-
+    qemu_opts_del(opts);
+    qemu_opts_free(create_opts);
     if (local_err) {
         error_propagate(errp, local_err);
     }
@@ -5500,12 +5538,36 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
     notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
 }
 
-int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options)
+int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options,
+                       QemuOpts *opts)
 {
-    if (bs->drv->bdrv_amend_options == NULL) {
+    int ret;
+    assert(!(options && opts));
+
+    if (!bs->drv->bdrv_amend_options && !bs->drv->bdrv_amend_options2) {
         return -ENOTSUP;
     }
-    return bs->drv->bdrv_amend_options(bs, options);
+    if (bs->drv->bdrv_amend_options2) {
+        QemuOptsList *opts_list = NULL;
+        if (options) {
+            opts_list = params_to_opts(options);
+            opts = qemu_opts_create(opts_list, NULL, 0, &error_abort);
+        }
+        ret = bs->drv->bdrv_amend_options2(bs, opts);
+        if (options) {
+            qemu_opts_del(opts);
+            qemu_opts_free(opts_list);
+        }
+    } else {
+        if (opts) {
+            options = opts_to_params(opts);
+        }
+        ret = bs->drv->bdrv_amend_options(bs, options);
+        if (opts) {
+            free_option_parameters(options);
+        }
+    }
+    return ret;
 }
 
 /* This function will be called by the bdrv_recurse_is_first_non_filter method
diff --git a/block/cow.c b/block/cow.c
index 164759f..7e61024 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -345,7 +345,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options, &local_err);
+    ret = bdrv_create_file(filename, options, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qcow.c b/block/qcow.c
index 937dd6d..3bac591 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -688,7 +688,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options, &local_err);
+    ret = bdrv_create_file(filename, options, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qcow2.c b/block/qcow2.c
index a4b97e8..fb267cc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1627,7 +1627,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_create_file(filename, options, &local_err);
+    ret = bdrv_create_file(filename, options, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qed.c b/block/qed.c
index c130e42..2982640 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -566,7 +566,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     int ret = 0;
     BlockDriverState *bs;
 
-    ret = bdrv_create_file(filename, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 01ea692..9ae5fc2 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -145,7 +145,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_create_file(filename, options, &local_err);
+    ret = bdrv_create_file(filename, options, NULL, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
     }
diff --git a/block/vhdx.c b/block/vhdx.c
index 509baaf..a9fcf6b 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1796,7 +1796,7 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
     block_size = block_size > VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX :
                                                     block_size;
 
-    ret = bdrv_create_file(filename, options, &local_err);
+    ret = bdrv_create_file(filename, options, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vmdk.c b/block/vmdk.c
index 06a1f9f..3802863 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1526,7 +1526,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     uint32_t *gd_buf = NULL;
     int gd_buf_size;
 
-    ret = bdrv_create_file(filename, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
@@ -1866,7 +1866,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
     if (!split && !flat) {
         desc_offset = 0x200;
     } else {
-        ret = bdrv_create_file(filename, options, &local_err);
+        ret = bdrv_create_file(filename, options, NULL, &local_err);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Could not create image file");
             goto exit;
diff --git a/block/vvfat.c b/block/vvfat.c
index c3af7ff..155fc9b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2926,7 +2926,7 @@ static int enable_write_target(BDRVVVFATState *s)
     set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
     set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
 
-    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
+    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/include/block/block.h b/include/block/block.h
index 467fb2b..faf15e0 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -178,9 +178,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
 BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
                                           bool readonly);
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options, Error **errp);
+    QEMUOptionParameter *options, QemuOpts *opts, Error **errp);
 int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
-                     Error **errp);
+                     QemuOpts *opts, Error **errp);
 BlockDriverState *bdrv_new(const char *device_name, Error **errp);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
@@ -285,7 +285,8 @@ typedef enum {
 
 int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
 
-int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options);
+int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options,
+                       QemuOpts *opts);
 
 /* external snapshots */
 bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9ffcb69..3312b02 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -118,6 +118,8 @@ struct BlockDriver {
     void (*bdrv_rebind)(BlockDriverState *bs);
     int (*bdrv_create)(const char *filename, QEMUOptionParameter *options,
                        Error **errp);
+    /* FIXME: will remove the duplicate and rename back to bdrv_create later */
+    int (*bdrv_create2)(const char *filename, QemuOpts *opts, Error **errp);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
     int (*bdrv_make_empty)(BlockDriverState *bs);
     /* aio */
@@ -217,7 +219,12 @@ struct BlockDriver {
 
     /* List of options for creating images, terminated by name == NULL */
     QEMUOptionParameter *create_options;
-
+    /* FIXME: will replace create_options.
+     * These two fields are mutually exclusive. At most one is non-NULL.
+     * create_options should only be set with bdrv_create, and create_opts
+     * should only be set with bdrv_create2.
+     */
+    QemuOptsList *create_opts;
 
     /*
      * Returns 0 for completed check, -errno for internal errors.
@@ -228,6 +235,10 @@ struct BlockDriver {
 
     int (*bdrv_amend_options)(BlockDriverState *bs,
         QEMUOptionParameter *options);
+    /* FIXME: will remove the duplicate and rename back to
+     * bdrv_amend_options later
+     */
+    int (*bdrv_amend_options2)(BlockDriverState *bs, QemuOpts *opts);
 
     void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
 
diff --git a/qemu-img.c b/qemu-img.c
index 96f4463..6e1f4f1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -252,7 +252,7 @@ static int read_password(char *buf, int buf_size)
 static int print_block_option_help(const char *filename, const char *fmt)
 {
     BlockDriver *drv, *proto_drv;
-    QEMUOptionParameter *create_options = NULL;
+    QemuOptsList *create_opts = NULL;
 
     /* Find driver and parse its options */
     drv = bdrv_find_format(fmt);
@@ -261,21 +261,20 @@ static int print_block_option_help(const char *filename, const char *fmt)
         return 1;
     }
 
-    create_options = append_option_parameters(create_options,
-                                              drv->create_options);
-
+    create_opts = qemu_opts_append(create_opts, drv->create_opts,
+                                   drv->create_options);
     if (filename) {
         proto_drv = bdrv_find_protocol(filename, true);
         if (!proto_drv) {
             error_report("Unknown protocol '%s'", filename);
             return 1;
         }
-        create_options = append_option_parameters(create_options,
-                                                  proto_drv->create_options);
+        create_opts = qemu_opts_append(create_opts, proto_drv->create_opts,
+                                       proto_drv->create_options);
     }
 
-    print_option_help(create_options);
-    free_option_parameters(create_options);
+    qemu_opts_print_help(create_opts);
+    qemu_opts_free(create_opts);
     return 0;
 }
 
@@ -329,19 +328,19 @@ fail:
     return NULL;
 }
 
-static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
+static int add_old_style_options(const char *fmt, QemuOpts *opts,
                                  const char *base_filename,
                                  const char *base_fmt)
 {
     if (base_filename) {
-        if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, base_filename)) {
+        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) {
             error_report("Backing file not supported for file format '%s'",
                          fmt);
             return -1;
         }
     }
     if (base_fmt) {
-        if (set_option_parameter(list, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+        if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) {
             error_report("Backing file format not supported for file "
                          "format '%s'", fmt);
             return -1;
@@ -1172,8 +1171,9 @@ static int img_convert(int argc, char **argv)
     size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
     const uint8_t *buf1;
     BlockDriverInfo bdi;
-    QEMUOptionParameter *param = NULL, *create_options = NULL;
-    QEMUOptionParameter *out_baseimg_param;
+    QemuOpts *opts = NULL;
+    QemuOptsList *create_opts = NULL;
+    const char *out_baseimg_param;
     char *options = NULL;
     const char *snapshot_name = NULL;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
@@ -1362,40 +1362,36 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    create_options = append_option_parameters(create_options,
-                                              drv->create_options);
-    create_options = append_option_parameters(create_options,
-                                              proto_drv->create_options);
+    create_opts = qemu_opts_append(create_opts, drv->create_opts,
+                                   drv->create_options);
+    create_opts = qemu_opts_append(create_opts, proto_drv->create_opts,
+                                   proto_drv->create_options);
 
-    if (options) {
-        param = parse_option_parameters(options, create_options, param);
-        if (param == NULL) {
-            error_report("Invalid options for file format '%s'.", out_fmt);
-            ret = -1;
-            goto out;
-        }
-    } else {
-        param = parse_option_parameters("", create_options, param);
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    if (options && qemu_opts_do_parse(opts, options, NULL)) {
+        error_report("Invalid options for file format '%s'", out_fmt);
+        ret = -1;
+        goto out;
     }
 
-    set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
-    ret = add_old_style_options(out_fmt, param, out_baseimg, NULL);
+    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512);
+    ret = add_old_style_options(out_fmt, opts, out_baseimg, NULL);
     if (ret < 0) {
         goto out;
     }
 
     /* Get backing file name if -o backing_file was used */
-    out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
+    out_baseimg_param = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
     if (out_baseimg_param) {
-        out_baseimg = out_baseimg_param->value.s;
+        out_baseimg = out_baseimg_param;
     }
 
     /* Check if compression is supported */
     if (compress) {
-        QEMUOptionParameter *encryption =
-            get_option_parameter(param, BLOCK_OPT_ENCRYPT);
-        QEMUOptionParameter *preallocation =
-            get_option_parameter(param, BLOCK_OPT_PREALLOC);
+        bool encryption =
+            qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, false);
+        const char *preallocation =
+            qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
 
         if (!drv->bdrv_write_compressed) {
             error_report("Compression not supported for this file format");
@@ -1403,15 +1399,15 @@ static int img_convert(int argc, char **argv)
             goto out;
         }
 
-        if (encryption && encryption->value.n) {
+        if (encryption) {
             error_report("Compression and encryption not supported at "
                          "the same time");
             ret = -1;
             goto out;
         }
 
-        if (preallocation && preallocation->value.s
-            && strcmp(preallocation->value.s, "off"))
+        if (preallocation
+            && strcmp(preallocation, "off"))
         {
             error_report("Compression and preallocation not supported at "
                          "the same time");
@@ -1422,7 +1418,7 @@ static int img_convert(int argc, char **argv)
 
     if (!skip_create) {
         /* Create the new image */
-        ret = bdrv_create(drv, out_filename, param, &local_err);
+        ret = bdrv_create(drv, out_filename, NULL, opts, &local_err);
         if (ret < 0) {
             error_report("%s: error while converting %s: %s",
                          out_filename, out_fmt, error_get_pretty(local_err));
@@ -1686,8 +1682,8 @@ out:
         qemu_progress_print(100, 0);
     }
     qemu_progress_end();
-    free_option_parameters(create_options);
-    free_option_parameters(param);
+    qemu_opts_del(opts);
+    qemu_opts_free(create_opts);
     qemu_vfree(buf);
     if (sn_opts) {
         qemu_opts_del(sn_opts);
@@ -2678,7 +2674,8 @@ static int img_amend(int argc, char **argv)
 {
     int c, ret = 0;
     char *options = NULL;
-    QEMUOptionParameter *create_options = NULL, *options_param = NULL;
+    QemuOptsList *create_opts = NULL;
+    QemuOpts *opts = NULL;
     const char *fmt = NULL, *filename;
     bool quiet = false;
     BlockDriverState *bs = NULL;
@@ -2749,17 +2746,16 @@ static int img_amend(int argc, char **argv)
         goto out;
     }
 
-    create_options = append_option_parameters(create_options,
-            bs->drv->create_options);
-    options_param = parse_option_parameters(options, create_options,
-            options_param);
-    if (options_param == NULL) {
+    create_opts = qemu_opts_append(create_opts, bs->drv->create_opts,
+                                   bs->drv->create_options);
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    if (options && qemu_opts_do_parse(opts, options, NULL)) {
         error_report("Invalid options for file format '%s'", fmt);
         ret = -1;
         goto out;
     }
 
-    ret = bdrv_amend_options(bs, options_param);
+    ret = bdrv_amend_options(bs, NULL, opts);
     if (ret < 0) {
         error_report("Error while amending options: %s", strerror(-ret));
         goto out;
@@ -2769,8 +2765,8 @@ out:
     if (bs) {
         bdrv_unref(bs);
     }
-    free_option_parameters(create_options);
-    free_option_parameters(options_param);
+    qemu_opts_del(opts);
+    qemu_opts_free(create_opts);
     g_free(options);
 
     if (ret) {
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index ceb2328..71ca44d 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -120,7 +120,7 @@ qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
 
 qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
 qemu-img: Parameter 'size' expects a size
-qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'.
+qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
 
 == Check correct interpretation of suffixes for cluster size ==
 
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 4027e00..e372470 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -281,7 +281,7 @@ Lazy refcounts only supported with compatibility level 1.1 and above (use compat
 qemu-img: Error while amending options: Invalid argument
 Unknown compatibility level 0.42.
 qemu-img: Error while amending options: Invalid argument
-Unknown option 'foo'
+qemu-img: Invalid parameter 'foo'
 qemu-img: Invalid options for file format 'qcow2'
 Changing the cluster size is not supported.
 qemu-img: Error while amending options: Operation not supported
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 13/33] vvfat.c: handle cross_driver's create_options and create_opts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (11 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 12/33] change block layer to support both QemuOpts and QEMUOptionParamter Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 14/33] cow.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

vvfat shares create options of qcow driver. To avoid vvfat breaking when
qcow driver changes from QEMUOptionParameter to QemuOpts, let it able
to handle both cases.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Change:
  * Fix memory free:
    - qemu_opts_free(create_opts);
    + if (bdrv_qcow->create_options) {
    +     qemu_opts_free(create_opts);
    + }

 block/vvfat.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 155fc9b..41fe623 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2906,8 +2906,9 @@ static BlockDriver vvfat_write_target = {
 
 static int enable_write_target(BDRVVVFATState *s)
 {
-    BlockDriver *bdrv_qcow;
-    QEMUOptionParameter *options;
+    BlockDriver *bdrv_qcow = NULL;
+    QemuOptsList *create_opts = NULL;
+    QemuOpts *opts = NULL;
     Error *local_err = NULL;
     int ret;
     int size = sector2cluster(s, s->sector_count);
@@ -2922,11 +2923,17 @@ static int enable_write_target(BDRVVVFATState *s)
     }
 
     bdrv_qcow = bdrv_find_format("qcow");
-    options = parse_option_parameters("", bdrv_qcow->create_options, NULL);
-    set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
-    set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
+    assert(!(bdrv_qcow->create_opts && bdrv_qcow->create_options));
+    if (bdrv_qcow->create_options) {
+        create_opts = params_to_opts(bdrv_qcow->create_options);
+    } else {
+        create_opts = bdrv_qcow->create_opts;
+    }
+    opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512);
+    qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:");
 
-    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, NULL, &local_err);
+    ret = bdrv_create(bdrv_qcow, s->qcow_filename, NULL, opts, &local_err);
     if (ret < 0) {
         qerror_report_err(local_err);
         error_free(local_err);
@@ -2955,6 +2962,10 @@ static int enable_write_target(BDRVVVFATState *s)
     return 0;
 
 err:
+    qemu_opts_del(opts);
+    if (bdrv_qcow->create_options) {
+        qemu_opts_free(create_opts);
+    }
     g_free(s->qcow_filename);
     s->qcow_filename = NULL;
     return ret;
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 14/33] cow.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (12 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 13/33] vvfat.c: handle cross_driver's create_options and create_opts Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 15/33] gluster.c: " Chunyan Liu
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/cow.c | 54 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 7e61024..af85753 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -324,31 +324,24 @@ static void cow_close(BlockDriverState *bs)
 {
 }
 
-static int cow_create(const char *filename, QEMUOptionParameter *options,
-                      Error **errp)
+static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     struct cow_header_v2 cow_header;
     struct stat st;
     int64_t image_sectors = 0;
-    const char *image_filename = NULL;
+    char *image_filename = NULL;
     Error *local_err = NULL;
     int ret;
     BlockDriverState *cow_bs;
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            image_sectors = options->value.n / 512;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-            image_filename = options->value.s;
-        }
-        options++;
-    }
+    image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
 
-    ret = bdrv_create_file(filename, options, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, opts, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto exit;
     }
 
     cow_bs = NULL;
@@ -356,7 +349,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
                     BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto exit;
     }
 
     memset(&cow_header, 0, sizeof(cow_header));
@@ -389,22 +382,27 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
     }
 
 exit:
+    g_free(image_filename);
     bdrv_unref(cow_bs);
     return ret;
 }
 
-static QEMUOptionParameter cow_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    {
-        .name = BLOCK_OPT_BACKING_FILE,
-        .type = OPT_STRING,
-        .help = "File name of a base image"
-    },
-    { NULL }
+static QemuOptsList cow_create_opts = {
+    .name = "cow-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FILE,
+            .type = QEMU_OPT_STRING,
+            .help = "File name of a base image"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_cow = {
@@ -414,14 +412,14 @@ static BlockDriver bdrv_cow = {
     .bdrv_probe     = cow_probe,
     .bdrv_open      = cow_open,
     .bdrv_close     = cow_close,
-    .bdrv_create    = cow_create,
+    .bdrv_create2   = cow_create,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
 
     .bdrv_read              = cow_co_read,
     .bdrv_write             = cow_co_write,
     .bdrv_co_get_block_status   = cow_co_get_block_status,
 
-    .create_options = cow_create_options,
+    .create_opts    = &cow_create_opts,
 };
 
 static void bdrv_cow_init(void)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 15/33] gluster.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (13 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 14/33] cow.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 16/33] iscsi.c: " Chunyan Liu
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/gluster.c | 81 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 8836085..c72ca77 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -471,13 +471,14 @@ static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset,
 #endif
 
 static int qemu_gluster_create(const char *filename,
-        QEMUOptionParameter *options, Error **errp)
+                               QemuOpts *opts, Error **errp)
 {
     struct glfs *glfs;
     struct glfs_fd *fd;
     int ret = 0;
     int prealloc = 0;
     int64_t total_size = 0;
+    char *tmp = NULL;
     GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
 
     glfs = qemu_gluster_init(gconf, filename, errp);
@@ -486,24 +487,21 @@ static int qemu_gluster_create(const char *filename,
         goto out;
     }
 
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / BDRV_SECTOR_SIZE;
-        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
-            if (!options->value.s || !strcmp(options->value.s, "off")) {
-                prealloc = 0;
-            } else if (!strcmp(options->value.s, "full") &&
-                    gluster_supports_zerofill()) {
-                prealloc = 1;
-            } else {
-                error_setg(errp, "Invalid preallocation mode: '%s'"
-                    " or GlusterFS doesn't support zerofill API",
-                           options->value.s);
-                ret = -EINVAL;
-                goto out;
-            }
-        }
-        options++;
+    total_size =
+        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+
+    tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+    if (!tmp || !strcmp(tmp, "off")) {
+        prealloc = 0;
+    } else if (!strcmp(tmp, "full") &&
+               gluster_supports_zerofill()) {
+        prealloc = 1;
+    } else {
+        error_setg(errp, "Invalid preallocation mode: '%s'"
+            " or GlusterFS doesn't support zerofill API",
+            tmp);
+        ret = -EINVAL;
+        goto out;
     }
 
     fd = glfs_creat(glfs, gconf->image,
@@ -525,6 +523,7 @@ static int qemu_gluster_create(const char *filename,
         }
     }
 out:
+    g_free(tmp);
     qemu_gluster_gconf_free(gconf);
     if (glfs) {
         glfs_fini(glfs);
@@ -688,18 +687,22 @@ static int qemu_gluster_has_zero_init(BlockDriverState *bs)
     return 0;
 }
 
-static QEMUOptionParameter qemu_gluster_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    {
-        .name = BLOCK_OPT_PREALLOC,
-        .type = OPT_STRING,
-        .help = "Preallocation mode (allowed values: off, full)"
-    },
-    { NULL }
+static QemuOptsList qemu_gluster_create_opts = {
+    .name = "qemu-gluster-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_PREALLOC,
+            .type = QEMU_OPT_STRING,
+            .help = "Preallocation mode (allowed values: off, full)"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_gluster = {
@@ -712,7 +715,7 @@ static BlockDriver bdrv_gluster = {
     .bdrv_reopen_commit           = qemu_gluster_reopen_commit,
     .bdrv_reopen_abort            = qemu_gluster_reopen_abort,
     .bdrv_close                   = qemu_gluster_close,
-    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_create2                 = qemu_gluster_create,
     .bdrv_getlength               = qemu_gluster_getlength,
     .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_truncate                = qemu_gluster_truncate,
@@ -726,7 +729,7 @@ static BlockDriver bdrv_gluster = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
-    .create_options               = qemu_gluster_create_options,
+    .create_opts                  = &qemu_gluster_create_opts,
 };
 
 static BlockDriver bdrv_gluster_tcp = {
@@ -739,7 +742,7 @@ static BlockDriver bdrv_gluster_tcp = {
     .bdrv_reopen_commit           = qemu_gluster_reopen_commit,
     .bdrv_reopen_abort            = qemu_gluster_reopen_abort,
     .bdrv_close                   = qemu_gluster_close,
-    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_create2                 = qemu_gluster_create,
     .bdrv_getlength               = qemu_gluster_getlength,
     .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_truncate                = qemu_gluster_truncate,
@@ -753,7 +756,7 @@ static BlockDriver bdrv_gluster_tcp = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
-    .create_options               = qemu_gluster_create_options,
+    .create_opts                  = &qemu_gluster_create_opts,
 };
 
 static BlockDriver bdrv_gluster_unix = {
@@ -766,7 +769,7 @@ static BlockDriver bdrv_gluster_unix = {
     .bdrv_reopen_commit           = qemu_gluster_reopen_commit,
     .bdrv_reopen_abort            = qemu_gluster_reopen_abort,
     .bdrv_close                   = qemu_gluster_close,
-    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_create2                 = qemu_gluster_create,
     .bdrv_getlength               = qemu_gluster_getlength,
     .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_truncate                = qemu_gluster_truncate,
@@ -780,7 +783,7 @@ static BlockDriver bdrv_gluster_unix = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
-    .create_options               = qemu_gluster_create_options,
+    .create_opts                  = &qemu_gluster_create_opts,
 };
 
 static BlockDriver bdrv_gluster_rdma = {
@@ -793,7 +796,7 @@ static BlockDriver bdrv_gluster_rdma = {
     .bdrv_reopen_commit           = qemu_gluster_reopen_commit,
     .bdrv_reopen_abort            = qemu_gluster_reopen_abort,
     .bdrv_close                   = qemu_gluster_close,
-    .bdrv_create                  = qemu_gluster_create,
+    .bdrv_create2                 = qemu_gluster_create,
     .bdrv_getlength               = qemu_gluster_getlength,
     .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
     .bdrv_truncate                = qemu_gluster_truncate,
@@ -807,7 +810,7 @@ static BlockDriver bdrv_gluster_rdma = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
-    .create_options               = qemu_gluster_create_options,
+    .create_opts                  = &qemu_gluster_create_opts,
 };
 
 static void bdrv_gluster_init(void)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 16/33] iscsi.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (14 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 15/33] gluster.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 17/33] nfs.c: " Chunyan Liu
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/iscsi.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index a30202b..e4a127b 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1391,8 +1391,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
     return 0;
 }
 
-static int iscsi_create(const char *filename, QEMUOptionParameter *options,
-                        Error **errp)
+static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     int64_t total_size = 0;
@@ -1403,13 +1402,8 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options,
     bs = bdrv_new("", &error_abort);
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, "size")) {
-            total_size = options->value.n / BDRV_SECTOR_SIZE;
-        }
-        options++;
-    }
-
+    total_size =
+        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
     bs->opaque = g_malloc0(sizeof(struct IscsiLun));
     iscsilun = bs->opaque;
 
@@ -1460,13 +1454,17 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
-static QEMUOptionParameter iscsi_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    { NULL }
+static QemuOptsList iscsi_create_opts = {
+    .name = "iscsi-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(iscsi_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_iscsi = {
@@ -1477,8 +1475,8 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_needs_filename = true,
     .bdrv_file_open  = iscsi_open,
     .bdrv_close      = iscsi_close,
-    .bdrv_create     = iscsi_create,
-    .create_options  = iscsi_create_options,
+    .bdrv_create2    = iscsi_create,
+    .create_opts     = &iscsi_create_opts,
     .bdrv_reopen_prepare  = iscsi_reopen_prepare,
 
     .bdrv_getlength  = iscsi_getlength,
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 17/33] nfs.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (15 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 16/33] iscsi.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 18/33] qcow.c: " Chunyan Liu
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/nfs.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index 9fa831f..8633c04 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -357,20 +357,14 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     return 0;
 }
 
-static int nfs_file_create(const char *url, QEMUOptionParameter *options,
-                           Error **errp)
+static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     int64_t total_size = 0;
     NFSClient *client = g_malloc0(sizeof(NFSClient));
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, "size")) {
-            total_size = options->value.n;
-        }
-        options++;
-    }
+    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 
     ret = nfs_client_open(client, url, O_CREAT, errp);
     if (ret < 0) {
@@ -427,7 +421,7 @@ static BlockDriver bdrv_nfs = {
 
     .bdrv_file_open  = nfs_file_open,
     .bdrv_close      = nfs_file_close,
-    .bdrv_create     = nfs_file_create,
+    .bdrv_create2    = nfs_file_create,
 
     .bdrv_co_readv         = nfs_co_readv,
     .bdrv_co_writev        = nfs_co_writev,
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 18/33] qcow.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (16 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 17/33] nfs.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find Chunyan Liu
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/qcow.c | 72 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 3bac591..e8fe874 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -663,35 +663,29 @@ static void qcow_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
-static int qcow_create(const char *filename, QEMUOptionParameter *options,
-                       Error **errp)
+static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int header_size, backing_filename_len, l1_size, shift, i;
     QCowHeader header;
     uint8_t *tmp;
     int64_t total_size = 0;
-    const char *backing_file = NULL;
+    char *backing_file = NULL;
     int flags = 0;
     Error *local_err = NULL;
     int ret;
     BlockDriverState *qcow_bs;
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / 512;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-            backing_file = options->value.s;
-        } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
-            flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
-        }
-        options++;
+    total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
+        flags |= BLOCK_FLAG_ENCRYPT;
     }
 
-    ret = bdrv_create_file(filename, options, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, opts, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto cleanup;
     }
 
     qcow_bs = NULL;
@@ -699,7 +693,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
                     BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
-        return ret;
+        goto cleanup;
     }
 
     ret = bdrv_truncate(qcow_bs, 0);
@@ -770,6 +764,8 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
     ret = 0;
 exit:
     bdrv_unref(qcow_bs);
+cleanup:
+    g_free(backing_file);
     return ret;
 }
 
@@ -882,24 +878,28 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
-
-static QEMUOptionParameter qcow_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    {
-        .name = BLOCK_OPT_BACKING_FILE,
-        .type = OPT_STRING,
-        .help = "File name of a base image"
-    },
-    {
-        .name = BLOCK_OPT_ENCRYPT,
-        .type = OPT_FLAG,
-        .help = "Encrypt the image"
-    },
-    { NULL }
+static QemuOptsList qcow_create_opts = {
+    .name = "qcow-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FILE,
+            .type = QEMU_OPT_STRING,
+            .help = "File name of a base image"
+        },
+        {
+            .name = BLOCK_OPT_ENCRYPT,
+            .type = QEMU_OPT_BOOL,
+            .help = "Encrypt the image",
+            .def_value_str = "off"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_qcow = {
@@ -908,8 +908,8 @@ static BlockDriver bdrv_qcow = {
     .bdrv_probe		= qcow_probe,
     .bdrv_open		= qcow_open,
     .bdrv_close		= qcow_close,
-    .bdrv_reopen_prepare = qcow_reopen_prepare,
-    .bdrv_create	= qcow_create,
+    .bdrv_reopen_prepare    = qcow_reopen_prepare,
+    .bdrv_create2           = qcow_create,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
 
     .bdrv_co_readv          = qcow_co_readv,
@@ -921,7 +921,7 @@ static BlockDriver bdrv_qcow = {
     .bdrv_write_compressed  = qcow_write_compressed,
     .bdrv_get_info          = qcow_get_info,
 
-    .create_options = qcow_create_options,
+    .create_opts            = &qcow_create_opts,
 };
 
 static void bdrv_qcow_init(void)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (17 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 18/33] qcow.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-06-05 22:12   ` Eric Blake
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (6 subsequent siblings)
  25 siblings, 1 reply; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Export qemu_opt_find for qcow2 driver using it.
After replacing QEMUOptionParameter with QemuOpts, qcow2 driver will
use qemu_opt_find to judge if an option is explicitly set, to replace
the usage of .assigned in QEMUOptionParameter.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * split qcow2 patch into two: 19/33, 20/33
    - export qemu_opt_find in a separate commit
    - then patch qcow2 driver

 include/qemu/option.h | 1 +
 util/qemu-option.c    | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 44d9961..3455267 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -130,6 +130,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name);
  * Returns: true if @opts includes 'help' or equivalent.
  */
 bool qemu_opt_has_help_opt(QemuOpts *opts);
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name);
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 71415ee..55f1726 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -568,7 +568,7 @@ void qemu_opts_print_help(QemuOptsList *list)
 }
 /* ------------------------------------------------------------------ */
 
-static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
 
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (18 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-06-05 22:27   ` Eric Blake
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 21/33] qed.c: " Chunyan Liu
                   ` (5 subsequent siblings)
  25 siblings, 1 reply; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
Changes:
  * remove exporting qemu_opt_find from qcow2 patch and commit separately.
  * remove unnecessary indention changes in bdrv_qcow2 as Eric points out.

 block/qcow2.c | 261 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 139 insertions(+), 122 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fb267cc..2261a1b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -31,6 +31,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
 #include "trace.h"
+#include "qemu/option_int.h"
 
 /*
   Differences with QCOW:
@@ -1595,7 +1596,7 @@ static int preallocate(BlockDriverState *bs)
 static int qcow2_create2(const char *filename, int64_t total_size,
                          const char *backing_file, const char *backing_format,
                          int flags, size_t cluster_size, int prealloc,
-                         QEMUOptionParameter *options, int version,
+                         QemuOpts *opts, int version,
                          Error **errp)
 {
     /* Calculate cluster_bits */
@@ -1627,7 +1628,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_create_file(filename, options, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, opts, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
@@ -1763,11 +1764,11 @@ out:
     return ret;
 }
 
-static int qcow2_create(const char *filename, QEMUOptionParameter *options,
-                        Error **errp)
+static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
 {
-    const char *backing_file = NULL;
-    const char *backing_fmt = NULL;
+    char *backing_file = NULL;
+    char *backing_fmt = NULL;
+    char *buf = NULL;
     uint64_t sectors = 0;
     int flags = 0;
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
@@ -1777,64 +1778,66 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     int ret;
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            sectors = options->value.n / 512;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-            backing_file = options->value.s;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
-            backing_fmt = options->value.s;
-        } else if (!strcmp(options->name, BLOCK_OPT_ENCRYPT)) {
-            flags |= options->value.n ? BLOCK_FLAG_ENCRYPT : 0;
-        } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) {
-            if (options->value.n) {
-                cluster_size = options->value.n;
-            }
-        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
-            if (!options->value.s || !strcmp(options->value.s, "off")) {
-                prealloc = 0;
-            } else if (!strcmp(options->value.s, "metadata")) {
-                prealloc = 1;
-            } else {
-                error_setg(errp, "Invalid preallocation mode: '%s'",
-                           options->value.s);
-                return -EINVAL;
-            }
-        } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
-            if (!options->value.s) {
-                /* keep the default */
-            } else if (!strcmp(options->value.s, "0.10")) {
-                version = 2;
-            } else if (!strcmp(options->value.s, "1.1")) {
-                version = 3;
-            } else {
-                error_setg(errp, "Invalid compatibility level: '%s'",
-                           options->value.s);
-                return -EINVAL;
-            }
-        } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
-            flags |= options->value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0;
-        }
-        options++;
+    sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+    backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
+        flags |= BLOCK_FLAG_ENCRYPT;
+    }
+    cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
+                                         DEFAULT_CLUSTER_SIZE);
+    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+    if (!buf || !strcmp(buf, "off")) {
+        prealloc = 0;
+    } else if (!strcmp(buf, "metadata")) {
+        prealloc = 1;
+    } else {
+        error_setg(errp, "Invalid preallocation mode: '%s'", buf);
+        ret = -EINVAL;
+        goto finish;
+    }
+    g_free(buf);
+    buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
+    if (!buf) {
+        /* keep the default */
+    } else if (!strcmp(buf, "0.10")) {
+        version = 2;
+    } else if (!strcmp(buf, "1.1")) {
+        version = 3;
+    } else {
+        error_setg(errp, "Invalid compatibility level: '%s'", buf);
+        ret = -EINVAL;
+        goto finish;
+    }
+
+    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_LAZY_REFCOUNTS, false)) {
+        flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
     }
 
     if (backing_file && prealloc) {
         error_setg(errp, "Backing file and preallocation cannot be used at "
                    "the same time");
-        return -EINVAL;
+        ret = -EINVAL;
+        goto finish;
     }
 
     if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) {
         error_setg(errp, "Lazy refcounts only supported with compatibility "
                    "level 1.1 and above (use compat=1.1 or greater)");
-        return -EINVAL;
+        ret = -EINVAL;
+        goto finish;
     }
 
     ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
-                        cluster_size, prealloc, options, version, &local_err);
+                        cluster_size, prealloc, opts, version, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
     }
+
+finish:
+    g_free(backing_file);
+    g_free(backing_fmt);
+    g_free(buf);
     return ret;
 }
 
@@ -2199,64 +2202,72 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version)
     return 0;
 }
 
-static int qcow2_amend_options(BlockDriverState *bs,
-                               QEMUOptionParameter *options)
+static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts)
 {
     BDRVQcowState *s = bs->opaque;
     int old_version = s->qcow_version, new_version = old_version;
     uint64_t new_size = 0;
     const char *backing_file = NULL, *backing_format = NULL;
     bool lazy_refcounts = s->use_lazy_refcounts;
+    const char *compat = NULL;
+    uint64_t cluster_size = s->cluster_size;
+    bool encrypt;
     int ret;
-    int i;
+    QemuOptDesc *desc = opts->list->desc;
 
-    for (i = 0; options[i].name; i++)
-    {
-        if (!options[i].assigned) {
+    while (desc && desc->name) {
+        if (!qemu_opt_find(opts, desc->name)) {
             /* only change explicitly defined options */
+            desc++;
             continue;
         }
 
-        if (!strcmp(options[i].name, "compat")) {
-            if (!options[i].value.s) {
+        if (!strcmp(desc->name, "compat")) {
+            compat = qemu_opt_get(opts, "compat");
+            if (!compat) {
                 /* preserve default */
-            } else if (!strcmp(options[i].value.s, "0.10")) {
+            } else if (!strcmp(compat, "0.10")) {
                 new_version = 2;
-            } else if (!strcmp(options[i].value.s, "1.1")) {
+            } else if (!strcmp(compat, "1.1")) {
                 new_version = 3;
             } else {
-                fprintf(stderr, "Unknown compatibility level %s.\n",
-                        options[i].value.s);
+                fprintf(stderr, "Unknown compatibility level %s.\n", compat);
                 return -EINVAL;
             }
-        } else if (!strcmp(options[i].name, "preallocation")) {
+        } else if (!strcmp(desc->name, "preallocation")) {
             fprintf(stderr, "Cannot change preallocation mode.\n");
             return -ENOTSUP;
-        } else if (!strcmp(options[i].name, "size")) {
-            new_size = options[i].value.n;
-        } else if (!strcmp(options[i].name, "backing_file")) {
-            backing_file = options[i].value.s;
-        } else if (!strcmp(options[i].name, "backing_fmt")) {
-            backing_format = options[i].value.s;
-        } else if (!strcmp(options[i].name, "encryption")) {
-            if ((options[i].value.n != !!s->crypt_method)) {
+        } else if (!strcmp(desc->name, "size")) {
+            new_size = qemu_opt_get_size(opts, "size", 0);
+        } else if (!strcmp(desc->name, "backing_file")) {
+            backing_file = qemu_opt_get(opts, "backing_file");
+        } else if (!strcmp(desc->name, "backing_fmt")) {
+            backing_format = qemu_opt_get(opts, "backing_fmt");
+        } else if (!strcmp(desc->name, "encryption")) {
+            encrypt = qemu_opt_get_bool(opts, "encryption", s->crypt_method);
+            if (encrypt != !!s->crypt_method) {
                 fprintf(stderr, "Changing the encryption flag is not "
                         "supported.\n");
                 return -ENOTSUP;
             }
-        } else if (!strcmp(options[i].name, "cluster_size")) {
-            if (options[i].value.n != s->cluster_size) {
+        } else if (!strcmp(desc->name, "cluster_size")) {
+            cluster_size = qemu_opt_get_size(opts, "cluster_size",
+                                             cluster_size);
+            if (cluster_size != s->cluster_size) {
                 fprintf(stderr, "Changing the cluster size is not "
                         "supported.\n");
                 return -ENOTSUP;
             }
-        } else if (!strcmp(options[i].name, "lazy_refcounts")) {
-            lazy_refcounts = options[i].value.n;
+        } else if (!strcmp(desc->name, "lazy_refcounts")) {
+            lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
+                                               lazy_refcounts);
         } else {
             /* if this assertion fails, this probably means a new option was
              * added without having it covered here */
             assert(false);
         }
+
+        desc++;
     }
 
     if (new_version != old_version) {
@@ -2325,49 +2336,55 @@ static int qcow2_amend_options(BlockDriverState *bs,
     return 0;
 }
 
-static QEMUOptionParameter qcow2_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    {
-        .name = BLOCK_OPT_COMPAT_LEVEL,
-        .type = OPT_STRING,
-        .help = "Compatibility level (0.10 or 1.1)"
-    },
-    {
-        .name = BLOCK_OPT_BACKING_FILE,
-        .type = OPT_STRING,
-        .help = "File name of a base image"
-    },
-    {
-        .name = BLOCK_OPT_BACKING_FMT,
-        .type = OPT_STRING,
-        .help = "Image format of the base image"
-    },
-    {
-        .name = BLOCK_OPT_ENCRYPT,
-        .type = OPT_FLAG,
-        .help = "Encrypt the image"
-    },
-    {
-        .name = BLOCK_OPT_CLUSTER_SIZE,
-        .type = OPT_SIZE,
-        .help = "qcow2 cluster size",
-        .value = { .n = DEFAULT_CLUSTER_SIZE },
-    },
-    {
-        .name = BLOCK_OPT_PREALLOC,
-        .type = OPT_STRING,
-        .help = "Preallocation mode (allowed values: off, metadata)"
-    },
-    {
-        .name = BLOCK_OPT_LAZY_REFCOUNTS,
-        .type = OPT_FLAG,
-        .help = "Postpone refcount updates",
-    },
-    { NULL }
+static QemuOptsList qcow2_create_opts = {
+    .name = "qcow2-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_COMPAT_LEVEL,
+            .type = QEMU_OPT_STRING,
+            .help = "Compatibility level (0.10 or 1.1)"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FILE,
+            .type = QEMU_OPT_STRING,
+            .help = "File name of a base image"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FMT,
+            .type = QEMU_OPT_STRING,
+            .help = "Image format of the base image"
+        },
+        {
+            .name = BLOCK_OPT_ENCRYPT,
+            .type = QEMU_OPT_BOOL,
+            .help = "Encrypt the image",
+            .def_value_str = "off"
+        },
+        {
+            .name = BLOCK_OPT_CLUSTER_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "qcow2 cluster size",
+            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
+        },
+        {
+            .name = BLOCK_OPT_PREALLOC,
+            .type = QEMU_OPT_STRING,
+            .help = "Preallocation mode (allowed values: off, metadata)"
+        },
+        {
+            .name = BLOCK_OPT_LAZY_REFCOUNTS,
+            .type = QEMU_OPT_BOOL,
+            .help = "Postpone refcount updates",
+            .def_value_str = "off"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_qcow2 = {
@@ -2377,7 +2394,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_open          = qcow2_open,
     .bdrv_close         = qcow2_close,
     .bdrv_reopen_prepare  = qcow2_reopen_prepare,
-    .bdrv_create        = qcow2_create,
+    .bdrv_create2       = qcow2_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,
     .bdrv_set_key       = qcow2_set_key,
@@ -2395,8 +2412,8 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_snapshot_goto     = qcow2_snapshot_goto,
     .bdrv_snapshot_delete   = qcow2_snapshot_delete,
     .bdrv_snapshot_list     = qcow2_snapshot_list,
-    .bdrv_snapshot_load_tmp     = qcow2_snapshot_load_tmp,
-    .bdrv_get_info      = qcow2_get_info,
+    .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
+    .bdrv_get_info          = qcow2_get_info,
     .bdrv_get_specific_info = qcow2_get_specific_info,
 
     .bdrv_save_vmstate    = qcow2_save_vmstate,
@@ -2407,9 +2424,9 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_refresh_limits        = qcow2_refresh_limits,
     .bdrv_invalidate_cache      = qcow2_invalidate_cache,
 
-    .create_options = qcow2_create_options,
-    .bdrv_check = qcow2_check,
-    .bdrv_amend_options = qcow2_amend_options,
+    .create_opts         = &qcow2_create_opts,
+    .bdrv_check          = qcow2_check,
+    .bdrv_amend_options2 = qcow2_amend_options,
 };
 
 static void bdrv_qcow2_init(void)
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 21/33] qed.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (19 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 22/33] raw-posix.c: " Chunyan Liu
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

One extra change is to define QED_DEFAULT_CLUSTER_SIZE = 65536 instead
of 64 * 1024; because:
according to existing create_options, "cluster size" has default value =
QED_DEFAULT_CLUSTER_SIZE, after switching to create_opts, this has to be
stringized and set to .def_value_str. That is,
  .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE),
so the QED_DEFAULT_CLUSTER_SIZE could not be a expression.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/qed.c | 114 ++++++++++++++++++++++++++++++++----------------------------
 block/qed.h |   3 +-
 2 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 2982640..9376996 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -621,55 +621,53 @@ out:
     return ret;
 }
 
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options,
-                           Error **errp)
+static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     uint64_t image_size = 0;
     uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
     uint32_t table_size = QED_DEFAULT_TABLE_SIZE;
-    const char *backing_file = NULL;
-    const char *backing_fmt = NULL;
-
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            image_size = options->value.n;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
-            backing_file = options->value.s;
-        } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FMT)) {
-            backing_fmt = options->value.s;
-        } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) {
-            if (options->value.n) {
-                cluster_size = options->value.n;
-            }
-        } else if (!strcmp(options->name, BLOCK_OPT_TABLE_SIZE)) {
-            if (options->value.n) {
-                table_size = options->value.n;
-            }
-        }
-        options++;
-    }
+    char *backing_file = NULL;
+    char *backing_fmt = NULL;
+    int ret;
+
+    image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+    backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+    cluster_size = qemu_opt_get_size_del(opts,
+                                         BLOCK_OPT_CLUSTER_SIZE,
+                                         QED_DEFAULT_CLUSTER_SIZE);
+    table_size = qemu_opt_get_size_del(opts, BLOCK_OPT_TABLE_SIZE,
+                                       QED_DEFAULT_TABLE_SIZE);
 
     if (!qed_is_cluster_size_valid(cluster_size)) {
         error_setg(errp, "QED cluster size must be within range [%u, %u] "
                          "and power of 2",
                    QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
-        return -EINVAL;
+        ret = -EINVAL;
+        goto finish;
     }
     if (!qed_is_table_size_valid(table_size)) {
         error_setg(errp, "QED table size must be within range [%u, %u] "
                          "and power of 2",
                    QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
-        return -EINVAL;
+        ret = -EINVAL;
+        goto finish;
     }
     if (!qed_is_image_size_valid(image_size, cluster_size, table_size)) {
         error_setg(errp, "QED image size must be a non-zero multiple of "
                          "cluster size and less than %" PRIu64 " bytes",
                    qed_max_image_size(cluster_size, table_size));
-        return -EINVAL;
+        ret = -EINVAL;
+        goto finish;
     }
 
-    return qed_create(filename, cluster_size, image_size, table_size,
-                      backing_file, backing_fmt, errp);
+    ret = qed_create(filename, cluster_size, image_size, table_size,
+                     backing_file, backing_fmt, errp);
+
+finish:
+    g_free(backing_file);
+    g_free(backing_fmt);
+    return ret;
 }
 
 typedef struct {
@@ -1595,43 +1593,51 @@ static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
     return qed_check(s, result, !!fix);
 }
 
-static QEMUOptionParameter qed_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size (in bytes)"
-    }, {
-        .name = BLOCK_OPT_BACKING_FILE,
-        .type = OPT_STRING,
-        .help = "File name of a base image"
-    }, {
-        .name = BLOCK_OPT_BACKING_FMT,
-        .type = OPT_STRING,
-        .help = "Image format of the base image"
-    }, {
-        .name = BLOCK_OPT_CLUSTER_SIZE,
-        .type = OPT_SIZE,
-        .help = "Cluster size (in bytes)",
-        .value = { .n = QED_DEFAULT_CLUSTER_SIZE },
-    }, {
-        .name = BLOCK_OPT_TABLE_SIZE,
-        .type = OPT_SIZE,
-        .help = "L1/L2 table size (in clusters)"
-    },
-    { /* end of list */ }
+static QemuOptsList qed_create_opts = {
+    .name = "qed-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(qed_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FILE,
+            .type = QEMU_OPT_STRING,
+            .help = "File name of a base image"
+        },
+        {
+            .name = BLOCK_OPT_BACKING_FMT,
+            .type = QEMU_OPT_STRING,
+            .help = "Image format of the base image"
+        },
+        {
+            .name = BLOCK_OPT_CLUSTER_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Cluster size (in bytes)",
+            .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE)
+        },
+        {
+            .name = BLOCK_OPT_TABLE_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "L1/L2 table size (in clusters)"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_qed = {
     .format_name              = "qed",
     .instance_size            = sizeof(BDRVQEDState),
-    .create_options           = qed_create_options,
+    .create_opts              = &qed_create_opts,
 
     .bdrv_probe               = bdrv_qed_probe,
     .bdrv_rebind              = bdrv_qed_rebind,
     .bdrv_open                = bdrv_qed_open,
     .bdrv_close               = bdrv_qed_close,
     .bdrv_reopen_prepare      = bdrv_qed_reopen_prepare,
-    .bdrv_create              = bdrv_qed_create,
+    .bdrv_create2             = bdrv_qed_create,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
     .bdrv_aio_readv           = bdrv_qed_aio_readv,
diff --git a/block/qed.h b/block/qed.h
index 5d65bea..b024751 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -43,7 +43,7 @@
  *
  * All fields are little-endian on disk.
  */
-
+#define  QED_DEFAULT_CLUSTER_SIZE  65536
 enum {
     QED_MAGIC = 'Q' | 'E' << 8 | 'D' << 16 | '\0' << 24,
 
@@ -69,7 +69,6 @@ enum {
      */
     QED_MIN_CLUSTER_SIZE = 4 * 1024, /* in bytes */
     QED_MAX_CLUSTER_SIZE = 64 * 1024 * 1024,
-    QED_DEFAULT_CLUSTER_SIZE = 64 * 1024,
 
     /* Allocated clusters are tracked using a 2-level pagetable.  Table size is
      * a multiple of clusters so large maximum image sizes can be supported
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 22/33] raw-posix.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (20 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 21/33] qed.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 23/33] raw-win32.c: " Chunyan Liu
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/raw-posix.c | 59 +++++++++++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 3ce026d..4bbc7cc 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1237,8 +1237,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     return (int64_t)st.st_blocks * 512;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-                      Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int fd;
     int result = 0;
@@ -1247,12 +1246,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / BDRV_SECTOR_SIZE;
-        }
-        options++;
-    }
+    total_size =
+        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
                    0644);
@@ -1413,13 +1408,17 @@ static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
-static QEMUOptionParameter raw_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    { NULL }
+static QemuOptsList raw_create_opts = {
+    .name = "raw-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_file = {
@@ -1434,7 +1433,7 @@ static BlockDriver bdrv_file = {
     .bdrv_reopen_commit = raw_reopen_commit,
     .bdrv_reopen_abort = raw_reopen_abort,
     .bdrv_close = raw_close,
-    .bdrv_create = raw_create,
+    .bdrv_create2 = raw_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = raw_co_get_block_status,
     .bdrv_co_write_zeroes = raw_co_write_zeroes,
@@ -1451,7 +1450,7 @@ static BlockDriver bdrv_file = {
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
-    .create_options = raw_create_options,
+    .create_opts = &raw_create_opts,
 };
 
 /***********************************************/
@@ -1772,7 +1771,7 @@ static coroutine_fn int hdev_co_write_zeroes(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
-static int hdev_create(const char *filename, QEMUOptionParameter *options,
+static int hdev_create(const char *filename, QemuOpts *opts,
                        Error **errp)
 {
     int fd;
@@ -1793,12 +1792,8 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options,
     (void)has_prefix;
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, "size")) {
-            total_size = options->value.n / BDRV_SECTOR_SIZE;
-        }
-        options++;
-    }
+    total_size =
+        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
 
     fd = qemu_open(filename, O_WRONLY | O_BINARY);
     if (fd < 0) {
@@ -1835,8 +1830,8 @@ static BlockDriver bdrv_host_device = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
-    .bdrv_create        = hdev_create,
-    .create_options     = raw_create_options,
+    .bdrv_create2        = hdev_create,
+    .create_opts         = &raw_create_opts,
     .bdrv_co_write_zeroes = hdev_co_write_zeroes,
 
     .bdrv_aio_readv	= raw_aio_readv,
@@ -1979,8 +1974,8 @@ static BlockDriver bdrv_host_floppy = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
-    .bdrv_create        = hdev_create,
-    .create_options     = raw_create_options,
+    .bdrv_create2        = hdev_create,
+    .create_opts         = &raw_create_opts,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
@@ -2104,8 +2099,8 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
-    .bdrv_create        = hdev_create,
-    .create_options     = raw_create_options,
+    .bdrv_create2        = hdev_create,
+    .create_opts         = &raw_create_opts,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
@@ -2235,8 +2230,8 @@ static BlockDriver bdrv_host_cdrom = {
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit  = raw_reopen_commit,
     .bdrv_reopen_abort   = raw_reopen_abort,
-    .bdrv_create        = hdev_create,
-    .create_options     = raw_create_options,
+    .bdrv_create2        = hdev_create,
+    .create_opts        = &raw_create_opts,
 
     .bdrv_aio_readv     = raw_aio_readv,
     .bdrv_aio_writev    = raw_aio_writev,
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 23/33] raw-win32.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (21 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 22/33] raw-posix.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 24/33] raw_bsd.c: " Chunyan Liu
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/raw-win32.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 064ea31..e5ac297 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -478,8 +478,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     return st.st_size;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-                      Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int fd;
     int64_t total_size = 0;
@@ -487,12 +486,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     strstart(filename, "file:", &filename);
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            total_size = options->value.n / 512;
-        }
-        options++;
-    }
+    total_size =
+        qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
 
     fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
                    0644);
@@ -506,13 +501,18 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     return 0;
 }
 
-static QEMUOptionParameter raw_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    { NULL }
+
+static QemuOptsList raw_create_opts = {
+    .name = "raw-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_file = {
@@ -521,9 +521,9 @@ static BlockDriver bdrv_file = {
     .instance_size	= sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_parse_filename = raw_parse_filename,
-    .bdrv_file_open	= raw_open,
-    .bdrv_close		= raw_close,
-    .bdrv_create	= raw_create,
+    .bdrv_file_open     = raw_open,
+    .bdrv_close         = raw_close,
+    .bdrv_create2       = raw_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
 
     .bdrv_aio_readv     = raw_aio_readv,
@@ -535,7 +535,7 @@ static BlockDriver bdrv_file = {
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
 
-    .create_options = raw_create_options,
+    .create_opts        = &raw_create_opts,
 };
 
 /***********************************************/
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 24/33] raw_bsd.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (22 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 23/33] raw-win32.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 25/33] rbd.c: " Chunyan Liu
  2014-06-04  9:18 ` [Qemu-devel] [PATCH v27 00/33] " Stefan Hajnoczi
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/raw_bsd.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 9ae5fc2..ee797fd 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -29,13 +29,17 @@
 #include "block/block_int.h"
 #include "qemu/option.h"
 
-static QEMUOptionParameter raw_create_options[] = {
-    {
-        .name = BLOCK_OPT_SIZE,
-        .type = OPT_SIZE,
-        .help = "Virtual disk size"
-    },
-    { 0 }
+static QemuOptsList raw_create_opts = {
+    .name = "raw-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        { /* end of list */ }
+    }
 };
 
 static int raw_reopen_prepare(BDRVReopenState *reopen_state,
@@ -139,13 +143,12 @@ static int raw_has_zero_init(BlockDriverState *bs)
     return bdrv_has_zero_init(bs->file);
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-                      Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_create_file(filename, options, NULL, &local_err);
+    ret = bdrv_create_file(filename, NULL, opts, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
     }
@@ -177,7 +180,7 @@ static BlockDriver bdrv_raw = {
     .bdrv_reopen_prepare  = &raw_reopen_prepare,
     .bdrv_open            = &raw_open,
     .bdrv_close           = &raw_close,
-    .bdrv_create          = &raw_create,
+    .bdrv_create2         = &raw_create,
     .bdrv_co_readv        = &raw_co_readv,
     .bdrv_co_writev       = &raw_co_writev,
     .bdrv_co_write_zeroes = &raw_co_write_zeroes,
@@ -194,7 +197,7 @@ static BlockDriver bdrv_raw = {
     .bdrv_lock_medium     = &raw_lock_medium,
     .bdrv_ioctl           = &raw_ioctl,
     .bdrv_aio_ioctl       = &raw_aio_ioctl,
-    .create_options       = &raw_create_options[0],
+    .create_opts          = &raw_create_opts,
     .bdrv_has_zero_init   = &raw_has_zero_init
 };
 
-- 
1.7.12.4

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

* [Qemu-devel] [PATCH v27 25/33] rbd.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (23 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 24/33] raw_bsd.c: " Chunyan Liu
@ 2014-05-07  9:58 ` Chunyan Liu
  2014-06-04  9:18 ` [Qemu-devel] [PATCH v27 00/33] " Stefan Hajnoczi
  25 siblings, 0 replies; 32+ messages in thread
From: Chunyan Liu @ 2014-05-07  9:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: l, stefanha

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 block/rbd.c | 63 +++++++++++++++++++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index dbc79f4..f878877 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -282,8 +282,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
     return ret;
 }
 
-static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
-                           Error **errp)
+static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int64_t bytes = 0;
     int64_t objsize;
@@ -306,24 +305,18 @@ static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
     }
 
     /* Read out options */
-    while (options && options->name) {
-        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-            bytes = options->value.n;
-        } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) {
-            if (options->value.n) {
-                objsize = options->value.n;
-                if ((objsize - 1) & objsize) {    /* not a power of 2? */
-                    error_report("obj size needs to be power of 2");
-                    return -EINVAL;
-                }
-                if (objsize < 4096) {
-                    error_report("obj size too small");
-                    return -EINVAL;
-                }
-                obj_order = ffs(objsize) - 1;
-            }
+    bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+    objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
+    if (objsize) {
+        if ((objsize - 1) & objsize) {    /* not a power of 2? */
+            error_report("obj size needs to be power of 2");
+            return -EINVAL;
+        }
+        if (objsize < 4096) {
+            error_report("obj size too small");
+            return -EINVAL;
         }
-        options++;
+        obj_order = ffs(objsize) - 1;
     }
 
     clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
@@ -900,18 +893,22 @@ static BlockDriverAIOCB* qemu_rbd_aio_discard(BlockDriverState *bs,
 }
 #endif
 
-static QEMUOptionParameter qemu_rbd_create_options[] = {
-    {
-     .name = BLOCK_OPT_SIZE,
-     .type = OPT_SIZE,
-     .help = "Virtual disk size"
-    },
-    {
-     .name = BLOCK_OPT_CLUSTER_SIZE,
-     .type = OPT_SIZE,
-     .help = "RBD object size"
-    },
-    {NULL}
+static QemuOptsList qemu_rbd_create_opts = {
+    .name = "rbd-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_rbd_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_CLUSTER_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "RBD object size"
+        },
+        { /* end of list */ }
+    }
 };
 
 static BlockDriver bdrv_rbd = {
@@ -920,10 +917,10 @@ static BlockDriver bdrv_rbd = {
     .bdrv_needs_filename = true,
     .bdrv_file_open     = qemu_rbd_open,
     .bdrv_close         = qemu_rbd_close,
-    .bdrv_create        = qemu_rbd_create,
+    .bdrv_create2       = qemu_rbd_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_get_info      = qemu_rbd_getinfo,
-    .create_options     = qemu_rbd_create_options,
+    .create_opts        = &qemu_rbd_create_opts,
     .bdrv_getlength     = qemu_rbd_getlength,
     .bdrv_truncate      = qemu_rbd_truncate,
     .protocol_name      = "rbd",
-- 
1.7.12.4

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

* Re: [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work Chunyan Liu
@ 2014-06-04  8:54   ` Stefan Hajnoczi
  2014-06-05  3:32     ` Chun Yan Liu
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2014-06-04  8:54 UTC (permalink / raw)
  To: Chunyan Liu; +Cc: qemu-devel, stefanha, l

On Wed, May 07, 2014 at 05:58:32PM +0800, Chunyan Liu wrote:
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 4d2d4d1..32e1d50 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -575,6 +575,19 @@ static void qemu_opt_del(QemuOpt *opt)
>      g_free(opt);
>  }
>  
> +/* qemu_opt_set allows many settings for the same option.
> + * This function deletes all settings for an option.
> + */
> +static void qemu_opt_del_all(QemuOpts *opts, const char *name)
> +{
> +    QemuOpt *opt, *next_opt;
> +
> +    QTAILQ_FOREACH_SAFE(opt, &opts->head, next, next_opt) {
> +        if (!strcmp(opt->name, name))
> +            qemu_opt_del(opt);

QEMU coding style always uses curlies, even when the if body is only one
statement.  Please use scripts/checkpatch.pl to scan your patches before
they are sent.

I can fix this up while merging.  No need to resend.

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

* Re: [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
                   ` (24 preceding siblings ...)
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 25/33] rbd.c: " Chunyan Liu
@ 2014-06-04  9:18 ` Stefan Hajnoczi
  2014-06-05  2:34   ` Chun Yan Liu
  25 siblings, 1 reply; 32+ messages in thread
From: Stefan Hajnoczi @ 2014-06-04  9:18 UTC (permalink / raw)
  To: Chunyan Liu; +Cc: qemu-devel, stefanha, l

On Wed, May 07, 2014 at 05:58:25PM +0800, Chunyan Liu wrote:
> This patch series is to replace QEMUOptionParameter with QemuOpts, so that only
> one Qemu Option structure is kept in QEMU code.
> 
> ---
> Changes to v26:
>   * Following Eric's comment, backward split 2/33, 3/33.
>     (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc later).
>   * Fix memory free in qemu_opts_append to solve iotest issue. 10/33
>   * Following Eric's comment, remove the end '.' in error message. And update
>     qemu-iotests .out file. 12/33
>   * Following Eric's comment, fix memory free in vvfat.c 13/33
>   * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33:
>     export qemu_opt_find first, add qcow2 driver patch later.
>   * rebase to git master
> 
> All patches are also available from:
> https://github.com/chunyanliu/qemu/commits/QemuOpts

Please git rebase -x 'make && make check && make check-block' -i master:

qemu-img.c: In function ‘print_block_option_help’:
qemu-img.c:289:36: error: ‘create_options’ undeclared (first use in this
function)
             free_option_parameters(create_options);
	                                         ^

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

* Re: [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
  2014-06-04  9:18 ` [Qemu-devel] [PATCH v27 00/33] " Stefan Hajnoczi
@ 2014-06-05  2:34   ` Chun Yan Liu
  0 siblings, 0 replies; 32+ messages in thread
From: Chun Yan Liu @ 2014-06-05  2:34 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: l, stefanha, qemu-devel



>>> On 6/4/2014 at 05:18 PM, in message
<20140604091824.GB26902@stefanha-thinkpad.redhat.com>, Stefan Hajnoczi
<stefanha@gmail.com> wrote: 
> On Wed, May 07, 2014 at 05:58:25PM +0800, Chunyan Liu wrote: 
> > This patch series is to replace QEMUOptionParameter with QemuOpts, so that  
> only 
> > one Qemu Option structure is kept in QEMU code. 
> >  
> > --- 
> > Changes to v26: 
> >   * Following Eric's comment, backward split 2/33, 3/33. 
> >     (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc  
> later). 
> >   * Fix memory free in qemu_opts_append to solve iotest issue. 10/33 
> >   * Following Eric's comment, remove the end '.' in error message. And  
> update 
> >     qemu-iotests .out file. 12/33 
> >   * Following Eric's comment, fix memory free in vvfat.c 13/33 
> >   * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33: 
> >     export qemu_opt_find first, add qcow2 driver patch later. 
> >   * rebase to git master 
> >  
> > All patches are also available from: 
> > https://github.com/chunyanliu/qemu/commits/QemuOpts 
>  
> Please git rebase -x 'make && make check && make check-block' -i master: 
>  
> qemu-img.c: In function ‘print_block_option_help’: 
> qemu-img.c:289:36: error: ‘create_options’ undeclared (first use in this 
> function) 
>              free_option_parameters(create_options); 
> 	                                         ^ 
>  
>  
See. Code has been updated during this period of time since I sent the patch series
nearly a month ago. I'll rebase and resend. Thanks.

- Chunyan 

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

* Re: [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work
  2014-06-04  8:54   ` Stefan Hajnoczi
@ 2014-06-05  3:32     ` Chun Yan Liu
  0 siblings, 0 replies; 32+ messages in thread
From: Chun Yan Liu @ 2014-06-05  3:32 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: l, stefanha, qemu-devel



>>> On 6/4/2014 at 04:54 PM, in message
<20140604085436.GA26902@stefanha-thinkpad.redhat.com>, Stefan Hajnoczi
<stefanha@gmail.com> wrote: 
> On Wed, May 07, 2014 at 05:58:32PM +0800, Chunyan Liu wrote: 
> > diff --git a/util/qemu-option.c b/util/qemu-option.c 
> > index 4d2d4d1..32e1d50 100644 
> > --- a/util/qemu-option.c 
> > +++ b/util/qemu-option.c 
> > @@ -575,6 +575,19 @@ static void qemu_opt_del(QemuOpt *opt) 
> >      g_free(opt); 
> >  } 
> >   
> > +/* qemu_opt_set allows many settings for the same option. 
> > + * This function deletes all settings for an option. 
> > + */ 
> > +static void qemu_opt_del_all(QemuOpts *opts, const char *name) 
> > +{ 
> > +    QemuOpt *opt, *next_opt; 
> > + 
> > +    QTAILQ_FOREACH_SAFE(opt, &opts->head, next, next_opt) { 
> > +        if (!strcmp(opt->name, name)) 
> > +            qemu_opt_del(opt); 
>  
> QEMU coding style always uses curlies, even when the if body is only one 
> statement.  Please use scripts/checkpatch.pl to scan your patches before 
> they are sent. 
>  
> I can fix this up while merging.  No need to resend. 
>  
>  

Since I need rebase, I'll update.
In fact I checked with scripts/checkpatch.pl but it doesn't report error or warning. :-(

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

* Re: [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find Chunyan Liu
@ 2014-06-05 22:12   ` Eric Blake
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2014-06-05 22:12 UTC (permalink / raw)
  To: Chunyan Liu, qemu-devel; +Cc: l, stefanha

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

On 05/07/2014 03:58 AM, Chunyan Liu wrote:
> Export qemu_opt_find for qcow2 driver using it.
> After replacing QEMUOptionParameter with QemuOpts, qcow2 driver will
> use qemu_opt_find to judge if an option is explicitly set, to replace
> the usage of .assigned in QEMUOptionParameter.
> 
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
> Changes:
>   * split qcow2 patch into two: 19/33, 20/33
>     - export qemu_opt_find in a separate commit
>     - then patch qcow2 driver
> 
>  include/qemu/option.h | 1 +
>  util/qemu-option.c    | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts
  2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
@ 2014-06-05 22:27   ` Eric Blake
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2014-06-05 22:27 UTC (permalink / raw)
  To: Chunyan Liu, qemu-devel; +Cc: l, stefanha

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

On 05/07/2014 03:58 AM, Chunyan Liu wrote:
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
> Changes:
>   * remove exporting qemu_opt_find from qcow2 patch and commit separately.
>   * remove unnecessary indention changes in bdrv_qcow2 as Eric points out.
> 
>  block/qcow2.c | 261 +++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 139 insertions(+), 122 deletions(-)
> 


>              } else {
> -                fprintf(stderr, "Unknown compatibility level %s.\n",
> -                        options[i].value.s);
> +                fprintf(stderr, "Unknown compatibility level %s.\n", compat);

Not your fault this is using fprintf instead of error_report.  But may
be worth a followup patch to start converting this file to use common
error reporting practices.  Since this suggestion doesn't affect the
validaty of this series...

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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] 32+ messages in thread

end of thread, other threads:[~2014-06-05 22:27 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-07  9:58 [Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt->name|str from (const char *) to (char *) Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work Chunyan Liu
2014-06-04  8:54   ` Stefan Hajnoczi
2014-06-05  3:32     ` Chun Yan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 11/33] QemuOpts: check NULL input for qemu_opts_del Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 12/33] change block layer to support both QemuOpts and QEMUOptionParamter Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 13/33] vvfat.c: handle cross_driver's create_options and create_opts Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 14/33] cow.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 15/33] gluster.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 16/33] iscsi.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 17/33] nfs.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 18/33] qcow.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 19/33] QemuOpts: export qemu_opt_find Chunyan Liu
2014-06-05 22:12   ` Eric Blake
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-06-05 22:27   ` Eric Blake
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 21/33] qed.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 22/33] raw-posix.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 23/33] raw-win32.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 24/33] raw_bsd.c: " Chunyan Liu
2014-05-07  9:58 ` [Qemu-devel] [PATCH v27 25/33] rbd.c: " Chunyan Liu
2014-06-04  9:18 ` [Qemu-devel] [PATCH v27 00/33] " Stefan Hajnoczi
2014-06-05  2:34   ` Chun Yan Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).