* [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser
@ 2012-09-27 5:14 Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication Dong Xu Wang
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
This is still a RFC, I cut some block image formats to simplify
the exploring.
patch 1-3 are from Luiz, added Markus's comments, discussion could be found here:
http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html
patch 4-5: because qemu_opts_create can not fail while id is null, so create
function qemu_opts_create_nofail and use it.
patch 6: create function qemu_opt_set_number, like qemu_opt_set_bool.
patch 7: remove QEMUOptionParameter and use QemuOpts parser.
v1->v2:
1) add Luiz's patches.
2) create qemu_opt_set_number() and qemu_opts_create_nofail() functions.
3) add QemuOptsList map to drivers.
4) use original opts parser, not creating new ones.
5) fix other bugs.
Dong Xu Wang (7):
qemu-option: qemu_opt_set_bool(): fix code duplication
qemu-option: opt_set(): split it up into more functions
qemu-option: qemu_opts_validate(): fix duplicated code
introduce qemu_opts_create_nofail function
use qemu_opts_create_nofail
create new function: qemu_opt_set_number
remove QEMUOptionParameter
block.c | 88 +++++-----
block.h | 5 +-
block/Makefile.objs | 9 +-
block/qcow2.c | 175 ++++++++++---------
block/raw-posix.c | 68 ++++----
block/raw.c | 31 ++--
block_int.h | 6 +-
blockdev.c | 2 +-
hw/watchdog.c | 2 +-
qemu-config.c | 7 +-
qemu-img.c | 60 +++----
qemu-option.c | 501 ++++++++++++++++-----------------------------------
qemu-option.h | 47 +----
qemu-sockets.c | 8 +-
vl.c | 6 +-
15 files changed, 401 insertions(+), 614 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 11:55 ` Paolo Bonzini
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 2/7] qemu-option: opt_set(): split it up into more functions Dong Xu Wang
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
Call qemu_opt_set() instead of duplicating opt_set().
It will set opt->str in qemu_opt_set_bool, without opt->str, there
will be some potential bugs.
These are uses of opt->str, and what happens when it isn't set:
* qemu_opt_get(): returns NULL, which means "not set". Bug can bite
when value isn't the default value.
* qemu_opt_parse(): passes NULL to parse_option_bool(), which treats it
like "on". Wrong if the value is actually false. Bug can bite when
qemu_opts_validate() runs after qemu_opt_set_bool().
* qemu_opt_del(): passes NULL to g_free(), which is just fine.
* qemu_opt_foreach(): passes NULL to the callback, which is unlikely to
be prepared for it.
* qemu_opts_print(): prints NULL, which crashes on some systems.
* qemu_opts_to_qdict(): passes NULL to qstring_from_str(), which
crashes.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
qemu-option.c | 28 +---------------------------
1 files changed, 1 insertions(+), 27 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 27891e7..0b81e77 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -667,33 +667,7 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
{
- QemuOpt *opt;
- const QemuOptDesc *desc = opts->list->desc;
- int i;
-
- for (i = 0; desc[i].name != NULL; i++) {
- if (strcmp(desc[i].name, name) == 0) {
- break;
- }
- }
- if (desc[i].name == NULL) {
- if (i == 0) {
- /* empty list -> allow any */;
- } else {
- qerror_report(QERR_INVALID_PARAMETER, name);
- return -1;
- }
- }
-
- opt = g_malloc0(sizeof(*opt));
- opt->name = g_strdup(name);
- opt->opts = opts;
- QTAILQ_INSERT_TAIL(&opts->head, opt, next);
- if (desc[i].name != NULL) {
- opt->desc = desc+i;
- }
- opt->value.boolean = !!val;
- return 0;
+ return qemu_opt_set(opts, name, val ? "on" : "off");
}
int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 2/7] qemu-option: opt_set(): split it up into more functions
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 3/7] qemu-option: qemu_opts_validate(): fix duplicated code Dong Xu Wang
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
The new functions are opts_accepts_any() and find_desc_by_name(), which
are also going to be used by qemu_opts_validate() (see next commit).
This also makes opt_set() slightly more readable.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
qemu-option.c | 40 ++++++++++++++++++++++++----------------
1 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 0b81e77..d8b0988 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -602,26 +602,36 @@ static void qemu_opt_del(QemuOpt *opt)
g_free(opt);
}
-static void opt_set(QemuOpts *opts, const char *name, const char *value,
- bool prepend, Error **errp)
+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)
{
- QemuOpt *opt;
- const QemuOptDesc *desc = opts->list->desc;
- Error *local_err = NULL;
int i;
for (i = 0; desc[i].name != NULL; i++) {
if (strcmp(desc[i].name, name) == 0) {
- break;
+ return &desc[i];
}
}
- if (desc[i].name == NULL) {
- if (i == 0) {
- /* empty list -> allow any */;
- } else {
- error_set(errp, QERR_INVALID_PARAMETER, name);
- return;
- }
+
+ return NULL;
+}
+
+static void opt_set(QemuOpts *opts, const char *name, const char *value,
+ bool prepend, Error **errp)
+{
+ QemuOpt *opt;
+ const QemuOptDesc *desc;
+ Error *local_err = NULL;
+
+ desc = find_desc_by_name(opts->list->desc, name);
+ if (!desc && !opts_accepts_any(opts)) {
+ error_set(errp, QERR_INVALID_PARAMETER, name);
+ return;
}
opt = g_malloc0(sizeof(*opt));
@@ -632,9 +642,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
} else {
QTAILQ_INSERT_TAIL(&opts->head, opt, next);
}
- if (desc[i].name != NULL) {
- opt->desc = desc+i;
- }
+ opt->desc = desc;
if (value) {
opt->str = g_strdup(value);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 3/7] qemu-option: qemu_opts_validate(): fix duplicated code
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 2/7] qemu-option: opt_set(): split it up into more functions Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 4/7] introduce qemu_opts_create_nofail function Dong Xu Wang
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
Use opts_accepts_any() and find_desc_by_name().
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
qemu-option.c | 14 +++-----------
1 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index d8b0988..b7baacc 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -1050,23 +1050,15 @@ void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
QemuOpt *opt;
Error *local_err = NULL;
- assert(opts->list->desc[0].name == NULL);
+ assert(opts_accepts_any(opts));
QTAILQ_FOREACH(opt, &opts->head, next) {
- int i;
-
- for (i = 0; desc[i].name != NULL; i++) {
- if (strcmp(desc[i].name, opt->name) == 0) {
- break;
- }
- }
- if (desc[i].name == NULL) {
+ opt->desc = find_desc_by_name(desc, opt->name);
+ if (!opt->desc) {
error_set(errp, QERR_INVALID_PARAMETER, opt->name);
return;
}
- opt->desc = &desc[i];
-
qemu_opt_parse(opt, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 4/7] introduce qemu_opts_create_nofail function
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
` (2 preceding siblings ...)
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 3/7] qemu-option: qemu_opts_validate(): fix duplicated code Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 5/7] use qemu_opts_create_nofail Dong Xu Wang
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
While id is NULL, qemu_opts_create can not fail, so ignore
errors is fine.
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
qemu-option.c | 5 +++++
qemu-option.h | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index b7baacc..818408b 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -763,6 +763,11 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
return opts;
}
+QemuOpts *qemu_opts_create_nofail(QemuOptsList *list)
+{
+ return qemu_opts_create(list, NULL, 0, NULL);
+}
+
void qemu_opts_reset(QemuOptsList *list)
{
QemuOpts *opts, *next_opts;
diff --git a/qemu-option.h b/qemu-option.h
index ca72986..b0f8d1e 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -133,6 +133,7 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
int fail_if_exists, Error **errp);
+QemuOpts *qemu_opts_create_nofail(QemuOptsList *list);
void qemu_opts_reset(QemuOptsList *list);
void qemu_opts_loc_restore(QemuOpts *opts);
int qemu_opts_set(QemuOptsList *list, const char *id,
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 5/7] use qemu_opts_create_nofail
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
` (3 preceding siblings ...)
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 4/7] introduce qemu_opts_create_nofail function Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter Dong Xu Wang
6 siblings, 0 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
use qemu_opts_create_nofail while id is NULL.
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
blockdev.c | 2 +-
hw/watchdog.c | 2 +-
qemu-config.c | 4 ++--
qemu-img.c | 2 +-
qemu-sockets.c | 8 ++++----
vl.c | 6 +++---
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 7c83baa..1d0fe0f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -559,7 +559,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
break;
case IF_VIRTIO:
/* add virtio block device */
- opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(qemu_find_opts("device"));
if (arch_type == QEMU_ARCH_S390X) {
qemu_opt_set(opts, "driver", "virtio-blk-s390");
} else {
diff --git a/hw/watchdog.c b/hw/watchdog.c
index b52aced..5c82c17 100644
--- a/hw/watchdog.c
+++ b/hw/watchdog.c
@@ -66,7 +66,7 @@ int select_watchdog(const char *p)
QLIST_FOREACH(model, &watchdog_list, entry) {
if (strcasecmp(model->wdt_name, p) == 0) {
/* add the device */
- opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(qemu_find_opts("device"));
qemu_opt_set(opts, "driver", p);
return 0;
}
diff --git a/qemu-config.c b/qemu-config.c
index 12eafbb..6e3718e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -762,7 +762,7 @@ int qemu_global_option(const char *str)
return -1;
}
- opts = qemu_opts_create(&qemu_global_opts, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(&qemu_global_opts);
qemu_opt_set(opts, "driver", driver);
qemu_opt_set(opts, "property", property);
qemu_opt_set(opts, "value", str+offset+1);
@@ -849,7 +849,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
error_free(local_err);
goto out;
}
- opts = qemu_opts_create(list, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(list);
continue;
}
if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/qemu-img.c b/qemu-img.c
index f17f187..f1c224e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1778,7 +1778,7 @@ static int img_resize(int argc, char **argv)
}
/* Parse size */
- param = qemu_opts_create(&resize_options, NULL, 0, NULL);
+ param = qemu_opts_create_nofail(&resize_options);
if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) {
/* Error message already printed when size parsing fails */
ret = -1;
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 361d890..a576a24 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -469,7 +469,7 @@ int inet_listen(const char *str, char *ostr, int olen,
char *optstr;
int sock = -1;
- opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(&dummy_opts);
if (inet_parse(opts, str) == 0) {
sock = inet_listen_opts(opts, port_offset, errp);
if (sock != -1 && ostr) {
@@ -498,7 +498,7 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
QemuOpts *opts;
int sock = -1;
- opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(&dummy_opts);
if (inet_parse(opts, str) == 0) {
if (block) {
qemu_opt_set(opts, "block", "on");
@@ -597,7 +597,7 @@ int unix_listen(const char *str, char *ostr, int olen)
char *path, *optstr;
int sock, len;
- opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(&dummy_opts);
optstr = strchr(str, ',');
if (optstr) {
@@ -625,7 +625,7 @@ int unix_connect(const char *path)
QemuOpts *opts;
int sock;
- opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(&dummy_opts);
qemu_opt_set(opts, "path", path);
sock = unix_connect_opts(opts);
qemu_opts_del(opts);
diff --git a/vl.c b/vl.c
index 7c577fa..e13aead 100644
--- a/vl.c
+++ b/vl.c
@@ -1863,7 +1863,7 @@ static int balloon_parse(const char *arg)
return -1;
} else {
/* create empty opts */
- opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
+ opts = qemu_opts_create_nofail(qemu_find_opts("device"));
}
qemu_opt_set(opts, "driver", "virtio-balloon");
return 0;
@@ -2111,14 +2111,14 @@ static int virtcon_parse(const char *devname)
exit(1);
}
- bus_opts = qemu_opts_create(device, NULL, 0, NULL);
+ bus_opts = qemu_opts_create_nofail(device);
if (arch_type == QEMU_ARCH_S390X) {
qemu_opt_set(bus_opts, "driver", "virtio-serial-s390");
} else {
qemu_opt_set(bus_opts, "driver", "virtio-serial-pci");
}
- dev_opts = qemu_opts_create(device, NULL, 0, NULL);
+ dev_opts = qemu_opts_create_nofail(device);
qemu_opt_set(dev_opts, "driver", "virtconsole");
snprintf(label, sizeof(label), "virtcon%d", index);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
` (4 preceding siblings ...)
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 5/7] use qemu_opts_create_nofail Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 11:56 ` Paolo Bonzini
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter Dong Xu Wang
6 siblings, 1 reply; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
qemu-option.c | 8 ++++++++
qemu-option.h | 1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 818408b..2b52576 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -678,6 +678,14 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
return qemu_opt_set(opts, name, val ? "on" : "off");
}
+int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val)
+{
+ char buffer[1024];
+ snprintf(buffer, sizeof(buffer), "%" PRId64, val);
+ return qemu_opt_set(opts, name, buffer);
+}
+
+
int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
int abort_on_failure)
{
diff --git a/qemu-option.h b/qemu-option.h
index b0f8d1e..002dd07 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -126,6 +126,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);
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);
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
` (5 preceding siblings ...)
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number Dong Xu Wang
@ 2012-09-27 5:14 ` Dong Xu Wang
2012-09-27 12:03 ` Paolo Bonzini
6 siblings, 1 reply; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-27 5:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Dong Xu Wang, armbru, lcapitulino
remove QEMUOptionParameter, and use QemuOpts and QemuOptsList.
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
block.c | 88 ++++++------
block.h | 5 +-
block/Makefile.objs | 9 +-
block/qcow2.c | 175 ++++++++++++-----------
block/raw-posix.c | 68 ++++-----
block/raw.c | 31 +++--
block_int.h | 6 +-
qemu-config.c | 3 +
qemu-img.c | 58 ++++----
qemu-option.c | 408 +++++++++++++++------------------------------------
qemu-option.h | 45 +-----
11 files changed, 347 insertions(+), 549 deletions(-)
diff --git a/block.c b/block.c
index e78039b..99a7a03 100644
--- a/block.c
+++ b/block.c
@@ -351,7 +351,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name)
typedef struct CreateCo {
BlockDriver *drv;
char *filename;
- QEMUOptionParameter *options;
+ QemuOpts *opts;
int ret;
} CreateCo;
@@ -360,11 +360,11 @@ 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)
+ QemuOpts *opts)
{
int ret;
@@ -372,7 +372,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
CreateCo cco = {
.drv = drv,
.filename = g_strdup(filename),
- .options = options,
+ .opts = opts,
.ret = NOT_DONE,
};
@@ -397,7 +397,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
return ret;
}
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char *filename, QemuOpts *opts)
{
BlockDriver *drv;
@@ -406,7 +406,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
return -ENOENT;
}
- return bdrv_create(drv, filename, options);
+ return bdrv_create(drv, filename, opts);
}
/*
@@ -746,7 +746,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
int64_t total_size;
int is_protocol = 0;
BlockDriver *bdrv_qcow2;
- QEMUOptionParameter *options;
+ QemuOpts *opts;
char backing_filename[PATH_MAX];
/* if snapshot, we create a temporary backing file and open it
@@ -779,17 +779,18 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
return -errno;
bdrv_qcow2 = bdrv_find_format("qcow2");
- options = parse_option_parameters("", bdrv_qcow2->create_options, NULL);
+ opts = qemu_opts_create(qemu_find_opts("qcow2-create-opts"),
+ NULL, 0, NULL);
- set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size);
- set_option_parameter(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(options, BLOCK_OPT_BACKING_FMT,
+ qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT,
drv->format_name);
}
- ret = bdrv_create(bdrv_qcow2, tmp_filename, options);
- free_option_parameters(options);
+ ret = bdrv_create(bdrv_qcow2, tmp_filename, opts);
+ qemu_opts_del(opts);
if (ret < 0) {
return ret;
}
@@ -3905,8 +3906,10 @@ int bdrv_img_create(const char *filename, const char *fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags)
{
- QEMUOptionParameter *param = NULL, *create_options = NULL;
- QEMUOptionParameter *backing_fmt, *backing_file, *size;
+ QemuOpts *param = NULL;
+ QemuOptsList *create_options = NULL;
+ const char *backing_fmt, *backing_file;
+ int64_t size;
BlockDriverState *bs = NULL;
BlockDriver *drv, *proto_drv;
BlockDriver *backing_drv = NULL;
@@ -3926,21 +3929,16 @@ int bdrv_img_create(const char *filename, const char *fmt,
ret = -EINVAL;
goto out;
}
-
- create_options = append_option_parameters(create_options,
- drv->create_options);
- create_options = append_option_parameters(create_options,
- proto_drv->create_options);
-
+ create_options = append_opts_list(drv->bdrv_create_options(),
+ proto_drv->bdrv_create_options());
/* Create parameter list with default values */
- param = parse_option_parameters("", create_options, param);
+ param = qemu_opts_create(create_options, NULL, 0, NULL);
- set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
+ qemu_opt_set_number(param, 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(param, options, NULL) != 0) {
error_report("Invalid options for file format '%s'.", fmt);
ret = -EINVAL;
goto out;
@@ -3948,7 +3946,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
if (base_filename) {
- if (set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
+ if (qemu_opt_set(param, BLOCK_OPT_BACKING_FILE,
base_filename)) {
error_report("Backing file not supported for file format '%s'",
fmt);
@@ -3958,7 +3956,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
if (base_fmt) {
- if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
+ if (qemu_opt_set(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
error_report("Backing file format not supported for file "
"format '%s'", fmt);
ret = -EINVAL;
@@ -3966,9 +3964,9 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
}
- 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(param, BLOCK_OPT_BACKING_FILE);
+ if (backing_file) {
+ if (!strcmp(filename, backing_file)) {
error_report("Error: Trying to create an image with the "
"same filename as the backing file");
ret = -EINVAL;
@@ -3976,12 +3974,12 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
}
- 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(param, BLOCK_OPT_BACKING_FMT);
+ if (backing_fmt) {
+ backing_drv = bdrv_find_format(backing_fmt);
if (!backing_drv) {
error_report("Unknown backing file format '%s'",
- backing_fmt->value.s);
+ backing_fmt);
ret = -EINVAL;
goto out;
}
@@ -3989,11 +3987,10 @@ int bdrv_img_create(const char *filename, const char *fmt,
// 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_number(param, BLOCK_OPT_SIZE, -1);
+ if (size == -1) {
+ if (backing_file) {
uint64_t size;
- char buf[32];
int back_flags;
/* backing files always opened read-only */
@@ -4002,16 +3999,15 @@ int bdrv_img_create(const char *filename, const char *fmt,
bs = bdrv_new("");
- ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv);
+ ret = bdrv_open(bs, backing_file, back_flags, backing_drv);
if (ret < 0) {
- error_report("Could not open '%s'", backing_file->value.s);
+ error_report("Could not open '%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_set_number(param, BLOCK_OPT_SIZE, size);
} else {
error_report("Image creation needs a size parameter");
ret = -EINVAL;
@@ -4020,7 +4016,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
printf("Formatting '%s', fmt=%s ", filename, fmt);
- print_option_parameters(param);
+ qemu_opts_print(param, NULL);
puts("");
ret = bdrv_create(drv, filename, param);
@@ -4039,8 +4035,10 @@ int bdrv_img_create(const char *filename, const char *fmt,
}
out:
- free_option_parameters(create_options);
- free_option_parameters(param);
+ free_opts_list(create_options);
+ if (param) {
+ qemu_opts_del(param);
+ }
if (bs) {
bdrv_delete(bs);
diff --git a/block.h b/block.h
index 2e2be11..df9a1de 100644
--- a/block.h
+++ b/block.h
@@ -119,8 +119,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);
int bdrv_create(BlockDriver *drv, const char* filename,
- QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+ QemuOpts *options);
+int bdrv_create_file(const char *filename, QemuOpts *options);
BlockDriverState *bdrv_new(const char *device_name);
void bdrv_make_anon(BlockDriverState *bs);
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
@@ -404,5 +404,4 @@ typedef enum {
#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
-
#endif
diff --git a/block/Makefile.objs b/block/Makefile.objs
index b5754d3..19de020 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,8 +1,9 @@
-block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
+#block-obj-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
-block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
-block-obj-y += qed-check.o
-block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
+#block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
+#block-obj-y += qed-check.o
+#block-obj-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
+block-obj-y += raw.o
block-obj-y += stream.o
block-obj-$(CONFIG_WIN32) += raw-win32.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
diff --git a/block/qcow2.c b/block/qcow2.c
index 8f183f1..aa2202f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1170,7 +1170,7 @@ static int preallocate(BlockDriverState *bs)
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, int prealloc,
- QEMUOptionParameter *options, int version)
+ QemuOpts *options, int version)
{
/* Calculate cluster_bits */
int cluster_bits;
@@ -1201,6 +1201,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
uint8_t* refcount_table;
int ret;
+ qemu_opt_set(options, BLOCK_OPT_SIZE, "0");
ret = bdrv_create_file(filename, options);
if (ret < 0) {
return ret;
@@ -1304,7 +1305,7 @@ out:
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;
@@ -1313,45 +1314,41 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options)
size_t cluster_size = DEFAULT_CLUSTER_SIZE;
int prealloc = 0;
int version = 2;
+ const char *buf;
/* 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_number(opts, BLOCK_OPT_SIZE, 0) / 512;
+ backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
+ backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
+ if (qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, 0)) {
+ flags |= BLOCK_FLAG_ENCRYPT;
+ }
+ cluster_size =
+ qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE, DEFAULT_CLUSTER_SIZE);
+ buf = qemu_opt_get(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);
+ return -EINVAL;
+ }
+
+ buf = qemu_opt_get(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);
+ return -EINVAL;
+ }
+
+ if (qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS, 0)) {
+ flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
}
if (backing_file && prealloc) {
@@ -1367,7 +1364,7 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options)
}
return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
- cluster_size, prealloc, options, version);
+ cluster_size, prealloc, opts, version);
}
static int qcow2_make_empty(BlockDriverState *bs)
@@ -1628,51 +1625,60 @@ 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_NUMBER,
+ .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"
+ },
+ {
+ .name = BLOCK_OPT_CLUSTER_SIZE,
+ .type = QEMU_OPT_SIZE,
+ .help = "qcow2 cluster size",
+ .def_value = 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",
+ },
+ { /* end of list */ }
+ }
};
+static QemuOptsList *qcow2_create_options(void)
+{
+ return &qcow2_create_opts;
+}
+
static BlockDriver bdrv_qcow2 = {
.format_name = "qcow2",
.instance_size = sizeof(BDRVQcowState),
@@ -1707,8 +1713,9 @@ static BlockDriver bdrv_qcow2 = {
.bdrv_invalidate_cache = qcow2_invalidate_cache,
- .create_options = qcow2_create_options,
.bdrv_check = qcow2_check,
+
+ .bdrv_create_options = qcow2_create_options,
};
static void bdrv_qcow2_init(void)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6be20b1..566ef95 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -118,6 +118,19 @@
#define MAX_BLOCKSIZE 4096
+static QemuOptsList file_proto_create_opts = {
+ .name = "file-proto-create-opts",
+ .head = QTAILQ_HEAD_INITIALIZER(file_proto_create_opts.head),
+ .desc = {
+ {
+ .name = BLOCK_OPT_SIZE,
+ .type = QEMU_OPT_SIZE,
+ .help = "Virtual disk size"
+ },
+ { /* end of list */ }
+ }
+};
+
typedef struct BDRVRawState {
int fd;
int type;
@@ -558,19 +571,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_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
0644);
@@ -720,14 +728,10 @@ static coroutine_fn int raw_co_discard(BlockDriverState *bs,
return 0;
}
-static QEMUOptionParameter raw_create_options[] = {
- {
- .name = BLOCK_OPT_SIZE,
- .type = OPT_SIZE,
- .help = "Virtual disk size"
- },
- { NULL }
-};
+static QemuOptsList *raw_create_options(void)
+{
+ return &file_proto_create_opts;
+}
static BlockDriver bdrv_file = {
.format_name = "file",
@@ -748,8 +752,7 @@ static BlockDriver bdrv_file = {
.bdrv_getlength = raw_getlength,
.bdrv_get_allocated_file_size
= raw_get_allocated_file_size,
-
- .create_options = raw_create_options,
+ .bdrv_create_options = raw_create_options,
};
/***********************************************/
@@ -962,20 +965,15 @@ static int fd_open(BlockDriverState *bs)
#endif /* !linux && !FreeBSD */
-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_number(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
fd = qemu_open(filename, O_WRONLY | O_BINARY);
if (fd < 0)
@@ -999,23 +997,23 @@ static int hdev_has_zero_init(BlockDriverState *bs)
static BlockDriver bdrv_host_device = {
.format_name = "host_device",
- .protocol_name = "host_device",
+ .protocol_name = "host_device",
.instance_size = sizeof(BDRVRawState),
.bdrv_probe_device = hdev_probe_device,
.bdrv_file_open = hdev_open,
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
- .create_options = raw_create_options,
.bdrv_has_zero_init = hdev_has_zero_init,
- .bdrv_aio_readv = raw_aio_readv,
- .bdrv_aio_writev = raw_aio_writev,
- .bdrv_aio_flush = raw_aio_flush,
+ .bdrv_aio_readv = raw_aio_readv,
+ .bdrv_aio_writev = raw_aio_writev,
+ .bdrv_aio_flush = raw_aio_flush,
.bdrv_truncate = raw_truncate,
- .bdrv_getlength = raw_getlength,
+ .bdrv_getlength = raw_getlength,
.bdrv_get_allocated_file_size
= raw_get_allocated_file_size,
+ .bdrv_create_options = raw_create_options,
/* generic scsi device */
#ifdef __linux__
@@ -1126,7 +1124,6 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_file_open = floppy_open,
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
- .create_options = raw_create_options,
.bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_aio_readv = raw_aio_readv,
@@ -1142,6 +1139,7 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_is_inserted = floppy_is_inserted,
.bdrv_media_changed = floppy_media_changed,
.bdrv_eject = floppy_eject,
+ .bdrv_create_options = raw_create_options,
};
static int cdrom_open(BlockDriverState *bs, const char *filename, int flags)
@@ -1225,7 +1223,6 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_file_open = cdrom_open,
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
- .create_options = raw_create_options,
.bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_aio_readv = raw_aio_readv,
@@ -1245,6 +1242,8 @@ static BlockDriver bdrv_host_cdrom = {
/* generic scsi device */
.bdrv_ioctl = hdev_ioctl,
.bdrv_aio_ioctl = hdev_aio_ioctl,
+
+ .bdrv_create_options = raw_create_options,
};
#endif /* __linux__ */
@@ -1344,7 +1343,6 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_file_open = cdrom_open,
.bdrv_close = raw_close,
.bdrv_create = hdev_create,
- .create_options = raw_create_options,
.bdrv_has_zero_init = hdev_has_zero_init,
.bdrv_aio_readv = raw_aio_readv,
diff --git a/block/raw.c b/block/raw.c
index ff34ea4..5a64e7a 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -87,25 +87,34 @@ 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)
+static int raw_create(const char *filename, QemuOpts *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_has_zero_init(BlockDriverState *bs)
{
return bdrv_has_zero_init(bs->file);
}
+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_NUMBER,
+ .help = "Virtual disk size"
+ },
+ { /* end of list */ }
+ }
+};
+
+static QemuOptsList *raw_create_options(void)
+{
+ return &raw_create_opts;
+}
+
static BlockDriver bdrv_raw = {
.format_name = "raw",
@@ -133,8 +142,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_options = raw_create_options,
};
static void bdrv_raw_init(void)
diff --git a/block_int.h b/block_int.h
index 4452f6f..5794cb9 100644
--- a/block_int.h
+++ b/block_int.h
@@ -147,7 +147,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 *options);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
/* aio */
@@ -236,9 +236,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_options)(void);
/*
* Returns 0 for completed check, -errno for internal errors.
diff --git a/qemu-config.c b/qemu-config.c
index 6e3718e..bd92251 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -5,6 +5,8 @@
#include "hw/qdev.h"
#include "error.h"
+/*******************************************************************/
+
static QemuOptsList qemu_drive_opts = {
.name = "drive",
.head = QTAILQ_HEAD_INITIALIZER(qemu_drive_opts.head),
@@ -666,6 +668,7 @@ static QemuOptsList *vm_config_groups[32] = {
&qemu_boot_opts,
&qemu_iscsi_opts,
&qemu_sandbox_opts,
+
NULL,
};
diff --git a/qemu-img.c b/qemu-img.c
index f1c224e..c89de70 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -200,7 +200,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_options = NULL;
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
@@ -215,12 +215,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_options = append_opts_list(drv->bdrv_create_options(),
+ proto_drv->bdrv_create_options());
+ print_opts_list(create_options);
+ free_opts_list(create_options);
return 0;
}
@@ -271,19 +269,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;
@@ -670,8 +668,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 *param = NULL;
+ QemuOptsList *create_options = NULL;
+ const char *out_baseimg_param;
char *options = NULL;
const char *snapshot_name = NULL;
float local_progress;
@@ -806,40 +805,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_options = append_opts_list(drv->bdrv_create_options(),
+ proto_drv->bdrv_create_options());
if (options) {
- param = parse_option_parameters(options, create_options, param);
- if (param == NULL) {
+ if (qemu_opts_do_parse(param, 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);
+ param = qemu_opts_create(create_options, NULL, 0, NULL);
}
-
- set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
+ qemu_opt_set_number(param, BLOCK_OPT_SIZE, total_sectors * 512);
ret = add_old_style_options(out_fmt, param, 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(param, 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);
+ const char *encryption =
+ qemu_opt_get(param, BLOCK_OPT_ENCRYPT);
+ const char *preallocation =
+ qemu_opt_get(param, BLOCK_OPT_PREALLOC);
if (!drv->bdrv_write_compressed) {
error_report("Compression not supported for this file format");
@@ -847,15 +842,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");
@@ -1069,8 +1064,7 @@ static int img_convert(int argc, char **argv)
}
out:
qemu_progress_end();
- free_option_parameters(create_options);
- free_option_parameters(param);
+ free_opts_list(create_options);
qemu_vfree(buf);
if (out_bs) {
bdrv_delete(out_bs);
diff --git a/qemu-option.c b/qemu-option.c
index 2b52576..0e441e7 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -153,22 +153,6 @@ int check_params(char *buf, int buf_size,
return 0;
}
-/*
- * 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)
{
@@ -240,275 +224,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)
@@ -827,14 +542,37 @@ void qemu_opts_del(QemuOpts *opts)
int qemu_opts_print(QemuOpts *opts, void *dummy)
{
- QemuOpt *opt;
+ QemuOpt *opt = NULL;
+ 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);
+ while (desc && desc->name) {
+ opt = qemu_opt_find(opts, desc->name);
+ switch (desc->type) {
+ case QEMU_OPT_STRING:
+ if (opt != NULL) {
+ printf("%s='%s' ", opt->name, opt->str);
+ }
+ break;
+ case QEMU_OPT_BOOL:
+ printf("%s=%s ", desc->name, (opt && opt->str) ? "on" : "off");
+ break;
+ case QEMU_OPT_NUMBER:
+ case QEMU_OPT_SIZE:
+ if (strcmp(desc->name, "cluster_size")) {
+ printf("%s=%" PRId64 " ", desc->name,
+ (opt && opt->value.uint) ? opt->value.uint : 0);
+ } else {
+ printf("%s=%" PRId64 " ", desc->name,
+ (opt && opt->value.uint) ?
+ opt->value.uint : desc->def_value);
+ }
+ break;
+ default:
+ printf("%s=(unknown type) ", desc->name);
+ break;
+ }
+ desc++;
}
- fprintf(stderr, "\n");
return 0;
}
@@ -1097,3 +835,91 @@ 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 = 0;
+
+ while (list && list->desc[i].name) {
+ i++;
+ }
+
+ return i;
+}
+
+/* Create a new QemuOptsList and make its desc to the merge of first and second.
+ * It will allocate space for one nuew QemuOptsList plus enouth space for
+ * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc
+ * members take precedence over second's.
+ */
+QemuOptsList *append_opts_list(QemuOptsList *first,
+ QemuOptsList *second)
+{
+ size_t num_first_options, num_second_options;
+ QemuOptsList *dest = NULL;
+ int i = 0;
+ int index = 0;
+
+ num_first_options = count_opts_list(first);
+ num_second_options = count_opts_list(second);
+ if (num_first_options + num_second_options == 0) {
+ return NULL;
+ }
+
+ dest = g_malloc0(sizeof(QemuOptsList)
+ + (num_first_options + num_second_options) * sizeof(QemuOptDesc));
+
+ dest->name = "test";
+ dest->implied_opt_name = NULL;
+ dest->merge_lists = false;
+ QTAILQ_INIT(&dest->head);
+ while (first && (first->desc[i].name)) {
+ if (!find_desc_by_name(dest->desc, first->desc[i].name)) {
+ dest->desc[index].name = g_strdup(first->desc[i].name);
+ dest->desc[index].help = g_strdup(first->desc[i].help);
+ dest->desc[index].type = first->desc[i].type;
+ dest->desc[index].def_value = first->desc[i].def_value;
+ ++index;
+ }
+ i++;
+ }
+ i = 0;
+ while (second && (second->desc[i].name)) {
+ if (!find_desc_by_name(dest->desc, second->desc[i].name)) {
+ dest->desc[index].name = g_strdup(first->desc[i].name);
+ dest->desc[index].help = g_strdup(first->desc[i].help);
+ dest->desc[index].type = second->desc[i].type;
+ dest->desc[index].def_value = second->desc[i].def_value;
+ ++index;
+ }
+ i++;
+ }
+ dest->desc[index].name = NULL;
+ return dest;
+}
+
+void free_opts_list(QemuOptsList *list)
+{
+ int i = 0;
+
+ while (list && list->desc[i].name) {
+ g_free((char *)list->desc[i].name);
+ g_free((char *)list->desc[i].help);
+ i++;
+ }
+
+ g_free(list);
+}
+
+void print_opts_list(QemuOptsList *list)
+{
+ int i = 0;
+ printf("Supported options:\n");
+ while (list && list->desc[i].name) {
+ printf("%-16s %s\n", list->desc[i].name,
+ list->desc[i].help ?
+ list->desc[i].help : "No description available");
+ i++;
+ }
+}
+
diff --git a/qemu-option.h b/qemu-option.h
index 002dd07..be9c251 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -31,24 +31,6 @@
#include "error.h"
#include "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,
@@ -58,27 +40,6 @@ int get_param_value(char *buf, int buf_size,
int check_params(char *buf, int buf_size,
const char * const *params, 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;
@@ -96,6 +57,7 @@ typedef struct QemuOptDesc {
const char *name;
enum QemuOptType type;
const char *help;
+ uint64_t def_value;
} QemuOptDesc;
struct QemuOptsList {
@@ -154,5 +116,8 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
int qemu_opts_print(QemuOpts *opts, void *dummy);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
int abort_on_failure);
-
+QemuOptsList *append_opts_list(QemuOptsList *dest,
+ QemuOptsList *list);
+void free_opts_list(QemuOptsList *list);
+void print_opts_list(QemuOptsList *list);
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication Dong Xu Wang
@ 2012-09-27 11:55 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2012-09-27 11:55 UTC (permalink / raw)
To: Dong Xu Wang; +Cc: kwolf, lcapitulino, qemu-devel, armbru
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
> Call qemu_opt_set() instead of duplicating opt_set().
>
> It will set opt->str in qemu_opt_set_bool, without opt->str, there
> will be some potential bugs.
>
> These are uses of opt->str, and what happens when it isn't set:
>
> * qemu_opt_get(): returns NULL, which means "not set". Bug can bite
> when value isn't the default value.
>
> * qemu_opt_parse(): passes NULL to parse_option_bool(), which treats it
> like "on". Wrong if the value is actually false. Bug can bite when
> qemu_opts_validate() runs after qemu_opt_set_bool().
>
> * qemu_opt_del(): passes NULL to g_free(), which is just fine.
>
> * qemu_opt_foreach(): passes NULL to the callback, which is unlikely to
> be prepared for it.
>
> * qemu_opts_print(): prints NULL, which crashes on some systems.
>
> * qemu_opts_to_qdict(): passes NULL to qstring_from_str(), which
> crashes.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> ---
> qemu-option.c | 28 +---------------------------
> 1 files changed, 1 insertions(+), 27 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index 27891e7..0b81e77 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -667,33 +667,7 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
>
> int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
> {
> - QemuOpt *opt;
> - const QemuOptDesc *desc = opts->list->desc;
> - int i;
> -
> - for (i = 0; desc[i].name != NULL; i++) {
> - if (strcmp(desc[i].name, name) == 0) {
> - break;
> - }
> - }
> - if (desc[i].name == NULL) {
> - if (i == 0) {
> - /* empty list -> allow any */;
> - } else {
> - qerror_report(QERR_INVALID_PARAMETER, name);
> - return -1;
> - }
> - }
> -
> - opt = g_malloc0(sizeof(*opt));
> - opt->name = g_strdup(name);
> - opt->opts = opts;
> - QTAILQ_INSERT_TAIL(&opts->head, opt, next);
> - if (desc[i].name != NULL) {
> - opt->desc = desc+i;
> - }
> - opt->value.boolean = !!val;
> - return 0;
> + return qemu_opt_set(opts, name, val ? "on" : "off");
This now calls qemu_opt_parse, which won't work if you have opt->desc =
NULL.
Instead of this patch, using the new functions you create in patch 2
should be enough to limit the code duplication in qemu_opt_set_bool.
Paolo
> }
>
> int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number Dong Xu Wang
@ 2012-09-27 11:56 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2012-09-27 11:56 UTC (permalink / raw)
To: Dong Xu Wang; +Cc: kwolf, lcapitulino, qemu-devel, armbru
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
> +int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val)
> +{
> + char buffer[1024];
> + snprintf(buffer, sizeof(buffer), "%" PRId64, val);
> + return qemu_opt_set(opts, name, buffer);
> +}
This has the same problem as qemu_opt_set_bool after patch 1.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter Dong Xu Wang
@ 2012-09-27 12:03 ` Paolo Bonzini
2012-09-28 6:42 ` Dong Xu Wang
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2012-09-27 12:03 UTC (permalink / raw)
To: Dong Xu Wang; +Cc: kwolf, lcapitulino, qemu-devel, armbru
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
> remove QEMUOptionParameter, and use QemuOpts and QemuOptsList.
>
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> ---
> block.c | 88 ++++++------
> block.h | 5 +-
> block/Makefile.objs | 9 +-
> block/qcow2.c | 175 ++++++++++++-----------
> block/raw-posix.c | 68 ++++-----
> block/raw.c | 31 +++--
> block_int.h | 6 +-
> qemu-config.c | 3 +
> qemu-img.c | 58 ++++----
> qemu-option.c | 408 +++++++++++++++------------------------------------
> qemu-option.h | 45 +-----
> 11 files changed, 347 insertions(+), 549 deletions(-)
The patch is already quite big, so please move the qemu-option.c changes
to separate patches.
For example, patch 7 could add def_value and use it in qemu_opts_print.
Patch 8 should add append_opts_list, free_opts_list, print_opts_list.
Patch 9 should touch only the block layer. Patch 10 should remove the
now-unuse QEMUOptionParameter code.
(Regarding def_value, it is quite unintuitive that you need to specify
the value again when calling qemu_opt_get_*. Perhaps,
qemu_opts_validate could instead walk through descriptors that are not
present but have a default value, and add new options with the default
value to the QemuOpts object).
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter
2012-09-27 12:03 ` Paolo Bonzini
@ 2012-09-28 6:42 ` Dong Xu Wang
0 siblings, 0 replies; 12+ messages in thread
From: Dong Xu Wang @ 2012-09-28 6:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, armbru, qemu-devel, lcapitulino
On Thu, Sep 27, 2012 at 8:03 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
>> remove QEMUOptionParameter, and use QemuOpts and QemuOptsList.
>>
>> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
>> ---
>> block.c | 88 ++++++------
>> block.h | 5 +-
>> block/Makefile.objs | 9 +-
>> block/qcow2.c | 175 ++++++++++++-----------
>> block/raw-posix.c | 68 ++++-----
>> block/raw.c | 31 +++--
>> block_int.h | 6 +-
>> qemu-config.c | 3 +
>> qemu-img.c | 58 ++++----
>> qemu-option.c | 408 +++++++++++++++------------------------------------
>> qemu-option.h | 45 +-----
>> 11 files changed, 347 insertions(+), 549 deletions(-)
>
> The patch is already quite big, so please move the qemu-option.c changes
> to separate patches.
>
> For example, patch 7 could add def_value and use it in qemu_opts_print.
> Patch 8 should add append_opts_list, free_opts_list, print_opts_list.
> Patch 9 should touch only the block layer. Patch 10 should remove the
> now-unuse QEMUOptionParameter code.
>
> (Regarding def_value, it is quite unintuitive that you need to specify
> the value again when calling qemu_opt_get_*. Perhaps,
> qemu_opts_validate could instead walk through descriptors that are not
> present but have a default value, and add new options with the default
> value to the QemuOpts object).
>
> Paolo
>
>
Thank you Paolo, I will take a week's vocation, so I will fix it once
I am back.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-09-28 6:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27 5:14 [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication Dong Xu Wang
2012-09-27 11:55 ` Paolo Bonzini
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 2/7] qemu-option: opt_set(): split it up into more functions Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 3/7] qemu-option: qemu_opts_validate(): fix duplicated code Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 4/7] introduce qemu_opts_create_nofail function Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 5/7] use qemu_opts_create_nofail Dong Xu Wang
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number Dong Xu Wang
2012-09-27 11:56 ` Paolo Bonzini
2012-09-27 5:14 ` [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter Dong Xu Wang
2012-09-27 12:03 ` Paolo Bonzini
2012-09-28 6:42 ` 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).