* [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser
@ 2013-07-17 9:29 Dong Xu Wang
2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 1/9] qemu-option: add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print Dong Xu Wang
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang
These patches will replace QEMUOptionParameter with QemuOpts. Change logs
please go to each patch's commit message.
Dong Xu Wang (9):
qemu-option: add def_value_str in QemuOptDesc struct and rewrite
qemu_opts_print
qemu-option: avoid duplication of default value in QemuOpts
qemu-option: create four QemuOptsList related functions
qemu-option: create some QemuOpts functons
block: use QemuOpts support in block layer
qapi: query-command-line-options outputs def_value_str
qemu-option: remove QEMUOptionParameter related functions and struct
qemu-option: make qemu_opts_del accept opts being NULL
qemu-option: use qemu_opts_del without judging NULL
block.c | 96 ++++----
block/cow.c | 54 ++---
block/gluster.c | 37 ++-
block/iscsi.c | 31 ++-
block/qcow.c | 69 +++---
block/qcow2.c | 205 +++++++++--------
block/qed.c | 114 ++++-----
block/qed.h | 2 +-
block/raw-posix.c | 49 ++--
block/raw-win32.c | 31 +--
block/raw.c | 30 +--
block/rbd.c | 62 +++--
block/sheepdog.c | 81 ++++---
block/ssh.c | 29 ++-
block/vdi.c | 70 +++---
block/vmdk.c | 179 +++++++-------
block/vpc.c | 65 +++---
block/vvfat.c | 11 +-
hw/core/qdev.c | 4 +-
include/block/block.h | 5 +-
include/block/block_int.h | 6 +-
include/qemu/option.h | 56 ++---
qapi-schema.json | 8 +-
qemu-char.c | 4 +-
qemu-img.c | 63 +++--
qmp-commands.hx | 2 +
util/qemu-config.c | 4 +
util/qemu-option.c | 576 +++++++++++++++++++++-------------------------
28 files changed, 930 insertions(+), 1013 deletions(-)
--
V17 add qemu_opts_del's support when opts is NULL, and made changes based on
Eric's comments.
I tried to split PATCH 5 into small pieces, but I found it is very hard, bdrv_*
functions are all using QEMUOptionParameter or QemuOpts, if I split them, I have
to support two parsers in block.c and qemu-img.c, it will make code very hard to
read, so I leave them in one big patch, I am sorry it is realy hard to review.
Changelogs please goto each patch.
1.7.11.7
^ permalink raw reply [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH V17 1/9] qemu-option: add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 2/9] qemu-option: avoid duplication of default value in QemuOpts Dong Xu Wang ` (8 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang qemu_opts_print has no user now, so can re-write the function safely. qemu_opts_print is used while using "qemu-img create", it produces the same output as previous code. The behavior of this function has changed: 1. Print every possible option, whether a value has been set or not. 2. Option descriptors may provide a default value. 3. Print to stdout instead of stderr. Previously the behavior was to print every option that has been set. Options that have not been set would be skipped. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v13->v14: 1) fix memory leak. 2) make opt_set do not accpet null value argument. v12->v13 1) re-write commit message. v11->v12 1) make def_value_str become the real default value string in opt_set function. v10->v11: 1) print all values that have actually been assigned while accept-any cases. v7->v8: 1) print "elements => accept any params" while opts_accepts_any() == true. 2) since def_print_str is the default value if an option isn't set, so rename it to def_value_str. include/qemu/option.h | 3 ++- util/qemu-option.c | 32 ++++++++++++++++++++++++++------ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index a83c700..0207209 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -94,6 +94,7 @@ typedef struct QemuOptDesc { const char *name; enum QemuOptType type; const char *help; + const char *def_value_str; } QemuOptDesc; struct QemuOptsList { @@ -150,7 +151,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); +int 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 e0ef426..be5dfaa 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -826,16 +826,36 @@ void qemu_opts_del(QemuOpts *opts) g_free(opts); } -int qemu_opts_print(QemuOpts *opts, void *dummy) +int qemu_opts_print(QemuOpts *opts) { QemuOpt *opt; + QemuOptDesc *desc = opts->list->desc; - fprintf(stderr, "%s: %s:", opts->list->name, - opts->id ? opts->id : "<noid>"); - QTAILQ_FOREACH(opt, &opts->head, next) { - fprintf(stderr, " %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 0; + } + for (; desc && desc->name; desc++) { + const char *value = desc->def_value_str; + QemuOpt *opt; + + opt = qemu_opt_find(opts, desc->name); + if (opt) { + value = opt->str; + } + + if (!value) { + continue; + } + + if (desc->type == QEMU_OPT_STRING) { + printf("%s='%s' ", desc->name, value); + } else { + printf("%s=%s ", desc->name, value); + } } - fprintf(stderr, "\n"); return 0; } -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 2/9] qemu-option: avoid duplication of default value in QemuOpts 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 1/9] qemu-option: add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 3/9] qemu-option: create four QemuOptsList related functions Dong Xu Wang ` (7 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang This patch moves the default value entirely to QemuOptDesc. When getting the value of an option that hasn't been set, and QemuOptDesc has a default value, return that. Else, behave as before. Example: qemu_opt_get_number(opts, "foo", 42) If "foo" has been set in opts, return its value. Else, if opt's QemuOptDesc has a default value for "foo", return that. Else, return 42. Note that the last argument is useless when QemuOptDesc has a default value. Ugly. If it bothers us, we could assert. Example: qemu_opt_get(opts, "bar") If "bar" has been set in opts, return its value. Else, if opt's QemuOptDesc has a default value for "bar", return that. Else, return NULL. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v13->v14: 1) change code style. 2) assert errors. util/qemu-option.c | 66 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index be5dfaa..86cc64e 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -495,9 +495,31 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) return 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; +} + const char *qemu_opt_get(QemuOpts *opts, const char *name) { QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + + if (!opt) { + 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; } @@ -516,9 +538,17 @@ bool qemu_opt_has_help_opt(QemuOpts *opts) bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) { QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + Error *local_err = NULL; - if (opt == NULL) + if (opt == NULL) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + parse_option_bool(name, desc->def_value_str, &defval, &local_err); + assert(!local_err); + } return defval; + } assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); return opt->value.boolean; } @@ -526,9 +556,17 @@ 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) { QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + Error *local_err = NULL; - if (opt == NULL) + if (opt == NULL) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + parse_option_number(name, desc->def_value_str, &defval, &local_err); + assert(!local_err); + } return defval; + } assert(opt->desc && opt->desc->type == QEMU_OPT_NUMBER); return opt->value.uint; } @@ -536,9 +574,17 @@ 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) { QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + Error *local_err = NULL; - if (opt == NULL) + if (opt == NULL) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + parse_option_size(name, desc->def_value_str, &defval, &local_err); + assert(!local_err); + } return defval; + } assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); return opt->value.uint; } @@ -579,20 +625,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; -} - static void opt_set(QemuOpts *opts, const char *name, const char *value, bool prepend, Error **errp) { -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 3/9] qemu-option: create four QemuOptsList related functions 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 1/9] qemu-option: add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 2/9] qemu-option: avoid duplication of default value in QemuOpts Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 4/9] qemu-option: create some QemuOpts functons Dong Xu Wang ` (6 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang This patch creates 4 functions, count_opts_list, qemu_opts_append, qemu_opts_free and qemu_opts_print_help, they are used in following commits. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v16->v17: 1) fix indentation. 2) fix typo. v15->v16: 1) discard double-initialization. 2) use pointer directly, not g_strdup. v12->v13: 1) simply assert that neither argument has merge_lists set. 2) drop superfluous paranthesesis around p == first. v11->v12: 1) renmae functions. 2) fix loop styles and code styles. 3) qemu_opts_apend will not return NULL now. 4) merge_lists value is from arguments in qemu_opts_append. v6->v7: 1) Fix typo. v5->v6: 1) allocate enough space in append_opts_list function. include/qemu/option.h | 3 +++ util/qemu-option.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/include/qemu/option.h b/include/qemu/option.h index 0207209..7473e2b 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -155,4 +155,7 @@ int qemu_opts_print(QemuOpts *opts); int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, int abort_on_failure); +QemuOptsList *qemu_opts_append(QemuOptsList *first, QemuOptsList *second); +void qemu_opts_free(QemuOptsList *list); +void qemu_opts_print_help(QemuOptsList *list); #endif diff --git a/util/qemu-option.c b/util/qemu-option.c index 86cc64e..3a055a7 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1176,3 +1176,76 @@ 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) +{ + size_t i; + + for (i = 0; list && list->desc[i].name; i++) { + ; + } + + return i; +} + +/* Create a new QemuOptsList with a desc of the merge of the first + * and second. It will allocate space for one new QemuOptsList plus + * enough space for QemuOptDesc in first and second QemuOptsList. + * First argument's QemuOptDesc members take precedence over second's. + * The result's name and implied_opt_name are not copied from them. + * Both merge_lists should not be set. Both lists can be NULL. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *first, + QemuOptsList *second) +{ + size_t num_first_opts, num_second_opts; + QemuOptsList *dest = NULL; + int i; + int index = 0; + QemuOptsList *p = first; + + num_first_opts = count_opts_list(first); + num_second_opts = count_opts_list(second); + + dest = g_malloc0(sizeof(QemuOptsList) + + (num_first_opts + num_second_opts + 1) * sizeof(QemuOptDesc)); + + dest->name = "append_opts_list"; + dest->implied_opt_name = NULL; + assert((!first || !first->merge_lists) + && (!second || !second->merge_lists)); + QTAILQ_INIT(&dest->head); + + for (i = 0; p && p->desc[i].name; i++) { + if (!find_desc_by_name(dest->desc, p->desc[i].name)) { + dest->desc[index].name = p->desc[i].name; + dest->desc[index].help = p->desc[i].help; + dest->desc[index].type = p->desc[i].type; + dest->desc[index].def_value_str = p->desc[i].def_value_str; + index++; + } + if (p == first && p && !p->desc[i].name) { + p = second; + i = 0; + } + } + dest->desc[index].name = NULL; + return dest; +} + +/* free a QemuOptsList, can accept NULL as arguments */ +void qemu_opts_free(QemuOptsList *list) +{ + g_free(list); +} + +void qemu_opts_print_help(QemuOptsList *list) +{ + int i; + printf("Supported options:\n"); + for (i = 0; list && list->desc[i].name; i++) { + printf("%-16s %s\n", list->desc[i].name, + list->desc[i].help ? + list->desc[i].help : "No description available"); + } +} -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 4/9] qemu-option: create some QemuOpts functons 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (2 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 3/9] qemu-option: create four QemuOptsList related functions Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer Dong Xu Wang ` (5 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang These functions will be used in next commit. qemu_opt_get_(*)_del functions are used to make sure we have the same behaviors as before: in block layer, after parsing a parameter value, parameter list will delete it to avoid parsing it twice. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v16->v17: 1) return "char *" instead of "const char*" v13->v14: 1) rewrite commit message. 2) use def_value_str in qemu_opt_get_FOO_del() and qemu_opt_get_del(). 3) delete redundant qemu_opt_del(opt). include/qemu/option.h | 11 ++++- util/qemu-option.c | 130 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 125 insertions(+), 16 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index 7473e2b..12fa8b4 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -106,6 +106,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 @@ -119,13 +120,18 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name); */ bool qemu_opt_has_help_opt(QemuOpts *opts); bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval); +bool qemu_opt_get_bool_del(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); +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, + uint64_t defval); int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value); void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp); int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val); +int qemu_opt_replace_set_number(QemuOpts *opts, const char *name, int64_t val); typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque); int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure); @@ -142,7 +148,10 @@ const char *qemu_opts_id(QemuOpts *opts); void qemu_opts_del(QemuOpts *opts); void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); -QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, + const char *firstname); +QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, + int permit_abbrev); void qemu_opts_set_defaults(QemuOptsList *list, const char *params, int permit_abbrev); QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, diff --git a/util/qemu-option.c b/util/qemu-option.c index 3a055a7..41f5b0f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -523,6 +523,32 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) return opt ? opt->str : NULL; } +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); +} + +char *qemu_opt_get_del(QemuOpts *opts, const char *name) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + char *str = NULL; + + if (!opt) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + str = g_strdup(desc->def_value_str); + } + } else { + str = g_strdup(opt->str); + qemu_opt_del(opt); + } + return str; +} + bool qemu_opt_has_help_opt(QemuOpts *opts) { QemuOpt *opt; @@ -553,6 +579,27 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) return opt->value.boolean; } +bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + Error *local_err = NULL; + bool ret = defval; + + if (opt == NULL) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + parse_option_bool(name, desc->def_value_str, &ret, &local_err); + assert(!local_err); + } + return ret; + } + assert(opt->desc && opt->desc->type == QEMU_OPT_BOOL); + ret = opt->value.boolean; + qemu_opt_del(opt); + return ret; +} + uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) { QemuOpt *opt = qemu_opt_find(opts, name); @@ -589,6 +636,28 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) return opt->value.uint; } +uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, + uint64_t defval) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + const QemuOptDesc *desc; + Error *local_err = NULL; + uint64_t ret = defval; + + if (opt == NULL) { + desc = find_desc_by_name(opts->list->desc, name); + if (desc && desc->def_value_str) { + parse_option_size(name, desc->def_value_str, &ret, &local_err); + assert(!local_err); + } + return ret; + } + assert(opt->desc && opt->desc->type == QEMU_OPT_SIZE); + ret = opt->value.uint; + qemu_opt_del(opt); + return ret; +} + static void qemu_opt_parse(QemuOpt *opt, Error **errp) { if (opt->desc == NULL) @@ -612,21 +681,13 @@ 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); -} - static bool opts_accepts_any(const QemuOpts *opts) { return opts->list->desc[0].name == NULL; } static void opt_set(QemuOpts *opts, const char *name, const char *value, - bool prepend, Error **errp) + bool prepend, bool replace, Error **errp) { QemuOpt *opt; const QemuOptDesc *desc; @@ -638,6 +699,11 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, return; } + opt = qemu_opt_find(opts, name); + if (replace && opt) { + qemu_opt_del(opt); + } + opt = g_malloc0(sizeof(*opt)); opt->name = g_strdup(name); opt->opts = opts; @@ -659,7 +725,25 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) { Error *local_err = NULL; - opt_set(opts, name, value, false, &local_err); + opt_set(opts, name, value, false, false, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + return -1; + } + + return 0; +} + +/* + * If name exists, the function will delete the opt first and then add a new + * one. + */ +int qemu_opt_replace_set(QemuOpts *opts, const char *name, const char *value) +{ + Error *local_err = NULL; + + opt_set(opts, name, value, false, true, &local_err); if (error_is_set(&local_err)) { qerror_report_err(local_err); error_free(local_err); @@ -672,7 +756,7 @@ 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, Error **errp) { - opt_set(opts, name, value, false, errp); + opt_set(opts, name, value, false, false, errp); } int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) @@ -719,6 +803,16 @@ int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) return 0; } +int qemu_opt_replace_set_number(QemuOpts *opts, const char *name, int64_t val) +{ + QemuOpt *opt = qemu_opt_find(opts, name); + + if (opt) { + qemu_opt_del(opt); + } + return qemu_opt_set_number(opts, name, val); +} + int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure) { @@ -892,7 +986,7 @@ int qemu_opts_print(QemuOpts *opts) } static int opts_do_parse(QemuOpts *opts, const char *params, - const char *firstname, bool prepend) + const char *firstname, bool prepend, bool replace) { char option[128], value[1024]; const char *p,*pe,*pc; @@ -928,7 +1022,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params, } if (strcmp(option, "id") != 0) { /* store and parse */ - opt_set(opts, option, value, prepend, &local_err); + opt_set(opts, option, value, prepend, replace, &local_err); if (error_is_set(&local_err)) { qerror_report_err(local_err); error_free(local_err); @@ -944,7 +1038,13 @@ static int opts_do_parse(QemuOpts *opts, const char *params, int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) { - return opts_do_parse(opts, params, firstname, false); + return opts_do_parse(opts, params, firstname, false, false); +} + +int qemu_opts_do_parse_replace(QemuOpts *opts, const char *params, + const char *firstname) +{ + return opts_do_parse(opts, params, firstname, false, true); } static QemuOpts *opts_parse(QemuOptsList *list, const char *params, @@ -975,7 +1075,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return NULL; } - if (opts_do_parse(opts, params, firstname, defaults) != 0) { + if (opts_do_parse(opts, params, firstname, defaults, false) != 0) { qemu_opts_del(opts); return NULL; } -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (3 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 4/9] qemu-option: create some QemuOpts functons Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:48 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 6/9] qapi: query-command-line-options outputs def_value_str Dong Xu Wang ` (4 subsequent siblings) 9 siblings, 1 reply; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang This patch uses QemuOpts related functions in block layer, add a member bdrv_create_opts to BlockDriver struct, it returns a QemuOptsList pointer, which includes the image format's create options. And create options' primary consumer is block creating related functions, so modify them together. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v16->v17: 1) remove const based on Patch 4/7 2) s/0/false/ in qemu_opt_get_bool_del. 3) change qed.c's logic based on Eric's comments. 4) use qemu_ prefix in rbd.c. 5) fix some alignments. v13->v14: 1) add block/ssh.c support. 2) fix memory leak after using qemu_opt_get_del. v12->v13: 1) split into 2 patches. one for block, one for QemuOpts funcitons. 2) fix leaking cco->opts. 3) other small fix. v11->v12: 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. These functions works like origin code. 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. 3) in bdrv_create, if opts is NULL, will create an empty one, so can discard if(opts) code safely. v10->v11: 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or qemu_opts_print produce un-expanded cluster_size. 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> opts, or while using protocol, there will be an error. v8->v9: 1) add qemu_ prefix to gluster_create_opts. 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be converted. v7->v8: 1) rebase to upstream source tree. 2) add gluster.c, raw-win32.c, and rbd.c. v6->v7: 1) use osdep.h:stringify(), not redefining new macro. 2) preserve TODO comment. 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. 4) initialize disk_type even when opts is NULL. v5->v6: 1) judge if opts == NULL in block layer create functions. 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create funtion. 3) made more readable while using qemu_opt_get_number. block.c | 100 +++++++++++----------- block/cow.c | 54 ++++++------ block/gluster.c | 37 ++++----- block/iscsi.c | 31 ++++--- block/qcow.c | 69 ++++++++-------- block/qcow2.c | 205 +++++++++++++++++++++++++--------------------- block/qed.c | 114 ++++++++++++++------------ block/qed.h | 2 +- block/raw-posix.c | 49 +++++------ block/raw-win32.c | 31 +++---- block/raw.c | 30 ++++--- block/rbd.c | 62 +++++++------- block/sheepdog.c | 81 +++++++++--------- block/ssh.c | 29 ++++--- block/vdi.c | 70 ++++++++-------- block/vmdk.c | 179 +++++++++++++++++++--------------------- block/vpc.c | 65 ++++++++------- block/vvfat.c | 11 +-- include/block/block.h | 5 +- include/block/block_int.h | 6 +- qemu-img.c | 65 +++++++-------- 21 files changed, 647 insertions(+), 648 deletions(-) diff --git a/block.c b/block.c index b560241..e1d1c3d 100644 --- a/block.c +++ b/block.c @@ -365,7 +365,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name, typedef struct CreateCo { BlockDriver *drv; char *filename; - QEMUOptionParameter *options; + QemuOpts *opts; int ret; } CreateCo; @@ -374,11 +374,10 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) CreateCo *cco = opaque; assert(cco->drv); - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); } -int bdrv_create(BlockDriver *drv, const char* filename, - QEMUOptionParameter *options) +int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts) { int ret; @@ -386,7 +385,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, CreateCo cco = { .drv = drv, .filename = g_strdup(filename), - .options = options, + .opts = opts ?: qemu_opts_create_nofail(drv->bdrv_create_opts), .ret = NOT_DONE, }; @@ -409,11 +408,14 @@ int bdrv_create(BlockDriver *drv, const char* filename, ret = cco.ret; out: + if (!opts) { + qemu_opts_del(cco.opts); + } g_free(cco.filename); return ret; } -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) +int bdrv_create_file(const char *filename, QemuOpts *opts) { BlockDriver *drv; @@ -422,7 +424,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) return -ENOENT; } - return bdrv_create(drv, filename, options); + return bdrv_create(drv, filename, opts); } /* @@ -984,7 +986,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, BlockDriverState *bs1; int64_t total_size; BlockDriver *bdrv_qcow2; - QEMUOptionParameter *create_options; + QemuOpts *opts; char backing_filename[PATH_MAX]; if (qdict_size(options) != 0) { @@ -1023,19 +1025,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } bdrv_qcow2 = bdrv_find_format("qcow2"); - create_options = parse_option_parameters("", bdrv_qcow2->create_options, - NULL); + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_opts); - set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size); - set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE, - backing_filename); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); if (drv) { - set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT, - drv->format_name); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); } - ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options); - free_option_parameters(create_options); + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); + qemu_opts_del(opts); if (ret < 0) { goto fail; } @@ -4451,8 +4450,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; + QemuOpts *opts = NULL; + QemuOptsList *create_opts = NULL; + const char *backing_fmt, *backing_file; + int64_t size; BlockDriverState *bs = NULL; BlockDriver *drv, *proto_drv; BlockDriver *backing_drv = NULL; @@ -4470,28 +4471,23 @@ void bdrv_img_create(const char *filename, const char *fmt, error_setg(errp, "Unknown protocol '%s'", filename); 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(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); /* Create parameter list with default values */ - param = parse_option_parameters("", create_options, param); + opts = qemu_opts_create_nofail(create_opts); - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); + 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) { + if (qemu_opts_do_parse_replace(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, + if (qemu_opt_replace_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) { error_setg(errp, "Backing file not supported for file format '%s'", fmt); @@ -4500,39 +4496,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_replace_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); + error_setg(errp, "Unknown backing file format '%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) { uint64_t size; - char buf[32]; int back_flags; /* backing files always opened read-only */ @@ -4541,18 +4535,16 @@ void bdrv_img_create(const char *filename, const char *fmt, bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags, - backing_drv); + ret = bdrv_open(bs, backing_file, NULL, back_flags, backing_drv); if (ret < 0) { error_setg_errno(errp, -ret, "Could not open '%s'", - backing_file->value.s); + backing_file); goto out; } bdrv_get_geometry(bs, &size); size *= 512; - snprintf(buf, sizeof(buf), "%" PRId64, size); - set_option_parameter(param, BLOCK_OPT_SIZE, buf); + qemu_opt_replace_set_number(opts, BLOCK_OPT_SIZE, size); } else { error_setg(errp, "Image creation needs a size parameter"); goto out; @@ -4561,17 +4553,17 @@ 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); + ret = bdrv_create(drv, filename, opts); if (ret < 0) { if (ret == -ENOTSUP) { error_setg(errp,"Formatting or formatting option not supported for " "file format '%s'", fmt); } else if (ret == -EFBIG) { 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'%s", @@ -4583,8 +4575,10 @@ void bdrv_img_create(const char *filename, const char *fmt, } out: - free_option_parameters(create_options); - free_option_parameters(param); + if (opts) { + qemu_opts_del(opts); + } + qemu_opts_free(create_opts); if (bs) { bdrv_delete(bs); diff --git a/block/cow.c b/block/cow.c index 1cc2e89..9d4da10 100644 --- a/block/cow.c +++ b/block/cow.c @@ -255,33 +255,27 @@ static void cow_close(BlockDriverState *bs) { } -static int cow_create(const char *filename, QEMUOptionParameter *options) +static int cow_create(const char *filename, QemuOpts *opts) { struct cow_header_v2 cow_header; struct stat st; int64_t image_sectors = 0; - const char *image_filename = NULL; + char *image_filename = 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++; - } + /* Read out opts */ + 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); + ret = bdrv_create_file(filename, opts); if (ret < 0) { - return ret; + goto finish; } ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { - return ret; + goto finish; } memset(&cow_header, 0, sizeof(cow_header)); @@ -315,21 +309,27 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) exit: bdrv_delete(cow_bs); +finish: + g_free(image_filename); 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 = { @@ -346,7 +346,7 @@ static BlockDriver bdrv_cow = { .bdrv_write = cow_co_write, .bdrv_co_is_allocated = cow_co_is_allocated, - .create_options = cow_create_options, + .bdrv_create_opts = &cow_create_opts, }; static void bdrv_cow_init(void) diff --git a/block/gluster.c b/block/gluster.c index 61424bc..8c52f8d 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -365,8 +365,7 @@ out: return ret; } -static int qemu_gluster_create(const char *filename, - QEMUOptionParameter *options) +static int qemu_gluster_create(const char *filename, QemuOpts *opts) { struct glfs *glfs; struct glfs_fd *fd; @@ -380,12 +379,8 @@ 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; - } - options++; - } + total_size = + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; fd = glfs_creat(glfs, gconf->image, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR); @@ -580,13 +575,17 @@ 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" - }, - { 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" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_gluster = { @@ -602,7 +601,7 @@ static BlockDriver bdrv_gluster = { .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, .bdrv_has_zero_init = qemu_gluster_has_zero_init, - .create_options = qemu_gluster_create_options, + .bdrv_create_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_tcp = { @@ -618,7 +617,7 @@ static BlockDriver bdrv_gluster_tcp = { .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, .bdrv_has_zero_init = qemu_gluster_has_zero_init, - .create_options = qemu_gluster_create_options, + .bdrv_create_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_unix = { @@ -634,7 +633,7 @@ static BlockDriver bdrv_gluster_unix = { .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, .bdrv_has_zero_init = qemu_gluster_has_zero_init, - .create_options = qemu_gluster_create_options, + .bdrv_create_opts = &qemu_gluster_create_opts, }; static BlockDriver bdrv_gluster_rdma = { @@ -650,7 +649,7 @@ static BlockDriver bdrv_gluster_rdma = { .bdrv_aio_writev = qemu_gluster_aio_writev, .bdrv_aio_flush = qemu_gluster_aio_flush, .bdrv_has_zero_init = qemu_gluster_has_zero_init, - .create_options = qemu_gluster_create_options, + .bdrv_create_opts = &qemu_gluster_create_opts, }; static void bdrv_gluster_init(void) diff --git a/block/iscsi.c b/block/iscsi.c index 0bbf0b1..e4770fb 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1196,7 +1196,7 @@ static int iscsi_has_zero_init(BlockDriverState *bs) return 0; } -static int iscsi_create(const char *filename, QEMUOptionParameter *options) +static int iscsi_create(const char *filename, QemuOpts *opts) { int ret = 0; int64_t total_size = 0; @@ -1207,13 +1207,8 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) memset(&bs, 0, sizeof(BlockDriverState)); /* 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; @@ -1246,13 +1241,17 @@ out: return ret; } -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 = { @@ -1263,7 +1262,7 @@ static BlockDriver bdrv_iscsi = { .bdrv_file_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_create = iscsi_create, - .create_options = iscsi_create_options, + .bdrv_create_opts = &iscsi_create_opts, .bdrv_getlength = iscsi_getlength, .bdrv_truncate = iscsi_truncate, diff --git a/block/qcow.c b/block/qcow.c index 5239bd6..a00f4cc 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -651,37 +651,32 @@ static void qcow_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static int qcow_create(const char *filename, QEMUOptionParameter *options) +static int qcow_create(const char *filename, QemuOpts *opts) { 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; 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++; + /* Read out opts */ + 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); + ret = bdrv_create_file(filename, opts); if (ret < 0) { - return ret; + goto finish; } ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { - return ret; + goto finish; } ret = bdrv_truncate(qcow_bs, 0); @@ -752,6 +747,8 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) ret = 0; exit: bdrv_delete(qcow_bs); +finish: + g_free(backing_file); return ret; } @@ -864,24 +861,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 = { @@ -903,7 +904,7 @@ static BlockDriver bdrv_qcow = { .bdrv_write_compressed = qcow_write_compressed, .bdrv_get_info = qcow_get_info, - .create_options = qcow_create_options, + .bdrv_create_opts = &qcow_create_opts, }; static void bdrv_qcow_init(void) diff --git a/block/qcow2.c b/block/qcow2.c index 0eceefe..d0b8540 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1258,12 +1258,13 @@ static int preallocate(BlockDriverState *bs) } static int qcow2_create2(const char *filename, int64_t total_size, - const char *backing_file, const char *backing_format, + char *backing_file, char *backing_format, int flags, size_t cluster_size, int prealloc, - QEMUOptionParameter *options, int version) + QemuOpts *opts, int version) { /* Calculate cluster_bits */ int cluster_bits; + int ret = 0; cluster_bits = ffs(cluster_size) - 1; if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS || (1 << cluster_bits) != cluster_size) @@ -1271,7 +1272,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, error_report( "Cluster size must be a power of two between %d and %dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10)); - return -EINVAL; + ret = -EINVAL; + goto finish; } /* @@ -1289,16 +1291,15 @@ static int qcow2_create2(const char *filename, int64_t total_size, BlockDriverState* bs; QCowHeader header; uint8_t* refcount_table; - int ret; - ret = bdrv_create_file(filename, options); + ret = bdrv_create_file(filename, opts); if (ret < 0) { - return ret; + goto finish; } ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR); if (ret < 0) { - return ret; + goto finish; } /* Write the header */ @@ -1391,73 +1392,82 @@ static int qcow2_create2(const char *filename, int64_t total_size, ret = 0; out: bdrv_delete(bs); +finish: + g_free(backing_file); + g_free(backing_format); return ret; } -static int qcow2_create(const char *filename, QEMUOptionParameter *options) +static int qcow2_create(const char *filename, QemuOpts *opts) { - const char *backing_file = NULL; - const char *backing_fmt = NULL; + char *backing_file = NULL; + char *backing_fmt = NULL; uint64_t sectors = 0; int flags = 0; size_t cluster_size = DEFAULT_CLUSTER_SIZE; int prealloc = 0; int version = 2; + char *buf; + int ret = 0; /* 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 { - fprintf(stderr, "Invalid preallocation mode: '%s'\n", - options->value.s); - return -EINVAL; - } - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) { - if (!options->value.s || !strcmp(options->value.s, "0.10")) { - version = 2; - } else if (!strcmp(options->value.s, "1.1")) { - version = 3; - } else { - fprintf(stderr, "Invalid compatibility level: '%s'\n", - 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 { + fprintf(stderr, "Invalid preallocation mode: '%s'\n", + buf); + ret = -EINVAL; + goto finish; + } + g_free(buf); + buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL); + if (!buf || !strcmp(buf, "0.10")) { + version = 2; + } else if (!strcmp(buf, "1.1")) { + version = 3; + } else { + fprintf(stderr, "Invalid compatibility level: '%s'\n", + 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) { fprintf(stderr, "Backing file and preallocation cannot be used at " "the same time\n"); - return -EINVAL; + ret = -EINVAL; + goto finish; } if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) { fprintf(stderr, "Lazy refcounts only supported with compatibility " "level 1.1 and above (use compat=1.1 or greater)\n"); - return -EINVAL; + ret = -EINVAL; + goto finish; } return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, - cluster_size, prealloc, options, version); + cluster_size, prealloc, opts, version); +finish: + g_free(backing_file); + g_free(backing_fmt); + g_free(buf); + return ret; } static int qcow2_make_empty(BlockDriverState *bs) @@ -1732,49 +1742,55 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, return ret; } -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 = { @@ -1813,8 +1829,9 @@ static BlockDriver bdrv_qcow2 = { .bdrv_invalidate_cache = qcow2_invalidate_cache, - .create_options = qcow2_create_options, .bdrv_check = qcow2_check, + + .bdrv_create_opts = &qcow2_create_opts, }; static void bdrv_qcow2_init(void) diff --git a/block/qed.c b/block/qed.c index f767b05..6514370 100644 --- a/block/qed.c +++ b/block/qed.c @@ -555,12 +555,12 @@ static int qed_create(const char *filename, uint32_t cluster_size, ret = bdrv_create_file(filename, NULL); if (ret < 0) { - return ret; + goto finish; } ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB); if (ret < 0) { - return ret; + goto finish; } /* File must start empty and grow, check truncate is supported */ @@ -600,55 +600,55 @@ static int qed_create(const char *filename, uint32_t cluster_size, out: g_free(l1_table); bdrv_delete(bs); +finish: return ret; } -static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) +static int bdrv_qed_create(const char *filename, QemuOpts *opts) { 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 = 0; + + 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)) { fprintf(stderr, "QED cluster size must be within range [%u, %u] and power of 2\n", QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE); - return -EINVAL; + ret = -EINVAL; + goto finish; } if (!qed_is_table_size_valid(table_size)) { fprintf(stderr, "QED table size must be within range [%u, %u] and power of 2\n", 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)) { fprintf(stderr, "QED image size must be a non-zero multiple of " "cluster size and less than %" PRIu64 " bytes\n", qed_max_image_size(cluster_size, table_size)); - return -EINVAL; + ret = -EINVAL; + goto finish; } - return qed_create(filename, cluster_size, image_size, table_size, + ret = qed_create(filename, cluster_size, image_size, table_size, backing_file, backing_fmt); + +finish: + g_free(backing_file); + g_free(backing_fmt); + return ret; } typedef struct { @@ -1537,36 +1537,44 @@ 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, + .bdrv_create_opts = &qed_create_opts, .bdrv_probe = bdrv_qed_probe, .bdrv_rebind = bdrv_qed_rebind, diff --git a/block/qed.h b/block/qed.h index 2b4dded..99a7726 100644 --- a/block/qed.h +++ b/block/qed.h @@ -43,6 +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 +70,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 diff --git a/block/raw-posix.c b/block/raw-posix.c index ba721d3..b47e688 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1040,20 +1040,14 @@ 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) +static int raw_create(const char *filename, QemuOpts *opts) { int fd; int result = 0; int64_t total_size = 0; - /* 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); if (fd < 0) { @@ -1179,13 +1173,17 @@ static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs, cb, opaque, QEMU_AIO_DISCARD); } -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 = { @@ -1212,7 +1210,7 @@ static BlockDriver bdrv_file = { .bdrv_get_allocated_file_size = raw_get_allocated_file_size, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, }; /***********************************************/ @@ -1498,20 +1496,15 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); } -static int hdev_create(const char *filename, QEMUOptionParameter *options) +static int hdev_create(const char *filename, QemuOpts *opts) { int fd; int ret = 0; struct stat stat_buf; int64_t total_size = 0; - /* 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) @@ -1539,7 +1532,7 @@ static BlockDriver bdrv_host_device = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, @@ -1663,7 +1656,7 @@ static BlockDriver bdrv_host_floppy = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, @@ -1764,7 +1757,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, .bdrv_create = hdev_create, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, .bdrv_aio_readv = raw_aio_readv, .bdrv_aio_writev = raw_aio_writev, diff --git a/block/raw-win32.c b/block/raw-win32.c index 9b5b2af..4a6afbd 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -420,18 +420,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) return st.st_size; } -static int raw_create(const char *filename, QEMUOptionParameter *options) +static int raw_create(const char *filename, QemuOpts *opts) { int fd; int64_t total_size = 0; /* 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); @@ -443,13 +440,17 @@ 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 = { @@ -470,7 +471,7 @@ static BlockDriver bdrv_file = { .bdrv_get_allocated_file_size = raw_get_allocated_file_size, - .create_options = raw_create_options, + .bdrv_create_opts = &raw_create_opts, }; /***********************************************/ diff --git a/block/raw.c b/block/raw.c index ce10422..f526ab8 100644 --- a/block/raw.c +++ b/block/raw.c @@ -95,18 +95,22 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque); } -static int raw_create(const char *filename, QEMUOptionParameter *options) -{ - return bdrv_create_file(filename, options); -} - -static QEMUOptionParameter raw_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } +static int raw_create(const char *filename, QemuOpts *opts) +{ + return bdrv_create_file(filename, opts); +} + +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_has_zero_init(BlockDriverState *bs) @@ -143,8 +147,8 @@ static BlockDriver bdrv_raw = { .bdrv_aio_ioctl = raw_aio_ioctl, .bdrv_create = raw_create, - .create_options = raw_create_options, .bdrv_has_zero_init = raw_has_zero_init, + .bdrv_create_opts = &raw_create_opts, }; static void bdrv_raw_init(void) diff --git a/block/rbd.c b/block/rbd.c index cb71751..f0b949b 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -288,7 +288,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) +static int qemu_rbd_create(const char *filename, QemuOpts *opts) { int64_t bytes = 0; int64_t objsize; @@ -311,24 +311,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); @@ -976,20 +970,24 @@ 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", + .def_value_str = stringify(0), + }, + { /* end of list */ } + } }; - static BlockDriver bdrv_rbd = { .format_name = "rbd", .instance_size = sizeof(BDRVRBDState), @@ -998,7 +996,7 @@ static BlockDriver bdrv_rbd = { .bdrv_create = qemu_rbd_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_get_info = qemu_rbd_getinfo, - .create_options = qemu_rbd_create_options, + .bdrv_create_opts = &qemu_rbd_create_opts, .bdrv_getlength = qemu_rbd_getlength, .bdrv_truncate = qemu_rbd_truncate, .protocol_name = "rbd", diff --git a/block/sheepdog.c b/block/sheepdog.c index 6a41ad9..fcf5ff6 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1454,12 +1454,12 @@ out: return ret; } -static int sd_create(const char *filename, QEMUOptionParameter *options) +static int sd_create(const char *filename, QemuOpts *opts) { int ret = 0; uint32_t vid = 0, base_vid = 0; int64_t vdi_size = 0; - char *backing_file = NULL; + char *backing_file = NULL, *buf = NULL; BDRVSheepdogState *s; char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; uint32_t snapid; @@ -1478,26 +1478,18 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) goto out; } - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - vdi_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_PREALLOC)) { - if (!options->value.s || !strcmp(options->value.s, "off")) { - prealloc = false; - } else if (!strcmp(options->value.s, "full")) { - prealloc = true; - } else { - error_report("Invalid preallocation mode: '%s'", - options->value.s); - ret = -EINVAL; - goto out; - } - } - options++; + vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); + if (!buf || !strcmp(buf, "off")) { + prealloc = false; + } else if (!strcmp(buf, "full")) { + prealloc = true; + } else { + error_report("Invalid preallocation mode: '%s'", buf); + ret = -EINVAL; + goto out; } - if (vdi_size > SD_MAX_VDI_SIZE) { error_report("too big image size"); ret = -EINVAL; @@ -1542,6 +1534,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) ret = sd_prealloc(filename); out: + g_free(backing_file); + g_free(buf); g_free(s); return ret; } @@ -2259,7 +2253,6 @@ static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data, return do_load_save_vmstate(s, data, pos, size, 1); } - static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) { @@ -2321,23 +2314,27 @@ sd_co_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, return ret; } -static QEMUOptionParameter sd_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_PREALLOC, - .type = OPT_STRING, - .help = "Preallocation mode (allowed values: off, full)" - }, - { NULL } +static QemuOptsList sd_create_opts = { + .name = "sheepdog-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(sd_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_PREALLOC, + .type = QEMU_OPT_STRING, + .help = "Preallocation mode (allowed values: off, full)" + }, + { /* end of list */ } + } }; static BlockDriver bdrv_sheepdog = { @@ -2364,7 +2361,7 @@ static BlockDriver bdrv_sheepdog = { .bdrv_save_vmstate = sd_save_vmstate, .bdrv_load_vmstate = sd_load_vmstate, - .create_options = sd_create_options, + .bdrv_create_opts = &sd_create_opts, }; static BlockDriver bdrv_sheepdog_tcp = { @@ -2391,7 +2388,7 @@ static BlockDriver bdrv_sheepdog_tcp = { .bdrv_save_vmstate = sd_save_vmstate, .bdrv_load_vmstate = sd_load_vmstate, - .create_options = sd_create_options, + .bdrv_create_opts = &sd_create_opts, }; static BlockDriver bdrv_sheepdog_unix = { @@ -2419,7 +2416,7 @@ static BlockDriver bdrv_sheepdog_unix = { .bdrv_save_vmstate = sd_save_vmstate, .bdrv_load_vmstate = sd_load_vmstate, - .create_options = sd_create_options, + .bdrv_create_opts = &sd_create_opts, }; static void bdrv_sheepdog_init(void) diff --git a/block/ssh.c b/block/ssh.c index d7e7bf8..7b46d0e 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -641,16 +641,20 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags) return ret; } -static QEMUOptionParameter ssh_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { NULL } +static QemuOptsList ssh_create_opts = { + .name = "ssh-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(ssh_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, + { /* end of list */ } + } }; -static int ssh_create(const char *filename, QEMUOptionParameter *options) +static int ssh_create(const char *filename, QemuOpts *opts) { int r, ret; Error *local_err = NULL; @@ -663,12 +667,7 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options) ssh_state_init(&s); /* Get desired file size. */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n; - } - options++; - } + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); DPRINTF("total_size=%" PRIi64, total_size); uri_options = qdict_new(); @@ -1057,7 +1056,7 @@ static BlockDriver bdrv_ssh = { .bdrv_co_writev = ssh_co_writev, .bdrv_getlength = ssh_getlength, .bdrv_co_flush_to_disk = ssh_co_flush, - .create_options = ssh_create_options, + .bdrv_create_opts = &ssh_create_opts, }; static void bdrv_ssh_init(void) diff --git a/block/vdi.c b/block/vdi.c index 8a91525..15bd904 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -633,7 +633,7 @@ static int vdi_co_write(BlockDriverState *bs, return ret; } -static int vdi_create(const char *filename, QEMUOptionParameter *options) +static int vdi_create(const char *filename, QemuOpts *opts) { int fd; int result = 0; @@ -648,25 +648,18 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) logout("\n"); /* Read out options. */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - bytes = options->value.n; + bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); #if defined(CONFIG_VDI_BLOCK_SIZE) - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { - if (options->value.n) { - /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ - block_size = options->value.n; - } + /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ + block_size = qemu_opt_get_size_del(opts, + BLOCK_OPT_CLUSTER_SIZE, + DEFAULT_CLUSTER_SIZE); #endif #if defined(CONFIG_VDI_STATIC_IMAGE) - } else if (!strcmp(options->name, BLOCK_OPT_STATIC)) { - if (options->value.n) { - image_type = VDI_TYPE_STATIC; - } -#endif - } - options++; + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) { + image_type = VDI_TYPE_STATIC; } +#endif fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, @@ -746,29 +739,34 @@ static void vdi_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static QEMUOptionParameter vdi_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, +static QemuOptsList vdi_create_opts = { + .name = "vdi-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, #if defined(CONFIG_VDI_BLOCK_SIZE) - { - .name = BLOCK_OPT_CLUSTER_SIZE, - .type = OPT_SIZE, - .help = "VDI cluster (block) size", - .value = { .n = DEFAULT_CLUSTER_SIZE }, - }, + { + .name = BLOCK_OPT_CLUSTER_SIZE, + .type = QEMU_OPT_SIZE, + .help = "VDI cluster (block) size", + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) + }, #endif #if defined(CONFIG_VDI_STATIC_IMAGE) - { - .name = BLOCK_OPT_STATIC, - .type = OPT_FLAG, - .help = "VDI static (pre-allocated) image" - }, + { + .name = BLOCK_OPT_STATIC, + .type = QEMU_OPT_BOOL, + .help = "VDI static (pre-allocated) image", + .def_value_str = "off" + }, #endif - /* TODO: An additional option to set UUID values might be useful. */ - { NULL } + /* TODO: An additional option to set UUID values might be useful. */ + { /* end of list */ } + } }; static BlockDriver bdrv_vdi = { @@ -790,7 +788,7 @@ static BlockDriver bdrv_vdi = { .bdrv_get_info = vdi_get_info, - .create_options = vdi_create_options, + .bdrv_create_opts = &vdi_create_opts, .bdrv_check = vdi_check, }; diff --git a/block/vmdk.c b/block/vmdk.c index 3756333..e00994a 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1487,13 +1487,13 @@ static int filename_decompose(const char *filename, char *path, char *prefix, return VMDK_OK; } -static int vmdk_create(const char *filename, QEMUOptionParameter *options) +static int vmdk_create(const char *filename, QemuOpts *opts) { int fd, idx = 0; char desc[BUF_SIZE]; int64_t total_size = 0, filesize; - const char *adapter_type = NULL; - const char *backing_file = NULL; + char *adapter_type = NULL; + char *backing_file = NULL; const char *fmt = NULL; int flags = 0; int ret = 0; @@ -1527,33 +1527,29 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) "ddb.adapterType = \"%s\"\n"; if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { - return -EINVAL; + ret = -EINVAL; + goto finish; } - /* Read out options */ - while (options && options->name) { - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { - total_size = options->value.n; - } else if (!strcmp(options->name, BLOCK_OPT_ADAPTER_TYPE)) { - adapter_type = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { - backing_file = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) { - flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0; - } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) { - fmt = options->value.s; - } else if (!strcmp(options->name, BLOCK_OPT_ZEROED_GRAIN)) { - zeroed_grain |= options->value.n; - } - options++; + /* Read out opts */ + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE); + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) { + flags |= BLOCK_FLAG_COMPAT6; + } + fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) { + zeroed_grain = true; } if (!adapter_type) { - adapter_type = "ide"; + adapter_type = g_strdup("ide"); } else if (strcmp(adapter_type, "ide") && strcmp(adapter_type, "buslogic") && strcmp(adapter_type, "lsilogic") && strcmp(adapter_type, "legacyESX")) { fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type); - return -EINVAL; + ret = -EINVAL; + goto finish; } if (strcmp(adapter_type, "ide") != 0) { /* that's the number of heads with which vmware operates when @@ -1569,7 +1565,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) strcmp(fmt, "twoGbMaxExtentFlat") && strcmp(fmt, "streamOptimized")) { fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt); - return -EINVAL; + ret = -EINVAL; + goto finish; } split = !(strcmp(fmt, "twoGbMaxExtentFlat") && strcmp(fmt, "twoGbMaxExtentSparse")); @@ -1583,18 +1580,20 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) } if (flat && backing_file) { /* not supporting backing file for flat image */ - return -ENOTSUP; + ret = -ENOTSUP; + goto finish; } if (backing_file) { BlockDriverState *bs = bdrv_new(""); ret = bdrv_open(bs, backing_file, NULL, 0, NULL); if (ret != 0) { bdrv_delete(bs); - return ret; + goto finish; } if (strcmp(bs->drv->format_name, "vmdk")) { bdrv_delete(bs); - return -EINVAL; + ret = -EINVAL; + goto finish; } parent_cid = vmdk_read_cid(bs, 0); bdrv_delete(bs); @@ -1657,7 +1656,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) 0644); } if (fd < 0) { - return -errno; + ret = -errno; + goto finish; } /* the descriptor offset = 0x200 */ if (!split && !flat && 0x200 != lseek(fd, 0x200, SEEK_SET)) { @@ -1672,6 +1672,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) ret = 0; exit: qemu_close(fd); +finish: + g_free(adapter_type); + g_free(backing_file); return ret; } @@ -1724,77 +1727,65 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs) return ret; } -static int vmdk_has_zero_init(BlockDriverState *bs) -{ - int i; - BDRVVmdkState *s = bs->opaque; - - /* If has a flat extent and its underlying storage doesn't have zero init, - * return 0. */ - for (i = 0; i < s->num_extents; i++) { - if (s->extents[i].flat) { - if (!bdrv_has_zero_init(s->extents[i].file)) { - return 0; - } - } +static QemuOptsList vmdk_create_opts = { + .name = "vmdk-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_ADAPTER_TYPE, + .type = QEMU_OPT_STRING, + .help = "Virtual adapter type, can be one of " + "ide (default), lsilogic, buslogic or legacyESX" + }, + { + .name = BLOCK_OPT_BACKING_FILE, + .type = QEMU_OPT_STRING, + .help = "File name of a base image" + }, + { + .name = BLOCK_OPT_COMPAT6, + .type = QEMU_OPT_BOOL, + .help = "VMDK version 6 image", + .def_value_str = "off" + }, + { + .name = BLOCK_OPT_SUBFMT, + .type = QEMU_OPT_STRING, + .help = + "VMDK flat extent format, can be one of " + "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " + }, + { + .name = BLOCK_OPT_ZEROED_GRAIN, + .type = QEMU_OPT_BOOL, + .help = "Enable efficient zero writes " + "using the zeroed-grain GTE feature" + }, + { /* end of list */ } } - return 1; -} - -static QEMUOptionParameter vmdk_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_ADAPTER_TYPE, - .type = OPT_STRING, - .help = "Virtual adapter type, can be one of " - "ide (default), lsilogic, buslogic or legacyESX" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = OPT_STRING, - .help = "File name of a base image" - }, - { - .name = BLOCK_OPT_COMPAT6, - .type = OPT_FLAG, - .help = "VMDK version 6 image" - }, - { - .name = BLOCK_OPT_SUBFMT, - .type = OPT_STRING, - .help = - "VMDK flat extent format, can be one of " - "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " - }, - { - .name = BLOCK_OPT_ZEROED_GRAIN, - .type = OPT_FLAG, - .help = "Enable efficient zero writes using the zeroed-grain GTE feature" - }, - { NULL } }; static BlockDriver bdrv_vmdk = { - .format_name = "vmdk", - .instance_size = sizeof(BDRVVmdkState), - .bdrv_probe = vmdk_probe, - .bdrv_open = vmdk_open, - .bdrv_reopen_prepare = vmdk_reopen_prepare, - .bdrv_read = vmdk_co_read, - .bdrv_write = vmdk_co_write, - .bdrv_co_write_zeroes = vmdk_co_write_zeroes, - .bdrv_close = vmdk_close, - .bdrv_create = vmdk_create, - .bdrv_co_flush_to_disk = vmdk_co_flush, - .bdrv_co_is_allocated = vmdk_co_is_allocated, - .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, - .bdrv_has_zero_init = vmdk_has_zero_init, - - .create_options = vmdk_create_options, + .format_name = "vmdk", + .instance_size = sizeof(BDRVVmdkState), + .bdrv_probe = vmdk_probe, + .bdrv_open = vmdk_open, + .bdrv_reopen_prepare = vmdk_reopen_prepare, + .bdrv_read = vmdk_co_read, + .bdrv_write = vmdk_co_write, + .bdrv_co_write_zeroes = vmdk_co_write_zeroes, + .bdrv_close = vmdk_close, + .bdrv_create = vmdk_create, + .bdrv_co_flush_to_disk = vmdk_co_flush, + .bdrv_co_is_allocated = vmdk_co_is_allocated, + .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, + + .bdrv_create_opts = &vmdk_create_opts, }; static void bdrv_vmdk_init(void) diff --git a/block/vpc.c b/block/vpc.c index fe4f311..27ea54c 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -683,40 +683,39 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size) return ret; } -static int vpc_create(const char *filename, QEMUOptionParameter *options) +static int vpc_create(const char *filename, QemuOpts *opts) { uint8_t buf[1024]; struct vhd_footer *footer = (struct vhd_footer *) buf; - QEMUOptionParameter *disk_type_param; + char *disk_type_param = NULL; int fd, i; uint16_t cyls = 0; uint8_t heads = 0; uint8_t secs_per_cyl = 0; int64_t total_sectors; - int64_t total_size; - int disk_type; + int64_t total_size = 0; + int disk_type = VHD_DYNAMIC; int ret = -EIO; - /* Read out options */ - total_size = get_option_parameter(options, BLOCK_OPT_SIZE)->value.n; - - disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT); - if (disk_type_param && disk_type_param->value.s) { - if (!strcmp(disk_type_param->value.s, "dynamic")) { + /* Read out opts */ + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); + disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); + if (disk_type_param) { + if (!strcmp(disk_type_param, "dynamic")) { disk_type = VHD_DYNAMIC; - } else if (!strcmp(disk_type_param->value.s, "fixed")) { + } else if (!strcmp(disk_type_param, "fixed")) { disk_type = VHD_FIXED; } else { - return -EINVAL; + ret = -EINVAL; + goto finish; } - } else { - disk_type = VHD_DYNAMIC; } /* Create the file */ fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) { - return -EIO; + ret = -EIO; + goto finish; } /* @@ -783,6 +782,8 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options) fail: qemu_close(fd); +finish: + g_free(disk_type_param); return ret; } @@ -810,20 +811,24 @@ static void vpc_close(BlockDriverState *bs) error_free(s->migration_blocker); } -static QEMUOptionParameter vpc_create_options[] = { - { - .name = BLOCK_OPT_SIZE, - .type = OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_SUBFMT, - .type = OPT_STRING, - .help = - "Type of virtual hard disk format. Supported formats are " - "{dynamic (default) | fixed} " - }, - { NULL } +static QemuOptsList vpc_create_opts = { + .name = "vpc-create-opts", + .head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head), + .desc = { + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "Virtual disk size" + }, + { + .name = BLOCK_OPT_SUBFMT, + .type = QEMU_OPT_STRING, + .help = + "Type of virtual hard disk format. Supported formats are " + "{dynamic (default) | fixed} " + }, + { /* end of list */ } + } }; static BlockDriver bdrv_vpc = { @@ -839,8 +844,8 @@ static BlockDriver bdrv_vpc = { .bdrv_read = vpc_co_read, .bdrv_write = vpc_co_write, - .create_options = vpc_create_options, .bdrv_has_zero_init = vpc_has_zero_init, + .bdrv_create_opts = &vpc_create_opts, }; static void bdrv_vpc_init(void) diff --git a/block/vvfat.c b/block/vvfat.c index 87b0279..7e5a89c 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2907,7 +2907,7 @@ static BlockDriver vvfat_write_target = { static int enable_write_target(BDRVVVFATState *s) { BlockDriver *bdrv_qcow; - QEMUOptionParameter *options; + QemuOpts *opts; int ret; int size = sector2cluster(s, s->sector_count); s->used_clusters = calloc(size, 1); @@ -2923,12 +2923,13 @@ 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:"); + opts = qemu_opts_create_nofail(bdrv_qcow->bdrv_create_opts); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); - if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0) + if (bdrv_create(bdrv_qcow, s->qcow_filename, opts) < 0) { return -1; + } s->qcow = bdrv_new(""); if (s->qcow == NULL) { diff --git a/include/block/block.h b/include/block/block.h index b6b9014..955b1cb 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -116,9 +116,8 @@ BlockDriver *bdrv_find_protocol(const char *filename, 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); -int bdrv_create_file(const char* filename, QEMUOptionParameter *options); +int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts); +int bdrv_create_file(const char *filename, QemuOpts *opts); BlockDriverState *bdrv_new(const char *device_name); void bdrv_make_anon(BlockDriverState *bs); void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); diff --git a/include/block/block_int.h b/include/block/block_int.h index c6ac871..0473656 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -104,7 +104,7 @@ struct BlockDriver { const uint8_t *buf, int nb_sectors); void (*bdrv_close)(BlockDriverState *bs); void (*bdrv_rebind)(BlockDriverState *bs); - int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); + int (*bdrv_create)(const char *filename, QemuOpts *opts); int (*bdrv_set_key)(BlockDriverState *bs, const char *key); int (*bdrv_make_empty)(BlockDriverState *bs); /* aio */ @@ -193,9 +193,7 @@ struct BlockDriver { unsigned long int req, void *buf, BlockDriverCompletionFunc *cb, void *opaque); - /* List of options for creating images, terminated by name == NULL */ - QEMUOptionParameter *create_options; - + QemuOptsList *bdrv_create_opts; /* * Returns 0 for completed check, -errno for internal errors. diff --git a/qemu-img.c b/qemu-img.c index c55ca5c..3f1b6ee 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -231,7 +231,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); @@ -246,12 +246,10 @@ 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_options = append_option_parameters(create_options, - proto_drv->create_options); - print_option_help(create_options); - free_option_parameters(create_options); + create_opts = qemu_opts_append(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); + qemu_opts_print_help(create_opts); + qemu_opts_free(create_opts); return 0; } @@ -303,19 +301,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 *list, 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(list, 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(list, BLOCK_OPT_BACKING_FMT, base_fmt)) { error_report("Backing file format not supported for file " "format '%s'", fmt); return -1; @@ -1123,8 +1121,9 @@ static int img_convert(int argc, char **argv) uint8_t * buf = NULL; 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; float local_progress = 0; @@ -1268,40 +1267,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(drv->bdrv_create_opts, + proto_drv->bdrv_create_opts); if (options) { - param = parse_option_parameters(options, create_options, param); - if (param == NULL) { + if (qemu_opts_do_parse_replace(opts, options, NULL) != 0) { 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_nofail(create_opts); } - - 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"); @@ -1309,15 +1304,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"); @@ -1327,7 +1322,7 @@ static int img_convert(int argc, char **argv) } /* Create the new image */ - ret = bdrv_create(drv, out_filename, param); + ret = bdrv_create(drv, out_filename, opts); if (ret < 0) { if (ret == -ENOTSUP) { error_report("Formatting not supported for file format '%s'", @@ -1531,8 +1526,10 @@ static int img_convert(int argc, char **argv) } out: qemu_progress_end(); - free_option_parameters(create_options); - free_option_parameters(param); + qemu_opts_free(create_opts); + if (opts) { + qemu_opts_del(opts); + } qemu_vfree(buf); if (out_bs) { bdrv_delete(out_bs); -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer Dong Xu Wang @ 2013-07-17 9:48 ` Dong Xu Wang 0 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:48 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, wdongxu, Stefan Hajnoczi, Dong Xu Wang On Wed, Jul 17, 2013 at 5:29 PM, Dong Xu Wang <wdongxu@linux.vnet.ibm.com> wrote: > This patch uses QemuOpts related functions in block layer, add > a member bdrv_create_opts to BlockDriver struct, it returns > a QemuOptsList pointer, which includes the image format's create > options. > > And create options' primary consumer is block creating related > functions, so modify them together. > > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > --- > v16->v17: > 1) remove const based on Patch 4/7 > 2) s/0/false/ in qemu_opt_get_bool_del. > 3) change qed.c's logic based on Eric's comments. > 4) use qemu_ prefix in rbd.c. > 5) fix some alignments. > > v13->v14: > 1) add block/ssh.c support. > 2) fix memory leak after using qemu_opt_get_del. > > v12->v13: > 1) split into 2 patches. one for block, one for QemuOpts funcitons. > 2) fix leaking cco->opts. > 3) other small fix. > > v11->v12: > 1) create functions, such as qemu_opt_get_del and qemu_opt_replace_set. > These functions works like origin code. > 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER. > 3) in bdrv_create, if opts is NULL, will create an empty one, so can > discard if(opts) code safely. > > v10->v11: > 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or > qemu_opts_print produce un-expanded cluster_size. > 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL -> > opts, > or while using protocol, there will be an error. > > v8->v9: > 1) add qemu_ prefix to gluster_create_opts. > 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be > converted. > > v7->v8: > 1) rebase to upstream source tree. > 2) add gluster.c, raw-win32.c, and rbd.c. > > v6->v7: > 1) use osdep.h:stringify(), not redefining new macro. > 2) preserve TODO comment. > 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC. > 4) initialize disk_type even when opts is NULL. > > v5->v6: > 1) judge if opts == NULL in block layer create functions. > 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create > funtion. > 3) made more readable while using qemu_opt_get_number. > block.c | 100 +++++++++++----------- > block/cow.c | 54 ++++++------ > block/gluster.c | 37 ++++----- > block/iscsi.c | 31 ++++--- > block/qcow.c | 69 ++++++++-------- > block/qcow2.c | 205 +++++++++++++++++++++++++--------------------- > block/qed.c | 114 ++++++++++++++------------ > block/qed.h | 2 +- > block/raw-posix.c | 49 +++++------ > block/raw-win32.c | 31 +++---- > block/raw.c | 30 ++++--- > block/rbd.c | 62 +++++++------- > block/sheepdog.c | 81 +++++++++--------- > block/ssh.c | 29 ++++--- > block/vdi.c | 70 ++++++++-------- > block/vmdk.c | 179 +++++++++++++++++++--------------------- > block/vpc.c | 65 ++++++++------- > block/vvfat.c | 11 +-- > include/block/block.h | 5 +- > include/block/block_int.h | 6 +- > qemu-img.c | 65 +++++++-------- > 21 files changed, 647 insertions(+), 648 deletions(-) > > diff --git a/block.c b/block.c > index b560241..e1d1c3d 100644 > --- a/block.c > +++ b/block.c > @@ -365,7 +365,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name, > typedef struct CreateCo { > BlockDriver *drv; > char *filename; > - QEMUOptionParameter *options; > + QemuOpts *opts; > int ret; > } CreateCo; > > @@ -374,11 +374,10 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) > CreateCo *cco = opaque; > assert(cco->drv); > > - cco->ret = cco->drv->bdrv_create(cco->filename, cco->options); > + cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts); > } > > -int bdrv_create(BlockDriver *drv, const char* filename, > - QEMUOptionParameter *options) > +int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts) > { > int ret; > > @@ -386,7 +385,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, > CreateCo cco = { > .drv = drv, > .filename = g_strdup(filename), > - .options = options, > + .opts = opts ?: qemu_opts_create_nofail(drv->bdrv_create_opts), > .ret = NOT_DONE, > }; > > @@ -409,11 +408,14 @@ int bdrv_create(BlockDriver *drv, const char* filename, > ret = cco.ret; > > out: > + if (!opts) { > + qemu_opts_del(cco.opts); > + } > g_free(cco.filename); > return ret; > } > > -int bdrv_create_file(const char* filename, QEMUOptionParameter *options) > +int bdrv_create_file(const char *filename, QemuOpts *opts) > { > BlockDriver *drv; > > @@ -422,7 +424,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) > return -ENOENT; > } > > - return bdrv_create(drv, filename, options); > + return bdrv_create(drv, filename, opts); > } > > /* > @@ -984,7 +986,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, > BlockDriverState *bs1; > int64_t total_size; > BlockDriver *bdrv_qcow2; > - QEMUOptionParameter *create_options; > + QemuOpts *opts; > char backing_filename[PATH_MAX]; > > if (qdict_size(options) != 0) { > @@ -1023,19 +1025,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, > } > > bdrv_qcow2 = bdrv_find_format("qcow2"); > - create_options = parse_option_parameters("", bdrv_qcow2->create_options, > - NULL); > + opts = qemu_opts_create_nofail(bdrv_qcow2->bdrv_create_opts); > > - set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size); > - set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE, > - backing_filename); > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, backing_filename); > if (drv) { > - set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT, > - drv->format_name); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, drv->format_name); > } > > - ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options); > - free_option_parameters(create_options); > + ret = bdrv_create(bdrv_qcow2, tmp_filename, opts); > + qemu_opts_del(opts); > if (ret < 0) { > goto fail; > } > @@ -4451,8 +4450,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; > + QemuOpts *opts = NULL; > + QemuOptsList *create_opts = NULL; > + const char *backing_fmt, *backing_file; > + int64_t size; > BlockDriverState *bs = NULL; > BlockDriver *drv, *proto_drv; > BlockDriver *backing_drv = NULL; > @@ -4470,28 +4471,23 @@ void bdrv_img_create(const char *filename, const char *fmt, > error_setg(errp, "Unknown protocol '%s'", filename); > 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(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > /* Create parameter list with default values */ > - param = parse_option_parameters("", create_options, param); > + opts = qemu_opts_create_nofail(create_opts); > > - set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size); > + 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) { > + if (qemu_opts_do_parse_replace(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, > + if (qemu_opt_replace_set(opts, BLOCK_OPT_BACKING_FILE, > base_filename)) { > error_setg(errp, "Backing file not supported for file format '%s'", > fmt); > @@ -4500,39 +4496,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_replace_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); > + error_setg(errp, "Unknown backing file format '%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) { > uint64_t size; > - char buf[32]; > int back_flags; > > /* backing files always opened read-only */ > @@ -4541,18 +4535,16 @@ void bdrv_img_create(const char *filename, const char *fmt, > > bs = bdrv_new(""); > > - ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags, > - backing_drv); > + ret = bdrv_open(bs, backing_file, NULL, back_flags, backing_drv); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not open '%s'", > - backing_file->value.s); > + backing_file); > goto out; > } > bdrv_get_geometry(bs, &size); > size *= 512; > > - snprintf(buf, sizeof(buf), "%" PRId64, size); > - set_option_parameter(param, BLOCK_OPT_SIZE, buf); > + qemu_opt_replace_set_number(opts, BLOCK_OPT_SIZE, size); > } else { > error_setg(errp, "Image creation needs a size parameter"); > goto out; > @@ -4561,17 +4553,17 @@ 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); > + ret = bdrv_create(drv, filename, opts); > if (ret < 0) { > if (ret == -ENOTSUP) { > error_setg(errp,"Formatting or formatting option not supported for " > "file format '%s'", fmt); > } else if (ret == -EFBIG) { > 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'%s", > @@ -4583,8 +4575,10 @@ void bdrv_img_create(const char *filename, const char *fmt, > } > > out: > - free_option_parameters(create_options); > - free_option_parameters(param); > + if (opts) { > + qemu_opts_del(opts); > + } > + qemu_opts_free(create_opts); > > if (bs) { > bdrv_delete(bs); > diff --git a/block/cow.c b/block/cow.c > index 1cc2e89..9d4da10 100644 > --- a/block/cow.c > +++ b/block/cow.c > @@ -255,33 +255,27 @@ static void cow_close(BlockDriverState *bs) > { > } > > -static int cow_create(const char *filename, QEMUOptionParameter *options) > +static int cow_create(const char *filename, QemuOpts *opts) > { > struct cow_header_v2 cow_header; > struct stat st; > int64_t image_sectors = 0; > - const char *image_filename = NULL; > + char *image_filename = 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++; > - } > + /* Read out opts */ > + 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); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > - return ret; > + goto finish; > } > > ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR); > if (ret < 0) { > - return ret; > + goto finish; > } > > memset(&cow_header, 0, sizeof(cow_header)); > @@ -315,21 +309,27 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) > > exit: > bdrv_delete(cow_bs); > +finish: > + g_free(image_filename); > 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 = { > @@ -346,7 +346,7 @@ static BlockDriver bdrv_cow = { > .bdrv_write = cow_co_write, > .bdrv_co_is_allocated = cow_co_is_allocated, > > - .create_options = cow_create_options, > + .bdrv_create_opts = &cow_create_opts, > }; > > static void bdrv_cow_init(void) > diff --git a/block/gluster.c b/block/gluster.c > index 61424bc..8c52f8d 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -365,8 +365,7 @@ out: > return ret; > } > > -static int qemu_gluster_create(const char *filename, > - QEMUOptionParameter *options) > +static int qemu_gluster_create(const char *filename, QemuOpts *opts) > { > struct glfs *glfs; > struct glfs_fd *fd; > @@ -380,12 +379,8 @@ 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; > - } > - options++; > - } > + total_size = > + qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE; > > fd = glfs_creat(glfs, gconf->image, > O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR); > @@ -580,13 +575,17 @@ 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" > - }, > - { 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" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_gluster = { > @@ -602,7 +601,7 @@ static BlockDriver bdrv_gluster = { > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > .bdrv_has_zero_init = qemu_gluster_has_zero_init, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_tcp = { > @@ -618,7 +617,7 @@ static BlockDriver bdrv_gluster_tcp = { > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > .bdrv_has_zero_init = qemu_gluster_has_zero_init, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_unix = { > @@ -634,7 +633,7 @@ static BlockDriver bdrv_gluster_unix = { > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > .bdrv_has_zero_init = qemu_gluster_has_zero_init, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_opts = &qemu_gluster_create_opts, > }; > > static BlockDriver bdrv_gluster_rdma = { > @@ -650,7 +649,7 @@ static BlockDriver bdrv_gluster_rdma = { > .bdrv_aio_writev = qemu_gluster_aio_writev, > .bdrv_aio_flush = qemu_gluster_aio_flush, > .bdrv_has_zero_init = qemu_gluster_has_zero_init, > - .create_options = qemu_gluster_create_options, > + .bdrv_create_opts = &qemu_gluster_create_opts, > }; > > static void bdrv_gluster_init(void) > diff --git a/block/iscsi.c b/block/iscsi.c > index 0bbf0b1..e4770fb 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1196,7 +1196,7 @@ static int iscsi_has_zero_init(BlockDriverState *bs) > return 0; > } > > -static int iscsi_create(const char *filename, QEMUOptionParameter *options) > +static int iscsi_create(const char *filename, QemuOpts *opts) > { > int ret = 0; > int64_t total_size = 0; > @@ -1207,13 +1207,8 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options) > memset(&bs, 0, sizeof(BlockDriverState)); > > /* 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; > > @@ -1246,13 +1241,17 @@ out: > return ret; > } > > -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 = { > @@ -1263,7 +1262,7 @@ static BlockDriver bdrv_iscsi = { > .bdrv_file_open = iscsi_open, > .bdrv_close = iscsi_close, > .bdrv_create = iscsi_create, > - .create_options = iscsi_create_options, > + .bdrv_create_opts = &iscsi_create_opts, > > .bdrv_getlength = iscsi_getlength, > .bdrv_truncate = iscsi_truncate, > diff --git a/block/qcow.c b/block/qcow.c > index 5239bd6..a00f4cc 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -651,37 +651,32 @@ static void qcow_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static int qcow_create(const char *filename, QEMUOptionParameter *options) > +static int qcow_create(const char *filename, QemuOpts *opts) > { > 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; > 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++; > + /* Read out opts */ > + 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); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > - return ret; > + goto finish; > } > > ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR); > if (ret < 0) { > - return ret; > + goto finish; > } > > ret = bdrv_truncate(qcow_bs, 0); > @@ -752,6 +747,8 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) > ret = 0; > exit: > bdrv_delete(qcow_bs); > +finish: > + g_free(backing_file); > return ret; > } > > @@ -864,24 +861,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 = { > @@ -903,7 +904,7 @@ static BlockDriver bdrv_qcow = { > .bdrv_write_compressed = qcow_write_compressed, > .bdrv_get_info = qcow_get_info, > > - .create_options = qcow_create_options, > + .bdrv_create_opts = &qcow_create_opts, > }; > > static void bdrv_qcow_init(void) > diff --git a/block/qcow2.c b/block/qcow2.c > index 0eceefe..d0b8540 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1258,12 +1258,13 @@ static int preallocate(BlockDriverState *bs) > } > > static int qcow2_create2(const char *filename, int64_t total_size, > - const char *backing_file, const char *backing_format, > + char *backing_file, char *backing_format, > int flags, size_t cluster_size, int prealloc, > - QEMUOptionParameter *options, int version) > + QemuOpts *opts, int version) > { > /* Calculate cluster_bits */ > int cluster_bits; > + int ret = 0; > cluster_bits = ffs(cluster_size) - 1; > if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS || > (1 << cluster_bits) != cluster_size) > @@ -1271,7 +1272,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, > error_report( > "Cluster size must be a power of two between %d and %dk", > 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10)); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > > /* > @@ -1289,16 +1291,15 @@ static int qcow2_create2(const char *filename, int64_t total_size, > BlockDriverState* bs; > QCowHeader header; > uint8_t* refcount_table; > - int ret; > > - ret = bdrv_create_file(filename, options); > + ret = bdrv_create_file(filename, opts); > if (ret < 0) { > - return ret; > + goto finish; > } > > ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR); > if (ret < 0) { > - return ret; > + goto finish; > } > > /* Write the header */ > @@ -1391,73 +1392,82 @@ static int qcow2_create2(const char *filename, int64_t total_size, > ret = 0; > out: > bdrv_delete(bs); > +finish: > + g_free(backing_file); > + g_free(backing_format); > return ret; > } > > -static int qcow2_create(const char *filename, QEMUOptionParameter *options) > +static int qcow2_create(const char *filename, QemuOpts *opts) > { > - const char *backing_file = NULL; > - const char *backing_fmt = NULL; > + char *backing_file = NULL; > + char *backing_fmt = NULL; > uint64_t sectors = 0; > int flags = 0; > size_t cluster_size = DEFAULT_CLUSTER_SIZE; > int prealloc = 0; > int version = 2; > + char *buf; > + int ret = 0; > > /* 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 { > - fprintf(stderr, "Invalid preallocation mode: '%s'\n", > - options->value.s); > - return -EINVAL; > - } > - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) { > - if (!options->value.s || !strcmp(options->value.s, "0.10")) { > - version = 2; > - } else if (!strcmp(options->value.s, "1.1")) { > - version = 3; > - } else { > - fprintf(stderr, "Invalid compatibility level: '%s'\n", > - 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 { > + fprintf(stderr, "Invalid preallocation mode: '%s'\n", > + buf); > + ret = -EINVAL; > + goto finish; > + } > + g_free(buf); > + buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL); > + if (!buf || !strcmp(buf, "0.10")) { > + version = 2; > + } else if (!strcmp(buf, "1.1")) { > + version = 3; > + } else { > + fprintf(stderr, "Invalid compatibility level: '%s'\n", > + 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) { > fprintf(stderr, "Backing file and preallocation cannot be used at " > "the same time\n"); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > > if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) { > fprintf(stderr, "Lazy refcounts only supported with compatibility " > "level 1.1 and above (use compat=1.1 or greater)\n"); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > > return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags, > - cluster_size, prealloc, options, version); > + cluster_size, prealloc, opts, version); > +finish: > + g_free(backing_file); > + g_free(backing_fmt); > + g_free(buf); > + return ret; > } > > static int qcow2_make_empty(BlockDriverState *bs) > @@ -1732,49 +1742,55 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf, > return ret; > } > > -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 = { > @@ -1813,8 +1829,9 @@ static BlockDriver bdrv_qcow2 = { > > .bdrv_invalidate_cache = qcow2_invalidate_cache, > > - .create_options = qcow2_create_options, > .bdrv_check = qcow2_check, > + > + .bdrv_create_opts = &qcow2_create_opts, > }; > > static void bdrv_qcow2_init(void) > diff --git a/block/qed.c b/block/qed.c > index f767b05..6514370 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -555,12 +555,12 @@ static int qed_create(const char *filename, uint32_t cluster_size, > > ret = bdrv_create_file(filename, NULL); > if (ret < 0) { > - return ret; > + goto finish; > } > > ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB); > if (ret < 0) { > - return ret; > + goto finish; > } > > /* File must start empty and grow, check truncate is supported */ > @@ -600,55 +600,55 @@ static int qed_create(const char *filename, uint32_t cluster_size, > out: > g_free(l1_table); > bdrv_delete(bs); > +finish: > return ret; > } > > -static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options) > +static int bdrv_qed_create(const char *filename, QemuOpts *opts) > { > 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 = 0; > + > + 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)) { > fprintf(stderr, "QED cluster size must be within range [%u, %u] and power of 2\n", > QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > if (!qed_is_table_size_valid(table_size)) { > fprintf(stderr, "QED table size must be within range [%u, %u] and power of 2\n", > 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)) { > fprintf(stderr, "QED image size must be a non-zero multiple of " > "cluster size and less than %" PRIu64 " bytes\n", > qed_max_image_size(cluster_size, table_size)); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > > - return qed_create(filename, cluster_size, image_size, table_size, > + ret = qed_create(filename, cluster_size, image_size, table_size, > backing_file, backing_fmt); > + > +finish: > + g_free(backing_file); > + g_free(backing_fmt); > + return ret; > } > > typedef struct { > @@ -1537,36 +1537,44 @@ 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, > + .bdrv_create_opts = &qed_create_opts, > > .bdrv_probe = bdrv_qed_probe, > .bdrv_rebind = bdrv_qed_rebind, > diff --git a/block/qed.h b/block/qed.h > index 2b4dded..99a7726 100644 > --- a/block/qed.h > +++ b/block/qed.h > @@ -43,6 +43,7 @@ > * > * All fields are little-endian on disk. > */ > +#define QED_DEFAULT_CLUSTER_SIZE 65536 > Eric, when I use enum, QED_DEFAULT_CLUSTER_SIZE can not be expanded, so I use macro. [VM:~/qemu]$ qemu-img create -f qed t.qed 10M Formatting 't.qed', fmt=qed size=10485760 cluster_size=QED_DEFAULT_CLUSTER_SIZE > enum { > QED_MAGIC = 'Q' | 'E' << 8 | 'D' << 16 | '\0' << 24, > @@ -69,7 +70,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 > diff --git a/block/raw-posix.c b/block/raw-posix.c > index ba721d3..b47e688 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -1040,20 +1040,14 @@ 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) > +static int raw_create(const char *filename, QemuOpts *opts) > { > int fd; > int result = 0; > int64_t total_size = 0; > > - /* 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); > if (fd < 0) { > @@ -1179,13 +1173,17 @@ static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs, > cb, opaque, QEMU_AIO_DISCARD); > } > > -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 = { > @@ -1212,7 +1210,7 @@ static BlockDriver bdrv_file = { > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > }; > > /***********************************************/ > @@ -1498,20 +1496,15 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, > cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); > } > > -static int hdev_create(const char *filename, QEMUOptionParameter *options) > +static int hdev_create(const char *filename, QemuOpts *opts) > { > int fd; > int ret = 0; > struct stat stat_buf; > int64_t total_size = 0; > > - /* 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) > @@ -1539,7 +1532,7 @@ static BlockDriver bdrv_host_device = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev = raw_aio_writev, > @@ -1663,7 +1656,7 @@ static BlockDriver bdrv_host_floppy = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev = raw_aio_writev, > @@ -1764,7 +1757,7 @@ static BlockDriver bdrv_host_cdrom = { > .bdrv_reopen_commit = raw_reopen_commit, > .bdrv_reopen_abort = raw_reopen_abort, > .bdrv_create = hdev_create, > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev = raw_aio_writev, > diff --git a/block/raw-win32.c b/block/raw-win32.c > index 9b5b2af..4a6afbd 100644 > --- a/block/raw-win32.c > +++ b/block/raw-win32.c > @@ -420,18 +420,15 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) > return st.st_size; > } > > -static int raw_create(const char *filename, QEMUOptionParameter *options) > +static int raw_create(const char *filename, QemuOpts *opts) > { > int fd; > int64_t total_size = 0; > > /* 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); > @@ -443,13 +440,17 @@ 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 = { > @@ -470,7 +471,7 @@ static BlockDriver bdrv_file = { > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > - .create_options = raw_create_options, > + .bdrv_create_opts = &raw_create_opts, > }; > > /***********************************************/ > diff --git a/block/raw.c b/block/raw.c > index ce10422..f526ab8 100644 > --- a/block/raw.c > +++ b/block/raw.c > @@ -95,18 +95,22 @@ static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, > return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque); > } > > -static int raw_create(const char *filename, QEMUOptionParameter *options) > -{ > - return bdrv_create_file(filename, options); > -} > - > -static QEMUOptionParameter raw_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > +static int raw_create(const char *filename, QemuOpts *opts) > +{ > + return bdrv_create_file(filename, opts); > +} > + > +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_has_zero_init(BlockDriverState *bs) > @@ -143,8 +147,8 @@ static BlockDriver bdrv_raw = { > .bdrv_aio_ioctl = raw_aio_ioctl, > > .bdrv_create = raw_create, > - .create_options = raw_create_options, > .bdrv_has_zero_init = raw_has_zero_init, > + .bdrv_create_opts = &raw_create_opts, > }; > > static void bdrv_raw_init(void) > diff --git a/block/rbd.c b/block/rbd.c > index cb71751..f0b949b 100644 > --- a/block/rbd.c > +++ b/block/rbd.c > @@ -288,7 +288,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) > +static int qemu_rbd_create(const char *filename, QemuOpts *opts) > { > int64_t bytes = 0; > int64_t objsize; > @@ -311,24 +311,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); > @@ -976,20 +970,24 @@ 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", > + .def_value_str = stringify(0), > + }, > + { /* end of list */ } > + } > }; > - > static BlockDriver bdrv_rbd = { > .format_name = "rbd", > .instance_size = sizeof(BDRVRBDState), > @@ -998,7 +996,7 @@ static BlockDriver bdrv_rbd = { > .bdrv_create = qemu_rbd_create, > .bdrv_has_zero_init = bdrv_has_zero_init_1, > .bdrv_get_info = qemu_rbd_getinfo, > - .create_options = qemu_rbd_create_options, > + .bdrv_create_opts = &qemu_rbd_create_opts, > .bdrv_getlength = qemu_rbd_getlength, > .bdrv_truncate = qemu_rbd_truncate, > .protocol_name = "rbd", > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 6a41ad9..fcf5ff6 100644 > --- a/block/sheepdog.c > +++ b/block/sheepdog.c > @@ -1454,12 +1454,12 @@ out: > return ret; > } > > -static int sd_create(const char *filename, QEMUOptionParameter *options) > +static int sd_create(const char *filename, QemuOpts *opts) > { > int ret = 0; > uint32_t vid = 0, base_vid = 0; > int64_t vdi_size = 0; > - char *backing_file = NULL; > + char *backing_file = NULL, *buf = NULL; > BDRVSheepdogState *s; > char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; > uint32_t snapid; > @@ -1478,26 +1478,18 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) > goto out; > } > > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - vdi_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_PREALLOC)) { > - if (!options->value.s || !strcmp(options->value.s, "off")) { > - prealloc = false; > - } else if (!strcmp(options->value.s, "full")) { > - prealloc = true; > - } else { > - error_report("Invalid preallocation mode: '%s'", > - options->value.s); > - ret = -EINVAL; > - goto out; > - } > - } > - options++; > + vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); > + if (!buf || !strcmp(buf, "off")) { > + prealloc = false; > + } else if (!strcmp(buf, "full")) { > + prealloc = true; > + } else { > + error_report("Invalid preallocation mode: '%s'", buf); > + ret = -EINVAL; > + goto out; > } > - > if (vdi_size > SD_MAX_VDI_SIZE) { > error_report("too big image size"); > ret = -EINVAL; > @@ -1542,6 +1534,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) > > ret = sd_prealloc(filename); > out: > + g_free(backing_file); > + g_free(buf); > g_free(s); > return ret; > } > @@ -2259,7 +2253,6 @@ static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data, > return do_load_save_vmstate(s, data, pos, size, 1); > } > > - > static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num, > int nb_sectors) > { > @@ -2321,23 +2314,27 @@ sd_co_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, > return ret; > } > > -static QEMUOptionParameter sd_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_PREALLOC, > - .type = OPT_STRING, > - .help = "Preallocation mode (allowed values: off, full)" > - }, > - { NULL } > +static QemuOptsList sd_create_opts = { > + .name = "sheepdog-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(sd_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_PREALLOC, > + .type = QEMU_OPT_STRING, > + .help = "Preallocation mode (allowed values: off, full)" > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_sheepdog = { > @@ -2364,7 +2361,7 @@ static BlockDriver bdrv_sheepdog = { > .bdrv_save_vmstate = sd_save_vmstate, > .bdrv_load_vmstate = sd_load_vmstate, > > - .create_options = sd_create_options, > + .bdrv_create_opts = &sd_create_opts, > }; > > static BlockDriver bdrv_sheepdog_tcp = { > @@ -2391,7 +2388,7 @@ static BlockDriver bdrv_sheepdog_tcp = { > .bdrv_save_vmstate = sd_save_vmstate, > .bdrv_load_vmstate = sd_load_vmstate, > > - .create_options = sd_create_options, > + .bdrv_create_opts = &sd_create_opts, > }; > > static BlockDriver bdrv_sheepdog_unix = { > @@ -2419,7 +2416,7 @@ static BlockDriver bdrv_sheepdog_unix = { > .bdrv_save_vmstate = sd_save_vmstate, > .bdrv_load_vmstate = sd_load_vmstate, > > - .create_options = sd_create_options, > + .bdrv_create_opts = &sd_create_opts, > }; > > static void bdrv_sheepdog_init(void) > diff --git a/block/ssh.c b/block/ssh.c > index d7e7bf8..7b46d0e 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -641,16 +641,20 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags) > return ret; > } > > -static QEMUOptionParameter ssh_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { NULL } > +static QemuOptsList ssh_create_opts = { > + .name = "ssh-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(ssh_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Virtual disk size" > + }, > + { /* end of list */ } > + } > }; > > -static int ssh_create(const char *filename, QEMUOptionParameter *options) > +static int ssh_create(const char *filename, QemuOpts *opts) > { > int r, ret; > Error *local_err = NULL; > @@ -663,12 +667,7 @@ static int ssh_create(const char *filename, QEMUOptionParameter *options) > ssh_state_init(&s); > > /* Get desired file size. */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n; > - } > - options++; > - } > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > DPRINTF("total_size=%" PRIi64, total_size); > > uri_options = qdict_new(); > @@ -1057,7 +1056,7 @@ static BlockDriver bdrv_ssh = { > .bdrv_co_writev = ssh_co_writev, > .bdrv_getlength = ssh_getlength, > .bdrv_co_flush_to_disk = ssh_co_flush, > - .create_options = ssh_create_options, > + .bdrv_create_opts = &ssh_create_opts, > }; > > static void bdrv_ssh_init(void) > diff --git a/block/vdi.c b/block/vdi.c > index 8a91525..15bd904 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -633,7 +633,7 @@ static int vdi_co_write(BlockDriverState *bs, > return ret; > } > > -static int vdi_create(const char *filename, QEMUOptionParameter *options) > +static int vdi_create(const char *filename, QemuOpts *opts) > { > int fd; > int result = 0; > @@ -648,25 +648,18 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) > logout("\n"); > > /* Read out options. */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - bytes = options->value.n; > + bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > #if defined(CONFIG_VDI_BLOCK_SIZE) > - } else if (!strcmp(options->name, BLOCK_OPT_CLUSTER_SIZE)) { > - if (options->value.n) { > - /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ > - block_size = options->value.n; > - } > + /* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */ > + block_size = qemu_opt_get_size_del(opts, > + BLOCK_OPT_CLUSTER_SIZE, > + DEFAULT_CLUSTER_SIZE); > #endif > #if defined(CONFIG_VDI_STATIC_IMAGE) > - } else if (!strcmp(options->name, BLOCK_OPT_STATIC)) { > - if (options->value.n) { > - image_type = VDI_TYPE_STATIC; > - } > -#endif > - } > - options++; > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) { > + image_type = VDI_TYPE_STATIC; > } > +#endif > > fd = qemu_open(filename, > O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, > @@ -746,29 +739,34 @@ static void vdi_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static QEMUOptionParameter vdi_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > +static QemuOptsList vdi_create_opts = { > + .name = "vdi-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Virtual disk size" > + }, > #if defined(CONFIG_VDI_BLOCK_SIZE) > - { > - .name = BLOCK_OPT_CLUSTER_SIZE, > - .type = OPT_SIZE, > - .help = "VDI cluster (block) size", > - .value = { .n = DEFAULT_CLUSTER_SIZE }, > - }, > + { > + .name = BLOCK_OPT_CLUSTER_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "VDI cluster (block) size", > + .def_value_str = stringify(DEFAULT_CLUSTER_SIZE) > + }, > #endif > #if defined(CONFIG_VDI_STATIC_IMAGE) > - { > - .name = BLOCK_OPT_STATIC, > - .type = OPT_FLAG, > - .help = "VDI static (pre-allocated) image" > - }, > + { > + .name = BLOCK_OPT_STATIC, > + .type = QEMU_OPT_BOOL, > + .help = "VDI static (pre-allocated) image", > + .def_value_str = "off" > + }, > #endif > - /* TODO: An additional option to set UUID values might be useful. */ > - { NULL } > + /* TODO: An additional option to set UUID values might be useful. */ > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vdi = { > @@ -790,7 +788,7 @@ static BlockDriver bdrv_vdi = { > > .bdrv_get_info = vdi_get_info, > > - .create_options = vdi_create_options, > + .bdrv_create_opts = &vdi_create_opts, > .bdrv_check = vdi_check, > }; > > diff --git a/block/vmdk.c b/block/vmdk.c > index 3756333..e00994a 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1487,13 +1487,13 @@ static int filename_decompose(const char *filename, char *path, char *prefix, > return VMDK_OK; > } > > -static int vmdk_create(const char *filename, QEMUOptionParameter *options) > +static int vmdk_create(const char *filename, QemuOpts *opts) > { > int fd, idx = 0; > char desc[BUF_SIZE]; > int64_t total_size = 0, filesize; > - const char *adapter_type = NULL; > - const char *backing_file = NULL; > + char *adapter_type = NULL; > + char *backing_file = NULL; > const char *fmt = NULL; > int flags = 0; > int ret = 0; > @@ -1527,33 +1527,29 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > "ddb.adapterType = \"%s\"\n"; > > if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > - /* Read out options */ > - while (options && options->name) { > - if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n; > - } else if (!strcmp(options->name, BLOCK_OPT_ADAPTER_TYPE)) { > - adapter_type = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) { > - backing_file = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) { > - flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0; > - } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) { > - fmt = options->value.s; > - } else if (!strcmp(options->name, BLOCK_OPT_ZEROED_GRAIN)) { > - zeroed_grain |= options->value.n; > - } > - options++; > + /* Read out opts */ > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE); > + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) { > + flags |= BLOCK_FLAG_COMPAT6; > + } > + fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); > + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) { > + zeroed_grain = true; > } > if (!adapter_type) { > - adapter_type = "ide"; > + adapter_type = g_strdup("ide"); > } else if (strcmp(adapter_type, "ide") && > strcmp(adapter_type, "buslogic") && > strcmp(adapter_type, "lsilogic") && > strcmp(adapter_type, "legacyESX")) { > fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > if (strcmp(adapter_type, "ide") != 0) { > /* that's the number of heads with which vmware operates when > @@ -1569,7 +1565,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > strcmp(fmt, "twoGbMaxExtentFlat") && > strcmp(fmt, "streamOptimized")) { > fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > split = !(strcmp(fmt, "twoGbMaxExtentFlat") && > strcmp(fmt, "twoGbMaxExtentSparse")); > @@ -1583,18 +1580,20 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > } > if (flat && backing_file) { > /* not supporting backing file for flat image */ > - return -ENOTSUP; > + ret = -ENOTSUP; > + goto finish; > } > if (backing_file) { > BlockDriverState *bs = bdrv_new(""); > ret = bdrv_open(bs, backing_file, NULL, 0, NULL); > if (ret != 0) { > bdrv_delete(bs); > - return ret; > + goto finish; > } > if (strcmp(bs->drv->format_name, "vmdk")) { > bdrv_delete(bs); > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > parent_cid = vmdk_read_cid(bs, 0); > bdrv_delete(bs); > @@ -1657,7 +1656,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > 0644); > } > if (fd < 0) { > - return -errno; > + ret = -errno; > + goto finish; > } > /* the descriptor offset = 0x200 */ > if (!split && !flat && 0x200 != lseek(fd, 0x200, SEEK_SET)) { > @@ -1672,6 +1672,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) > ret = 0; > exit: > qemu_close(fd); > +finish: > + g_free(adapter_type); > + g_free(backing_file); > return ret; > } > > @@ -1724,77 +1727,65 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs) > return ret; > } > > -static int vmdk_has_zero_init(BlockDriverState *bs) > -{ > - int i; > - BDRVVmdkState *s = bs->opaque; > - > - /* If has a flat extent and its underlying storage doesn't have zero init, > - * return 0. */ > - for (i = 0; i < s->num_extents; i++) { > - if (s->extents[i].flat) { > - if (!bdrv_has_zero_init(s->extents[i].file)) { > - return 0; > - } > - } > +static QemuOptsList vmdk_create_opts = { > + .name = "vmdk-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_ADAPTER_TYPE, > + .type = QEMU_OPT_STRING, > + .help = "Virtual adapter type, can be one of " > + "ide (default), lsilogic, buslogic or legacyESX" > + }, > + { > + .name = BLOCK_OPT_BACKING_FILE, > + .type = QEMU_OPT_STRING, > + .help = "File name of a base image" > + }, > + { > + .name = BLOCK_OPT_COMPAT6, > + .type = QEMU_OPT_BOOL, > + .help = "VMDK version 6 image", > + .def_value_str = "off" > + }, > + { > + .name = BLOCK_OPT_SUBFMT, > + .type = QEMU_OPT_STRING, > + .help = > + "VMDK flat extent format, can be one of " > + "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " > + }, > + { > + .name = BLOCK_OPT_ZEROED_GRAIN, > + .type = QEMU_OPT_BOOL, > + .help = "Enable efficient zero writes " > + "using the zeroed-grain GTE feature" > + }, > + { /* end of list */ } > } > - return 1; > -} > - > -static QEMUOptionParameter vmdk_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_ADAPTER_TYPE, > - .type = OPT_STRING, > - .help = "Virtual adapter type, can be one of " > - "ide (default), lsilogic, buslogic or legacyESX" > - }, > - { > - .name = BLOCK_OPT_BACKING_FILE, > - .type = OPT_STRING, > - .help = "File name of a base image" > - }, > - { > - .name = BLOCK_OPT_COMPAT6, > - .type = OPT_FLAG, > - .help = "VMDK version 6 image" > - }, > - { > - .name = BLOCK_OPT_SUBFMT, > - .type = OPT_STRING, > - .help = > - "VMDK flat extent format, can be one of " > - "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} " > - }, > - { > - .name = BLOCK_OPT_ZEROED_GRAIN, > - .type = OPT_FLAG, > - .help = "Enable efficient zero writes using the zeroed-grain GTE feature" > - }, > - { NULL } > }; > > static BlockDriver bdrv_vmdk = { > - .format_name = "vmdk", > - .instance_size = sizeof(BDRVVmdkState), > - .bdrv_probe = vmdk_probe, > - .bdrv_open = vmdk_open, > - .bdrv_reopen_prepare = vmdk_reopen_prepare, > - .bdrv_read = vmdk_co_read, > - .bdrv_write = vmdk_co_write, > - .bdrv_co_write_zeroes = vmdk_co_write_zeroes, > - .bdrv_close = vmdk_close, > - .bdrv_create = vmdk_create, > - .bdrv_co_flush_to_disk = vmdk_co_flush, > - .bdrv_co_is_allocated = vmdk_co_is_allocated, > - .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, > - .bdrv_has_zero_init = vmdk_has_zero_init, > - > - .create_options = vmdk_create_options, > + .format_name = "vmdk", > + .instance_size = sizeof(BDRVVmdkState), > + .bdrv_probe = vmdk_probe, > + .bdrv_open = vmdk_open, > + .bdrv_reopen_prepare = vmdk_reopen_prepare, > + .bdrv_read = vmdk_co_read, > + .bdrv_write = vmdk_co_write, > + .bdrv_co_write_zeroes = vmdk_co_write_zeroes, > + .bdrv_close = vmdk_close, > + .bdrv_create = vmdk_create, > + .bdrv_co_flush_to_disk = vmdk_co_flush, > + .bdrv_co_is_allocated = vmdk_co_is_allocated, > + .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size, > + > + .bdrv_create_opts = &vmdk_create_opts, > }; > > static void bdrv_vmdk_init(void) > diff --git a/block/vpc.c b/block/vpc.c > index fe4f311..27ea54c 100644 > --- a/block/vpc.c > +++ b/block/vpc.c > @@ -683,40 +683,39 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size) > return ret; > } > > -static int vpc_create(const char *filename, QEMUOptionParameter *options) > +static int vpc_create(const char *filename, QemuOpts *opts) > { > uint8_t buf[1024]; > struct vhd_footer *footer = (struct vhd_footer *) buf; > - QEMUOptionParameter *disk_type_param; > + char *disk_type_param = NULL; > int fd, i; > uint16_t cyls = 0; > uint8_t heads = 0; > uint8_t secs_per_cyl = 0; > int64_t total_sectors; > - int64_t total_size; > - int disk_type; > + int64_t total_size = 0; > + int disk_type = VHD_DYNAMIC; > int ret = -EIO; > > - /* Read out options */ > - total_size = get_option_parameter(options, BLOCK_OPT_SIZE)->value.n; > - > - disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT); > - if (disk_type_param && disk_type_param->value.s) { > - if (!strcmp(disk_type_param->value.s, "dynamic")) { > + /* Read out opts */ > + total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT); > + if (disk_type_param) { > + if (!strcmp(disk_type_param, "dynamic")) { > disk_type = VHD_DYNAMIC; > - } else if (!strcmp(disk_type_param->value.s, "fixed")) { > + } else if (!strcmp(disk_type_param, "fixed")) { > disk_type = VHD_FIXED; > } else { > - return -EINVAL; > + ret = -EINVAL; > + goto finish; > } > - } else { > - disk_type = VHD_DYNAMIC; > } > > /* Create the file */ > fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); > if (fd < 0) { > - return -EIO; > + ret = -EIO; > + goto finish; > } > > /* > @@ -783,6 +782,8 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options) > > fail: > qemu_close(fd); > +finish: > + g_free(disk_type_param); > return ret; > } > > @@ -810,20 +811,24 @@ static void vpc_close(BlockDriverState *bs) > error_free(s->migration_blocker); > } > > -static QEMUOptionParameter vpc_create_options[] = { > - { > - .name = BLOCK_OPT_SIZE, > - .type = OPT_SIZE, > - .help = "Virtual disk size" > - }, > - { > - .name = BLOCK_OPT_SUBFMT, > - .type = OPT_STRING, > - .help = > - "Type of virtual hard disk format. Supported formats are " > - "{dynamic (default) | fixed} " > - }, > - { NULL } > +static QemuOptsList vpc_create_opts = { > + .name = "vpc-create-opts", > + .head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head), > + .desc = { > + { > + .name = BLOCK_OPT_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Virtual disk size" > + }, > + { > + .name = BLOCK_OPT_SUBFMT, > + .type = QEMU_OPT_STRING, > + .help = > + "Type of virtual hard disk format. Supported formats are " > + "{dynamic (default) | fixed} " > + }, > + { /* end of list */ } > + } > }; > > static BlockDriver bdrv_vpc = { > @@ -839,8 +844,8 @@ static BlockDriver bdrv_vpc = { > .bdrv_read = vpc_co_read, > .bdrv_write = vpc_co_write, > > - .create_options = vpc_create_options, > .bdrv_has_zero_init = vpc_has_zero_init, > + .bdrv_create_opts = &vpc_create_opts, > }; > > static void bdrv_vpc_init(void) > diff --git a/block/vvfat.c b/block/vvfat.c > index 87b0279..7e5a89c 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -2907,7 +2907,7 @@ static BlockDriver vvfat_write_target = { > static int enable_write_target(BDRVVVFATState *s) > { > BlockDriver *bdrv_qcow; > - QEMUOptionParameter *options; > + QemuOpts *opts; > int ret; > int size = sector2cluster(s, s->sector_count); > s->used_clusters = calloc(size, 1); > @@ -2923,12 +2923,13 @@ 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:"); > + opts = qemu_opts_create_nofail(bdrv_qcow->bdrv_create_opts); > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); > + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); > > - if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0) > + if (bdrv_create(bdrv_qcow, s->qcow_filename, opts) < 0) { > return -1; > + } > > s->qcow = bdrv_new(""); > if (s->qcow == NULL) { > diff --git a/include/block/block.h b/include/block/block.h > index b6b9014..955b1cb 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -116,9 +116,8 @@ BlockDriver *bdrv_find_protocol(const char *filename, > 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); > -int bdrv_create_file(const char* filename, QEMUOptionParameter *options); > +int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts); > +int bdrv_create_file(const char *filename, QemuOpts *opts); > BlockDriverState *bdrv_new(const char *device_name); > void bdrv_make_anon(BlockDriverState *bs); > void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); > diff --git a/include/block/block_int.h b/include/block/block_int.h > index c6ac871..0473656 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -104,7 +104,7 @@ struct BlockDriver { > const uint8_t *buf, int nb_sectors); > void (*bdrv_close)(BlockDriverState *bs); > void (*bdrv_rebind)(BlockDriverState *bs); > - int (*bdrv_create)(const char *filename, QEMUOptionParameter *options); > + int (*bdrv_create)(const char *filename, QemuOpts *opts); > int (*bdrv_set_key)(BlockDriverState *bs, const char *key); > int (*bdrv_make_empty)(BlockDriverState *bs); > /* aio */ > @@ -193,9 +193,7 @@ struct BlockDriver { > unsigned long int req, void *buf, > BlockDriverCompletionFunc *cb, void *opaque); > > - /* List of options for creating images, terminated by name == NULL */ > - QEMUOptionParameter *create_options; > - > + QemuOptsList *bdrv_create_opts; > > /* > * Returns 0 for completed check, -errno for internal errors. > diff --git a/qemu-img.c b/qemu-img.c > index c55ca5c..3f1b6ee 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -231,7 +231,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); > @@ -246,12 +246,10 @@ 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_options = append_option_parameters(create_options, > - proto_drv->create_options); > - print_option_help(create_options); > - free_option_parameters(create_options); > + create_opts = qemu_opts_append(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > + qemu_opts_print_help(create_opts); > + qemu_opts_free(create_opts); > return 0; > } > > @@ -303,19 +301,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 *list, > 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(list, 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(list, BLOCK_OPT_BACKING_FMT, base_fmt)) { > error_report("Backing file format not supported for file " > "format '%s'", fmt); > return -1; > @@ -1123,8 +1121,9 @@ static int img_convert(int argc, char **argv) > uint8_t * buf = NULL; > 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; > float local_progress = 0; > @@ -1268,40 +1267,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(drv->bdrv_create_opts, > + proto_drv->bdrv_create_opts); > > if (options) { > - param = parse_option_parameters(options, create_options, param); > - if (param == NULL) { > + if (qemu_opts_do_parse_replace(opts, options, NULL) != 0) { > 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_nofail(create_opts); > } > - > - 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"); > @@ -1309,15 +1304,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"); > @@ -1327,7 +1322,7 @@ static int img_convert(int argc, char **argv) > } > > /* Create the new image */ > - ret = bdrv_create(drv, out_filename, param); > + ret = bdrv_create(drv, out_filename, opts); > if (ret < 0) { > if (ret == -ENOTSUP) { > error_report("Formatting not supported for file format '%s'", > @@ -1531,8 +1526,10 @@ static int img_convert(int argc, char **argv) > } > out: > qemu_progress_end(); > - free_option_parameters(create_options); > - free_option_parameters(param); > + qemu_opts_free(create_opts); > + if (opts) { > + qemu_opts_del(opts); > + } > qemu_vfree(buf); > if (out_bs) { > bdrv_delete(out_bs); > -- > 1.7.11.7 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 6/9] qapi: query-command-line-options outputs def_value_str 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (4 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 7/9] qemu-option: remove QEMUOptionParameter related functions and struct Dong Xu Wang ` (3 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang QMP command query-command-line-options shows details information of parameters, since added def_value_str, also output it in the QMP command. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- v16->v17: 1) add "Since 1.6" tag. 2) rename def_str_value to "default". qapi-schema.json | 8 ++++++-- qmp-commands.hx | 2 ++ util/qemu-config.c | 4 ++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/qapi-schema.json b/qapi-schema.json index 7b9fef1..cb9098c 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3612,12 +3612,16 @@ # # @help: #optional human readable text string, not suitable for parsing. # -# Since 1.5 +# @default: #optional string representation of the default used +# if the option is omitted. +# +# Since 1.6 ## { '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 e075df4..a88f310 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2571,6 +2571,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 a59568d..315ecbf 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -67,6 +67,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.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 7/9] qemu-option: remove QEMUOptionParameter related functions and struct 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (5 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 6/9] qapi: query-command-line-options outputs def_value_str Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL Dong Xu Wang ` (2 subsequent siblings) 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- include/qemu/option.h | 39 ------- util/qemu-option.c | 285 -------------------------------------------------- 2 files changed, 324 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index 12fa8b4..2aaa057 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -31,24 +31,6 @@ #include "qapi/error.h" #include "qapi/qmp/qdict.h" -enum QEMUOptionParType { - OPT_FLAG, - OPT_NUMBER, - OPT_SIZE, - OPT_STRING, -}; - -typedef struct QEMUOptionParameter { - const char *name; - enum QEMUOptionParType type; - union { - uint64_t n; - char* s; - } value; - const char *help; -} QEMUOptionParameter; - - const char *get_opt_name(char *buf, int buf_size, const char *p, char delim); const char *get_opt_value(char *buf, int buf_size, const char *p); int get_next_param_value(char *buf, int buf_size, @@ -56,27 +38,6 @@ int get_next_param_value(char *buf, int buf_size, int get_param_value(char *buf, int buf_size, const char *tag, const char *str); - -/* - * The following functions take a parameter list as input. This is a pointer to - * the first element of a QEMUOptionParameter array which is terminated by an - * entry with entry->name == NULL. - */ - -QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list, - const char *name); -int set_option_parameter(QEMUOptionParameter *list, const char *name, - const char *value); -int set_option_parameter_int(QEMUOptionParameter *list, const char *name, - uint64_t value); -QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest, - QEMUOptionParameter *list); -QEMUOptionParameter *parse_option_parameters(const char *param, - QEMUOptionParameter *list, QEMUOptionParameter *dest); -void free_option_parameters(QEMUOptionParameter *list); -void print_option_parameters(QEMUOptionParameter *list); -void print_option_help(QEMUOptionParameter *list); - /* ------------------------------------------------------------------ */ typedef struct QemuOpt QemuOpt; diff --git a/util/qemu-option.c b/util/qemu-option.c index 41f5b0f..7545486 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -123,22 +123,6 @@ int get_param_value(char *buf, int buf_size, return get_next_param_value(buf, buf_size, tag, &str); } -/* - * Searches an option list for an option with the given name - */ -QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list, - const char *name) -{ - while (list && list->name) { - if (!strcmp(list->name, name)) { - return list; - } - list++; - } - - return NULL; -} - static void parse_option_bool(const char *name, const char *value, bool *ret, Error **errp) { @@ -212,275 +196,6 @@ static void parse_option_size(const char *name, const char *value, } } -/* - * Sets the value of a parameter in a given option list. The parsing of the - * value depends on the type of option: - * - * OPT_FLAG (uses value.n): - * If no value is given, the flag is set to 1. - * Otherwise the value must be "on" (set to 1) or "off" (set to 0) - * - * OPT_STRING (uses value.s): - * value is strdup()ed and assigned as option value - * - * OPT_SIZE (uses value.n): - * The value is converted to an integer. Suffixes for kilobytes etc. are - * allowed (powers of 1024). - * - * Returns 0 on succes, -1 in error cases - */ -int set_option_parameter(QEMUOptionParameter *list, const char *name, - const char *value) -{ - bool flag; - Error *local_err = NULL; - - // Find a matching parameter - list = get_option_parameter(list, name); - if (list == NULL) { - fprintf(stderr, "Unknown option '%s'\n", name); - return -1; - } - - // Process parameter - switch (list->type) { - case OPT_FLAG: - parse_option_bool(name, value, &flag, &local_err); - if (!error_is_set(&local_err)) { - list->value.n = flag; - } - break; - - case OPT_STRING: - if (value != NULL) { - list->value.s = g_strdup(value); - } else { - fprintf(stderr, "Option '%s' needs a parameter\n", name); - return -1; - } - break; - - case OPT_SIZE: - parse_option_size(name, value, &list->value.n, &local_err); - break; - - default: - fprintf(stderr, "Bug: Option '%s' has an unknown type\n", name); - return -1; - } - - if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); - return -1; - } - - return 0; -} - -/* - * Sets the given parameter to an integer instead of a string. - * This function cannot be used to set string options. - * - * Returns 0 on success, -1 in error cases - */ -int set_option_parameter_int(QEMUOptionParameter *list, const char *name, - uint64_t value) -{ - // Find a matching parameter - list = get_option_parameter(list, name); - if (list == NULL) { - fprintf(stderr, "Unknown option '%s'\n", name); - return -1; - } - - // Process parameter - switch (list->type) { - case OPT_FLAG: - case OPT_NUMBER: - case OPT_SIZE: - list->value.n = value; - break; - - default: - return -1; - } - - return 0; -} - -/* - * Frees a option list. If it contains strings, the strings are freed as well. - */ -void free_option_parameters(QEMUOptionParameter *list) -{ - QEMUOptionParameter *cur = list; - - while (cur && cur->name) { - if (cur->type == OPT_STRING) { - g_free(cur->value.s); - } - cur++; - } - - g_free(list); -} - -/* - * Count valid options in list - */ -static size_t count_option_parameters(QEMUOptionParameter *list) -{ - size_t num_options = 0; - - while (list && list->name) { - num_options++; - list++; - } - - return num_options; -} - -/* - * Append an option list (list) to an option list (dest). - * - * If dest is NULL, a new copy of list is created. - * - * Returns a pointer to the first element of dest (or the newly allocated copy) - */ -QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest, - QEMUOptionParameter *list) -{ - size_t num_options, num_dest_options; - - num_options = count_option_parameters(dest); - num_dest_options = num_options; - - num_options += count_option_parameters(list); - - dest = g_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter)); - dest[num_dest_options].name = NULL; - - while (list && list->name) { - if (get_option_parameter(dest, list->name) == NULL) { - dest[num_dest_options++] = *list; - dest[num_dest_options].name = NULL; - } - list++; - } - - return dest; -} - -/* - * Parses a parameter string (param) into an option list (dest). - * - * list is the template option list. If dest is NULL, a new copy of list is - * created. If list is NULL, this function fails. - * - * A parameter string consists of one or more parameters, separated by commas. - * Each parameter consists of its name and possibly of a value. In the latter - * case, the value is delimited by an = character. To specify a value which - * contains commas, double each comma so it won't be recognized as the end of - * the parameter. - * - * For more details of the parsing see above. - * - * Returns a pointer to the first element of dest (or the newly allocated copy) - * or NULL in error cases - */ -QEMUOptionParameter *parse_option_parameters(const char *param, - QEMUOptionParameter *list, QEMUOptionParameter *dest) -{ - QEMUOptionParameter *allocated = NULL; - char name[256]; - char value[256]; - char *param_delim, *value_delim; - char next_delim; - - if (list == NULL) { - return NULL; - } - - if (dest == NULL) { - dest = allocated = append_option_parameters(NULL, list); - } - - while (*param) { - - // Find parameter name and value in the string - param_delim = strchr(param, ','); - value_delim = strchr(param, '='); - - if (value_delim && (value_delim < param_delim || !param_delim)) { - next_delim = '='; - } else { - next_delim = ','; - value_delim = NULL; - } - - param = get_opt_name(name, sizeof(name), param, next_delim); - if (value_delim) { - param = get_opt_value(value, sizeof(value), param + 1); - } - if (*param != '\0') { - param++; - } - - // Set the parameter - if (set_option_parameter(dest, name, value_delim ? value : NULL)) { - goto fail; - } - } - - return dest; - -fail: - // Only free the list if it was newly allocated - free_option_parameters(allocated); - return NULL; -} - -/* - * Prints all options of a list that have a value to stdout - */ -void print_option_parameters(QEMUOptionParameter *list) -{ - while (list && list->name) { - switch (list->type) { - case OPT_STRING: - if (list->value.s != NULL) { - printf("%s='%s' ", list->name, list->value.s); - } - break; - case OPT_FLAG: - printf("%s=%s ", list->name, list->value.n ? "on" : "off"); - break; - case OPT_SIZE: - case OPT_NUMBER: - printf("%s=%" PRId64 " ", list->name, list->value.n); - break; - default: - printf("%s=(unknown type) ", list->name); - break; - } - list++; - } -} - -/* - * Prints an overview of all available options - */ -void print_option_help(QEMUOptionParameter *list) -{ - printf("Supported options:\n"); - while (list && list->name) { - printf("%-16s %s\n", list->name, - list->help ? list->help : "No description available"); - list++; - } -} - /* ------------------------------------------------------------------ */ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (6 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 7/9] qemu-option: remove QEMUOptionParameter related functions and struct Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 9:50 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 9/9] qemu-option: use qemu_opts_del without judging NULL Dong Xu Wang 2013-07-17 12:54 ` [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Eric Blake 9 siblings, 1 reply; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- util/qemu-option.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/util/qemu-option.c b/util/qemu-option.c index 7545486..f4a0282 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -656,6 +656,10 @@ void qemu_opts_del(QemuOpts *opts) { QemuOpt *opt; + if (!opts) { + return; + } + for (;;) { opt = QTAILQ_FIRST(&opts->head); if (opt == NULL) -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL Dong Xu Wang @ 2013-07-17 9:50 ` Dong Xu Wang 2013-07-17 11:48 ` Kevin Wolf 0 siblings, 1 reply; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:50 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, wdongxu, Stefan Hajnoczi, Dong Xu Wang Please ignore patch 8 and patch9, it won't work and I will re-send them seperately. Sorry for the inconvenience. On Wed, Jul 17, 2013 at 5:29 PM, Dong Xu Wang <wdongxu@linux.vnet.ibm.com> wrote: > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > --- > util/qemu-option.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/util/qemu-option.c b/util/qemu-option.c > index 7545486..f4a0282 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -656,6 +656,10 @@ void qemu_opts_del(QemuOpts *opts) > { > QemuOpt *opt; > > + if (!opts) { > + return; > + } > + > for (;;) { > opt = QTAILQ_FIRST(&opts->head); > if (opt == NULL) > -- > 1.7.11.7 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL 2013-07-17 9:50 ` Dong Xu Wang @ 2013-07-17 11:48 ` Kevin Wolf 0 siblings, 0 replies; 15+ messages in thread From: Kevin Wolf @ 2013-07-17 11:48 UTC (permalink / raw) To: Dong Xu Wang; +Cc: wdongxu, qemu-devel, Stefan Hajnoczi Am 17.07.2013 um 11:50 hat Dong Xu Wang geschrieben: > Please ignore patch 8 and patch9, it won't work and I will re-send > them seperately. > Sorry for the inconvenience. Seriously? We're at v17 and you're still sending code that is broken enough that you recall it after half an hour? How much and what kind of testing did you give the series after you made the latest changes? I'm not planning to review anything that is likely to be broken anyway because you rushed things. Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH V17 9/9] qemu-option: use qemu_opts_del without judging NULL 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (7 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL Dong Xu Wang @ 2013-07-17 9:29 ` Dong Xu Wang 2013-07-17 12:54 ` [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Eric Blake 9 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 9:29 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, wdongxu, stefanha, Dong Xu Wang Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- block.c | 8 ++------ hw/core/qdev.c | 4 +--- qemu-char.c | 4 +--- qemu-img.c | 4 +--- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/block.c b/block.c index e1d1c3d..e700593 100644 --- a/block.c +++ b/block.c @@ -408,9 +408,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts) ret = cco.ret; out: - if (!opts) { - qemu_opts_del(cco.opts); - } + qemu_opts_del(cco.opts); g_free(cco.filename); return ret; } @@ -4575,9 +4573,7 @@ void bdrv_img_create(const char *filename, const char *fmt, } out: - if (opts) { - qemu_opts_del(opts); - } + qemu_opts_del(opts); qemu_opts_free(create_opts); if (bs) { diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 9190a7e..2a33eca 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -768,9 +768,7 @@ static void device_initfn(Object *obj) static void device_finalize(Object *obj) { DeviceState *dev = DEVICE(obj); - if (dev->opts) { - qemu_opts_del(dev->opts); - } + qemu_opts_del(dev->opts); } static void device_class_base_init(ObjectClass *class, void *data) diff --git a/qemu-char.c b/qemu-char.c index 800d6a6..6cd650d 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3379,9 +3379,7 @@ void qemu_chr_delete(CharDriverState *chr) } g_free(chr->filename); g_free(chr->label); - if (chr->opts) { - qemu_opts_del(chr->opts); - } + qemu_opts_del(chr->opts); g_free(chr); } diff --git a/qemu-img.c b/qemu-img.c index 3f1b6ee..44dd07d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1527,9 +1527,7 @@ static int img_convert(int argc, char **argv) out: qemu_progress_end(); qemu_opts_free(create_opts); - if (opts) { - qemu_opts_del(opts); - } + qemu_opts_del(opts); qemu_vfree(buf); if (out_bs) { bdrv_delete(out_bs); -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang ` (8 preceding siblings ...) 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 9/9] qemu-option: use qemu_opts_del without judging NULL Dong Xu Wang @ 2013-07-17 12:54 ` Eric Blake 2013-07-17 14:11 ` Dong Xu Wang 9 siblings, 1 reply; 15+ messages in thread From: Eric Blake @ 2013-07-17 12:54 UTC (permalink / raw) To: Dong Xu Wang; +Cc: kwolf, wdongxu, qemu-devel, stefanha [-- Attachment #1: Type: text/plain, Size: 2270 bytes --] On 07/17/2013 03:29 AM, Dong Xu Wang wrote: > These patches will replace QEMUOptionParameter with QemuOpts. Change logs > please go to each patch's commit message. > > Dong Xu Wang (9): > qemu-option: add def_value_str in QemuOptDesc struct and rewrite > qemu_opts_print > qemu-option: avoid duplication of default value in QemuOpts > qemu-option: create four QemuOptsList related functions > qemu-option: create some QemuOpts functons > block: use QemuOpts support in block layer > qapi: query-command-line-options outputs def_value_str > qemu-option: remove QEMUOptionParameter related functions and struct > qemu-option: make qemu_opts_del accept opts being NULL > qemu-option: use qemu_opts_del without judging NULL > > > -- Ouch - by putting your additional comments after an 'end-of-message' marker, you made it harder for me to reply. (Sane mailers intentionally drop text after a "-- " line when replying). > V17 add qemu_opts_del's support when opts is NULL, and made changes > based on Eric's comments. I tried to split PATCH 5 into small pieces, > but I found it is very hard, bdrv_* functions are all using > QEMUOptionParameter or QemuOpts, if I split them, I have to support two > parsers in block.c and qemu-img.c, it will make code very hard to read, Yes, if you split the patches, then there will be a window of time in the series where you have to support BOTH parsers at the same time. But since the existing parser is already present, what's so hard about that? By having two different callbacks, one with the old signature and one with the new, with exactly one of the callbacks being non-NULL, it should be relatively easy to decide which of the two callbacks to use. > so I leave them in one big patch, I am sorry it is realy hard to review. I still think that the ease of review that would be added by splitting the patch is worth making the effort. Yes, it may be harder for you, but if the end result is that more people are willing to review the patch, then the community as a whole spends less collective time, and the series will be accepted sooner. -- 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: 621 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser 2013-07-17 12:54 ` [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Eric Blake @ 2013-07-17 14:11 ` Dong Xu Wang 0 siblings, 0 replies; 15+ messages in thread From: Dong Xu Wang @ 2013-07-17 14:11 UTC (permalink / raw) To: Eric Blake; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi On Wed, Jul 17, 2013 at 8:54 PM, Eric Blake <eblake@redhat.com> wrote: > On 07/17/2013 03:29 AM, Dong Xu Wang wrote: >> These patches will replace QEMUOptionParameter with QemuOpts. Change logs >> please go to each patch's commit message. >> >> Dong Xu Wang (9): >> qemu-option: add def_value_str in QemuOptDesc struct and rewrite >> qemu_opts_print >> qemu-option: avoid duplication of default value in QemuOpts >> qemu-option: create four QemuOptsList related functions >> qemu-option: create some QemuOpts functons >> block: use QemuOpts support in block layer >> qapi: query-command-line-options outputs def_value_str >> qemu-option: remove QEMUOptionParameter related functions and struct >> qemu-option: make qemu_opts_del accept opts being NULL >> qemu-option: use qemu_opts_del without judging NULL >> > >> >> -- > > Ouch - by putting your additional comments after an 'end-of-message' > marker, you made it harder for me to reply. (Sane mailers intentionally > drop text after a "-- " line when replying). > >> V17 add qemu_opts_del's support when opts is NULL, and made changes >> based on Eric's comments. I tried to split PATCH 5 into small pieces, >> but I found it is very hard, bdrv_* functions are all using >> QEMUOptionParameter or QemuOpts, if I split them, I have to support two >> parsers in block.c and qemu-img.c, it will make code very hard to read, > > Yes, if you split the patches, then there will be a window of time in > the series where you have to support BOTH parsers at the same time. But > since the existing parser is already present, what's so hard about that? > By having two different callbacks, one with the old signature and one > with the new, with exactly one of the callbacks being non-NULL, it > should be relatively easy to decide which of the two callbacks to use. > >> so I leave them in one big patch, I am sorry it is realy hard to review. > > I still think that the ease of review that would be added by splitting > the patch is worth making the effort. Yes, it may be harder for you, Such as bdrv > but if the end result is that more people are willing to review the > patch, then the community as a whole spends less collective time, and > the series will be accepted sooner. > Thank you Eric. If splited into patches and modified each block backend in each patch, code must look like this: if (drv->create_options) { create_options = append_option_parameters(create_options, drv->create_options); create_options = append_option_parameters(create_options, proto_drv->create_options); print_option_help(create_options); free_option_parameters(create_options); } if (drv->bdrv_create_opts) { create_opts = qemu_opts_append(drv->bdrv_create_opts, proto_drv->bdrv_create_opts); qemu_opts_print_help(create_opts); qemu_opts_free(create_opts); } It will be many "if", I think that will make code hard to review. If you insist, I can split into small patches. > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-07-17 14:12 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-17 9:29 [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 1/9] qemu-option: add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 2/9] qemu-option: avoid duplication of default value in QemuOpts Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 3/9] qemu-option: create four QemuOptsList related functions Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 4/9] qemu-option: create some QemuOpts functons Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 5/9] block: use QemuOpts support in block layer Dong Xu Wang 2013-07-17 9:48 ` Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 6/9] qapi: query-command-line-options outputs def_value_str Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 7/9] qemu-option: remove QEMUOptionParameter related functions and struct Dong Xu Wang 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 8/9] qemu-option: make qemu_opts_del accept opts being NULL Dong Xu Wang 2013-07-17 9:50 ` Dong Xu Wang 2013-07-17 11:48 ` Kevin Wolf 2013-07-17 9:29 ` [Qemu-devel] [PATCH V17 9/9] qemu-option: use qemu_opts_del without judging NULL Dong Xu Wang 2013-07-17 12:54 ` [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser Eric Blake 2013-07-17 14:11 ` Dong Xu Wang
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).