* [Qemu-devel] [PATCH 01/13] qemu-option: qemu_opts_create(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 02/13] qemu-option: parse_option_number(): " Luiz Capitulino
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
This commit converts qemu_opts_create() from qerror_report() to
error_set().
This means that qemu_opts_create() now takes an Error argument and
callers need to pass it if they're interested in getting error
information.
Currently, most calls to qemu_opts_create() can't fail, so most
callers don't need any changes.
The two cases where code checks for qemu_opts_create() erros are:
1. Initialization code in vl.c. All of them print their own
error messages directly to stderr, no need to pass the Error
object
2. The functions opts_parse(), qemu_opts_from_qdict() and
qemu_chr_parse_compat() make use of the error information and
they can be called from HMP or QMP. In this case, to allow for
incremental conversion, we propagate the error up using
qerror_report_err(), which keep the QError semantics
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
blockdev.c | 2 +-
hw/usb/dev-storage.c | 2 +-
hw/watchdog.c | 2 +-
qemu-char.c | 8 ++++++--
qemu-config.c | 6 +++---
qemu-option.c | 37 +++++++++++++++++++++++++++----------
qemu-option.h | 4 +++-
qemu-sockets.c | 8 ++++----
vl.c | 22 +++++++++++++---------
9 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 1a500b8..77be66a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -565,7 +565,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);
+ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
if (arch_type == QEMU_ARCH_S390X) {
qemu_opt_set(opts, "driver", "virtio-blk-s390");
} else {
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index bdbe7bd..c9c59be 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -580,7 +580,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
/* parse -usbdevice disk: syntax into drive opts */
snprintf(id, sizeof(id), "usb%d", nr++);
- opts = qemu_opts_create(qemu_find_opts("drive"), id, 0);
+ opts = qemu_opts_create(qemu_find_opts("drive"), id, 0, NULL);
p1 = strchr(filename, ':');
if (p1++) {
diff --git a/hw/watchdog.c b/hw/watchdog.c
index 4c18965..a42124d 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);
+ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
qemu_opt_set(opts, "driver", p);
return 0;
}
diff --git a/qemu-char.c b/qemu-char.c
index bb9e3f5..67a5ff0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2581,10 +2581,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
int pos;
const char *p;
QemuOpts *opts;
+ Error *local_err = NULL;
- opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1);
- if (NULL == opts)
+ opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return NULL;
+ }
if (strstart(filename, "mon:", &p)) {
filename = p;
diff --git a/qemu-config.c b/qemu-config.c
index be84a03..f876646 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -709,7 +709,7 @@ int qemu_global_option(const char *str)
return -1;
}
- opts = qemu_opts_create(&qemu_global_opts, NULL, 0);
+ opts = qemu_opts_create(&qemu_global_opts, NULL, 0, NULL);
qemu_opt_set(opts, "driver", driver);
qemu_opt_set(opts, "property", property);
qemu_opt_set(opts, "value", str+offset+1);
@@ -781,7 +781,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
list = find_list(lists, group);
if (list == NULL)
goto out;
- opts = qemu_opts_create(list, id, 1);
+ opts = qemu_opts_create(list, id, 1, NULL);
continue;
}
if (sscanf(line, "[%63[^]]]", group) == 1) {
@@ -789,7 +789,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
list = find_list(lists, group);
if (list == NULL)
goto out;
- opts = qemu_opts_create(list, NULL, 0);
+ opts = qemu_opts_create(list, NULL, 0, NULL);
continue;
}
if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/qemu-option.c b/qemu-option.c
index 35cd609..9f531c8 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -30,6 +30,7 @@
#include "qemu-error.h"
#include "qemu-objects.h"
#include "qemu-option.h"
+#include "error.h"
#include "qerror.h"
/*
@@ -729,20 +730,21 @@ static int id_wellformed(const char *id)
return 1;
}
-QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists)
+QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
+ int fail_if_exists, Error **errp)
{
QemuOpts *opts = NULL;
if (id) {
if (!id_wellformed(id)) {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+ error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n");
return NULL;
}
opts = qemu_opts_find(list, id);
if (opts != NULL) {
if (fail_if_exists && !list->merge_lists) {
- qerror_report(QERR_DUPLICATE_ID, id, list->name);
+ error_set(errp, QERR_DUPLICATE_ID, id, list->name);
return NULL;
} else {
return opts;
@@ -783,9 +785,12 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
const char *name, const char *value)
{
QemuOpts *opts;
+ Error *local_err = NULL;
- opts = qemu_opts_create(list, id, 1);
- if (opts == NULL) {
+ opts = qemu_opts_create(list, id, 1, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return -1;
}
return qemu_opt_set(opts, name, value);
@@ -883,6 +888,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
char value[1024], *id = NULL;
const char *p;
QemuOpts *opts;
+ Error *local_err = NULL;
assert(!permit_abbrev || list->implied_opt_name);
firstname = permit_abbrev ? list->implied_opt_name : NULL;
@@ -898,13 +904,18 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
if (!id && !QTAILQ_EMPTY(&list->head)) {
opts = qemu_opts_find(list, NULL);
} else {
- opts = qemu_opts_create(list, id, 0);
+ opts = qemu_opts_create(list, id, 0, &local_err);
}
} else {
- opts = qemu_opts_create(list, id, 1);
+ opts = qemu_opts_create(list, id, 1, &local_err);
}
- if (opts == NULL)
+ if (opts == NULL) {
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
return NULL;
+ }
if (opts_do_parse(opts, params, firstname, defaults) != 0) {
qemu_opts_del(opts);
@@ -975,11 +986,17 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict)
{
QemuOpts *opts;
+ Error *local_err = NULL;
- opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1);
- if (opts == NULL)
+ opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1,
+ &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return NULL;
+ }
+ assert(opts != NULL);
qdict_iter(qdict, qemu_opts_from_qdict_1, opts);
return opts;
}
diff --git a/qemu-option.h b/qemu-option.h
index 3ca00c3..4d5b3d3 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -28,6 +28,7 @@
#include <stdint.h>
#include "qemu-queue.h"
+#include "error.h"
#include "qdict.h"
enum QEMUOptionParType {
@@ -116,7 +117,8 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
int abort_on_failure);
QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
-QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists);
+QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
+ int fail_if_exists, Error **errp);
void qemu_opts_reset(QemuOptsList *list);
void qemu_opts_loc_restore(QemuOpts *opts);
int qemu_opts_set(QemuOptsList *list, const char *id,
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 6bcb8e3..5966d5f 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -427,7 +427,7 @@ int inet_listen(const char *str, char *ostr, int olen,
char *optstr;
int sock = -1;
- opts = qemu_opts_create(&dummy_opts, NULL, 0);
+ opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
if (inet_parse(opts, str) == 0) {
sock = inet_listen_opts(opts, port_offset);
if (sock != -1 && ostr) {
@@ -454,7 +454,7 @@ int inet_connect(const char *str, int socktype)
QemuOpts *opts;
int sock = -1;
- opts = qemu_opts_create(&dummy_opts, NULL, 0);
+ opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
if (inet_parse(opts, str) == 0)
sock = inet_connect_opts(opts);
qemu_opts_del(opts);
@@ -547,7 +547,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);
+ opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
optstr = strchr(str, ',');
if (optstr) {
@@ -575,7 +575,7 @@ int unix_connect(const char *path)
QemuOpts *opts;
int sock;
- opts = qemu_opts_create(&dummy_opts, NULL, 0);
+ opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
qemu_opt_set(opts, "path", path);
sock = unix_connect_opts(opts);
qemu_opts_del(opts);
diff --git a/vl.c b/vl.c
index 0fccf50..6467d6b 100644
--- a/vl.c
+++ b/vl.c
@@ -1772,7 +1772,7 @@ static int balloon_parse(const char *arg)
return -1;
} else {
/* create empty opts */
- opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
}
qemu_opt_set(opts, "driver", "virtio-balloon");
return 0;
@@ -1907,7 +1907,7 @@ static void monitor_parse(const char *optarg, const char *mode)
}
}
- opts = qemu_opts_create(qemu_find_opts("mon"), label, 1);
+ opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, NULL);
if (!opts) {
fprintf(stderr, "duplicate chardev: %s\n", label);
exit(1);
@@ -2021,14 +2021,14 @@ static int virtcon_parse(const char *devname)
exit(1);
}
- bus_opts = qemu_opts_create(device, NULL, 0);
+ bus_opts = qemu_opts_create(device, NULL, 0, NULL);
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);
+ dev_opts = qemu_opts_create(device, NULL, 0, NULL);
qemu_opt_set(dev_opts, "driver", "virtconsole");
snprintf(label, sizeof(label), "virtcon%d", index);
@@ -2051,7 +2051,7 @@ static int debugcon_parse(const char *devname)
if (!qemu_chr_new("debugcon", devname, NULL)) {
exit(1);
}
- opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1);
+ opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL);
if (!opts) {
fprintf(stderr, "qemu: already have a debugcon device\n");
exit(1);
@@ -2799,7 +2799,8 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
- qemu_opt_get(opts, "mount_tag"), 1);
+ qemu_opt_get(opts, "mount_tag"),
+ 1, NULL);
if (!fsdev) {
fprintf(stderr, "duplicate fsdev id: %s\n",
qemu_opt_get(opts, "mount_tag"));
@@ -2831,7 +2832,8 @@ int main(int argc, char **argv, char **envp)
qemu_opt_set_bool(fsdev, "readonly",
qemu_opt_get_bool(opts, "readonly", 0));
- device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+ device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ NULL);
qemu_opt_set(device, "driver", "virtio-9p-pci");
qemu_opt_set(device, "fsdev",
qemu_opt_get(opts, "mount_tag"));
@@ -2843,14 +2845,16 @@ int main(int argc, char **argv, char **envp)
QemuOpts *fsdev;
QemuOpts *device;
- fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth", 1);
+ fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth",
+ 1, NULL);
if (!fsdev) {
fprintf(stderr, "duplicate option: %s\n", "virtfs_synth");
exit(1);
}
qemu_opt_set(fsdev, "fsdriver", "synth");
- device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+ device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ NULL);
qemu_opt_set(device, "driver", "virtio-9p-pci");
qemu_opt_set(device, "fsdev", "v_synth");
qemu_opt_set(device, "mount_tag", "v_synth");
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 02/13] qemu-option: parse_option_number(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 01/13] qemu-option: qemu_opts_create(): use error_set() Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 03/13] qemu-option: parse_option_bool(): " Luiz Capitulino
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
Note that qemu_opt_parse() callers still expect automatic error reporting
with QError, so qemu_opts_parse() calls qerror_report_err() to keep the
same semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 9f531c8..72dcb56 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -186,7 +186,8 @@ static int parse_option_bool(const char *name, const char *value, bool *ret)
return 0;
}
-static int parse_option_number(const char *name, const char *value, uint64_t *ret)
+static void parse_option_number(const char *name, const char *value,
+ uint64_t *ret, Error **errp)
{
char *postfix;
uint64_t number;
@@ -194,15 +195,13 @@ static int parse_option_number(const char *name, const char *value, uint64_t *re
if (value != NULL) {
number = strtoull(value, &postfix, 0);
if (*postfix != '\0') {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
- return -1;
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
+ return;
}
*ret = number;
} else {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
- return -1;
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
}
- return 0;
}
static int parse_option_size(const char *name, const char *value, uint64_t *ret)
@@ -579,8 +578,11 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
static int qemu_opt_parse(QemuOpt *opt)
{
+ Error *local_err = NULL;
+
if (opt->desc == NULL)
return 0;
+
switch (opt->desc->type) {
case QEMU_OPT_STRING:
/* nothing */
@@ -588,12 +590,22 @@ static int qemu_opt_parse(QemuOpt *opt)
case QEMU_OPT_BOOL:
return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
case QEMU_OPT_NUMBER:
- return parse_option_number(opt->name, opt->str, &opt->value.uint);
+ parse_option_number(opt->name, opt->str, &opt->value.uint,
+ &local_err);
+ break;
case QEMU_OPT_SIZE:
return parse_option_size(opt->name, opt->str, &opt->value.uint);
default:
abort();
}
+
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
+ }
+
+ return 0;
}
static void qemu_opt_del(QemuOpt *opt)
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 03/13] qemu-option: parse_option_bool(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 01/13] qemu-option: qemu_opts_create(): use error_set() Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 02/13] qemu-option: parse_option_number(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 04/13] qemu-option: parse_option_size(): " Luiz Capitulino
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
Note that set_option_parameter() callers still expect automatic error
reporting with QError, so set_option_parameter() calls
qerror_report_err() to keep the same semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 72dcb56..a8b50af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -169,7 +169,8 @@ QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list,
return NULL;
}
-static int parse_option_bool(const char *name, const char *value, bool *ret)
+static void parse_option_bool(const char *name, const char *value, bool *ret,
+ Error **errp)
{
if (value != NULL) {
if (!strcmp(value, "on")) {
@@ -177,13 +178,11 @@ static int parse_option_bool(const char *name, const char *value, bool *ret)
} else if (!strcmp(value, "off")) {
*ret = 0;
} else {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
- return -1;
+ error_set(errp,QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
}
} else {
*ret = 1;
}
- return 0;
}
static void parse_option_number(const char *name, const char *value,
@@ -263,6 +262,7 @@ 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);
@@ -274,8 +274,10 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
// Process parameter
switch (list->type) {
case OPT_FLAG:
- if (parse_option_bool(name, value, &flag) == -1)
- return -1;
+ parse_option_bool(name, value, &flag, &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
+ }
list->value.n = flag;
break;
@@ -299,6 +301,11 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
}
return 0;
+
+exit_err:
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
}
/*
@@ -588,7 +595,8 @@ static int qemu_opt_parse(QemuOpt *opt)
/* nothing */
return 0;
case QEMU_OPT_BOOL:
- return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
+ parse_option_bool(opt->name, opt->str, &opt->value.boolean, &local_err);
+ break;
case QEMU_OPT_NUMBER:
parse_option_number(opt->name, opt->str, &opt->value.uint,
&local_err);
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 04/13] qemu-option: parse_option_size(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (2 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 03/13] qemu-option: parse_option_bool(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 05/13] qemu-option: qemu_opt_parse(): " Luiz Capitulino
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index a8b50af..61354af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -203,7 +203,8 @@ static void parse_option_number(const char *name, const char *value,
}
}
-static int parse_option_size(const char *name, const char *value, uint64_t *ret)
+static void parse_option_size(const char *name, const char *value,
+ uint64_t *ret, Error **errp)
{
char *postfix;
double sizef;
@@ -229,16 +230,14 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
*ret = (uint64_t) sizef;
break;
default:
- qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
error_printf_unless_qmp("You may use k, M, G or T suffixes for "
"kilobytes, megabytes, gigabytes and terabytes.\n");
- return -1;
+ return;
}
} else {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
- return -1;
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
}
- return 0;
}
/*
@@ -291,8 +290,10 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
break;
case OPT_SIZE:
- if (parse_option_size(name, value, &list->value.n) == -1)
- return -1;
+ parse_option_size(name, value, &list->value.n, &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
+ }
break;
default:
@@ -602,7 +603,8 @@ static int qemu_opt_parse(QemuOpt *opt)
&local_err);
break;
case QEMU_OPT_SIZE:
- return parse_option_size(opt->name, opt->str, &opt->value.uint);
+ parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+ break;
default:
abort();
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 05/13] qemu-option: qemu_opt_parse(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (3 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 04/13] qemu-option: parse_option_size(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 06/13] qemu-option: opt_set(): " Luiz Capitulino
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
The functions opt_set() and qemu_opts_validate() both call qemu_opt_parse(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 54 +++++++++++++++++++++++++++++-------------------------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 61354af..4829de5 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -584,38 +584,27 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval)
return opt->value.uint;
}
-static int qemu_opt_parse(QemuOpt *opt)
+static void qemu_opt_parse(QemuOpt *opt, Error **errp)
{
- Error *local_err = NULL;
-
if (opt->desc == NULL)
- return 0;
+ return;
switch (opt->desc->type) {
case QEMU_OPT_STRING:
/* nothing */
- return 0;
+ return;
case QEMU_OPT_BOOL:
- parse_option_bool(opt->name, opt->str, &opt->value.boolean, &local_err);
+ parse_option_bool(opt->name, opt->str, &opt->value.boolean, errp);
break;
case QEMU_OPT_NUMBER:
- parse_option_number(opt->name, opt->str, &opt->value.uint,
- &local_err);
+ parse_option_number(opt->name, opt->str, &opt->value.uint, errp);
break;
case QEMU_OPT_SIZE:
- parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+ parse_option_size(opt->name, opt->str, &opt->value.uint, errp);
break;
default:
abort();
}
-
- if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
- return -1;
- }
-
- return 0;
}
static void qemu_opt_del(QemuOpt *opt)
@@ -631,6 +620,7 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
{
QemuOpt *opt;
const QemuOptDesc *desc = opts->list->desc;
+ Error *local_err = NULL;
int i;
for (i = 0; desc[i].name != NULL; i++) {
@@ -642,8 +632,8 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
if (i == 0) {
/* empty list -> allow any */;
} else {
- qerror_report(QERR_INVALID_PARAMETER, name);
- return -1;
+ error_set(&local_err, QERR_INVALID_PARAMETER, name);
+ goto exit_err;
}
}
@@ -661,11 +651,18 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
if (value) {
opt->str = g_strdup(value);
}
- if (qemu_opt_parse(opt) < 0) {
+ qemu_opt_parse(opt, &local_err);
+ if (error_is_set(&local_err)) {
qemu_opt_del(opt);
- return -1;
+ goto exit_err;
}
+
return 0;
+
+exit_err:
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
}
int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
@@ -1053,6 +1050,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
{
QemuOpt *opt;
+ Error *local_err = NULL;
assert(opts->list->desc[0].name == NULL);
@@ -1065,18 +1063,24 @@ int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
}
}
if (desc[i].name == NULL) {
- qerror_report(QERR_INVALID_PARAMETER, opt->name);
- return -1;
+ error_set(&local_err, QERR_INVALID_PARAMETER, opt->name);
+ goto exit_err;
}
opt->desc = &desc[i];
- if (qemu_opt_parse(opt) < 0) {
- return -1;
+ qemu_opt_parse(opt, &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
}
}
return 0;
+
+exit_err:
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
}
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 06/13] qemu-option: opt_set(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (4 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 05/13] qemu-option: qemu_opt_parse(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 07/13] qemu-option: opts_do_parse(): " Luiz Capitulino
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
The functions qemu_opt_set() and opts_do_parse() both call opt_set(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 4829de5..38461d4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -615,12 +615,11 @@ static void qemu_opt_del(QemuOpt *opt)
g_free(opt);
}
-static int opt_set(QemuOpts *opts, const char *name, const char *value,
- bool prepend)
+static void opt_set(QemuOpts *opts, const char *name, const char *value,
+ bool prepend, Error **errp)
{
QemuOpt *opt;
const QemuOptDesc *desc = opts->list->desc;
- Error *local_err = NULL;
int i;
for (i = 0; desc[i].name != NULL; i++) {
@@ -632,8 +631,8 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
if (i == 0) {
/* empty list -> allow any */;
} else {
- error_set(&local_err, QERR_INVALID_PARAMETER, name);
- goto exit_err;
+ error_set(errp, QERR_INVALID_PARAMETER, name);
+ return;
}
}
@@ -651,23 +650,24 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
if (value) {
opt->str = g_strdup(value);
}
- qemu_opt_parse(opt, &local_err);
- if (error_is_set(&local_err)) {
+ qemu_opt_parse(opt, errp);
+ if (error_is_set(errp)) {
qemu_opt_del(opt);
- goto exit_err;
}
-
- return 0;
-
-exit_err:
- qerror_report_err(local_err);
- error_free(local_err);
- return -1;
}
int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
{
- return opt_set(opts, name, value, false);
+ Error *local_err = NULL;
+
+ opt_set(opts, name, value, false, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
+ }
+
+ return 0;
}
int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
@@ -853,6 +853,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params,
{
char option[128], value[1024];
const char *p,*pe,*pc;
+ Error *local_err = NULL;
for (p = params; *p != '\0'; p++) {
pe = strchr(p, '=');
@@ -884,7 +885,10 @@ static int opts_do_parse(QemuOpts *opts, const char *params,
}
if (strcmp(option, "id") != 0) {
/* store and parse */
- if (opt_set(opts, option, value, prepend) == -1) {
+ opt_set(opts, option, value, prepend, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return -1;
}
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 07/13] qemu-option: opts_do_parse(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (5 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 06/13] qemu-option: opt_set(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 08/13] qemu-option: introduce qemu_opt_set_err() Luiz Capitulino
` (5 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
The functions qemu_opts_do_parse() and opts_parse() both call
opts_do_parse(), but their callers expect QError semantics. Thus,
both functions call qerro_report_err() to keep the expected semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 38461d4..666eacd 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -848,12 +848,11 @@ int qemu_opts_print(QemuOpts *opts, void *dummy)
return 0;
}
-static int opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname, bool prepend)
+static void opts_do_parse(QemuOpts *opts, const char *params,
+ const char *firstname, bool prepend, Error **errp)
{
char option[128], value[1024];
const char *p,*pe,*pc;
- Error *local_err = NULL;
for (p = params; *p != '\0'; p++) {
pe = strchr(p, '=');
@@ -885,23 +884,29 @@ 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);
- if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
- return -1;
+ opt_set(opts, option, value, prepend, errp);
+ if (error_is_set(errp)) {
+ return;
}
}
if (*p != ',') {
break;
}
}
- return 0;
}
int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname)
{
- return opts_do_parse(opts, params, firstname, false);
+ Error *local_err = NULL;
+
+ opts_do_parse(opts, params, firstname, false, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
+ }
+
+ return 0;
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
@@ -934,18 +939,23 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
}
if (opts == NULL) {
if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
+ goto exit_err;
}
return NULL;
}
- if (opts_do_parse(opts, params, firstname, defaults) != 0) {
+ opts_do_parse(opts, params, firstname, defaults, &local_err);
+ if (error_is_set(&local_err)) {
qemu_opts_del(opts);
- return NULL;
+ goto exit_err;
}
return opts;
+
+exit_err:
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return NULL;
}
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 08/13] qemu-option: introduce qemu_opt_set_err()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (6 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 07/13] qemu-option: opts_do_parse(): " Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 09/13] qerror: introduce QERR_INVALID_OPTION_GROUP Luiz Capitulino
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
This is like qemu_opt_set(), except that it takes an Error argument.
This new function allows for a incremental conversion of code using
qemu_opt_set().
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-option.c | 6 ++++++
qemu-option.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/qemu-option.c b/qemu-option.c
index 666eacd..881467d 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -670,6 +670,12 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
return 0;
}
+void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
+ Error **errp)
+{
+ opt_set(opts, name, value, false, errp);
+}
+
int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
{
QemuOpt *opt;
diff --git a/qemu-option.h b/qemu-option.h
index 4d5b3d3..52e9ec1 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -111,6 +111,8 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
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);
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,
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 09/13] qerror: introduce QERR_INVALID_OPTION_GROUP
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (7 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 08/13] qemu-option: introduce qemu_opt_set_err() Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 10/13] qemu-config: find_list(): use error_set() Luiz Capitulino
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qerror.c | 4 ++++
qerror.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/qerror.c b/qerror.c
index 41c729a..02e3a14 100644
--- a/qerror.c
+++ b/qerror.c
@@ -156,6 +156,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Invalid block format '%(name)'",
},
{
+ .error_fmt = QERR_INVALID_OPTION_GROUP,
+ .desc = "There is no option group '%(group)'",
+ },
+ {
.error_fmt = QERR_INVALID_PARAMETER,
.desc = "Invalid parameter '%(name)'",
},
diff --git a/qerror.h b/qerror.h
index e16f9c2..c7eeee9 100644
--- a/qerror.h
+++ b/qerror.h
@@ -139,6 +139,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_INVALID_BLOCK_FORMAT \
"{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
+#define QERR_INVALID_OPTION_GROUP \
+ "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+
#define QERR_INVALID_PARAMETER \
"{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 10/13] qemu-config: find_list(): use error_set()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (8 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 09/13] qerror: introduce QERR_INVALID_OPTION_GROUP Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 11/13] qemu-config: introduce qemu_find_opts_err() Luiz Capitulino
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
Note that qemu_find_opts() callers still expect automatic error reporting
with QError, so qemu_find_opts() calls qerror_report_err() to keep the
same semantics.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-config.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index f876646..bdb381d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -3,6 +3,7 @@
#include "qemu-option.h"
#include "qemu-config.h"
#include "hw/qdev.h"
+#include "error.h"
static QemuOptsList qemu_drive_opts = {
.name = "drive",
@@ -631,7 +632,8 @@ static QemuOptsList *vm_config_groups[32] = {
NULL,
};
-static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
+static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
+ Error **errp)
{
int i;
@@ -640,14 +642,23 @@ static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
break;
}
if (lists[i] == NULL) {
- error_report("there is no option group \"%s\"", group);
+ error_set(errp, QERR_INVALID_OPTION_GROUP, group);
}
return lists[i];
}
QemuOptsList *qemu_find_opts(const char *group)
{
- return find_list(vm_config_groups, group);
+ QemuOptsList *ret;
+ Error *local_err = NULL;
+
+ ret = find_list(vm_config_groups, group, &local_err);
+ if (error_is_set(&local_err)) {
+ error_report("%s\n", error_get_pretty(local_err));
+ error_free(local_err);
+ }
+
+ return ret;
}
void qemu_add_opts(QemuOptsList *list)
@@ -762,6 +773,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
char line[1024], group[64], id[64], arg[64], value[1024];
Location loc;
QemuOptsList *list = NULL;
+ Error *local_err = NULL;
QemuOpts *opts = NULL;
int res = -1, lno = 0;
@@ -778,17 +790,23 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
}
if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
/* group with id */
- list = find_list(lists, group);
- if (list == NULL)
+ list = find_list(lists, group, &local_err);
+ if (error_is_set(&local_err)) {
+ error_report("%s\n", error_get_pretty(local_err));
+ error_free(local_err);
goto out;
+ }
opts = qemu_opts_create(list, id, 1, NULL);
continue;
}
if (sscanf(line, "[%63[^]]]", group) == 1) {
/* group without id */
- list = find_list(lists, group);
- if (list == NULL)
+ list = find_list(lists, group, &local_err);
+ if (error_is_set(&local_err)) {
+ error_report("%s\n", error_get_pretty(local_err));
+ error_free(local_err);
goto out;
+ }
opts = qemu_opts_create(list, NULL, 0, NULL);
continue;
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 11/13] qemu-config: introduce qemu_find_opts_err()
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (9 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 10/13] qemu-config: find_list(): use error_set() Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list Luiz Capitulino
2012-03-29 17:26 ` [Qemu-devel] [PATCH 13/13] qapi: convert device_add Luiz Capitulino
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
This is like qemu_find_opts(), except that it takes an Error argument.
This new function allows for a incremental conversion of code using
qemu_find_opts().
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qemu-config.c | 5 +++++
qemu-config.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/qemu-config.c b/qemu-config.c
index bdb381d..bb3bff4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -661,6 +661,11 @@ QemuOptsList *qemu_find_opts(const char *group)
return ret;
}
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp)
+{
+ return find_list(vm_config_groups, group, errp);
+}
+
void qemu_add_opts(QemuOptsList *list)
{
int entries, i;
diff --git a/qemu-config.h b/qemu-config.h
index 20d707f..a093e3f 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1,11 +1,14 @@
#ifndef QEMU_CONFIG_H
#define QEMU_CONFIG_H
+#include "error.h"
+
extern QemuOptsList qemu_fsdev_opts;
extern QemuOptsList qemu_virtfs_opts;
extern QemuOptsList qemu_spice_opts;
QemuOptsList *qemu_find_opts(const char *group);
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
void qemu_add_opts(QemuOptsList *list);
int qemu_set_option(const char *str);
int qemu_global_option(const char *str);
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (10 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 11/13] qemu-config: introduce qemu_find_opts_err() Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
2012-03-29 18:26 ` Anthony Liguori
2012-03-29 17:26 ` [Qemu-devel] [PATCH 13/13] qapi: convert device_add Luiz Capitulino
12 siblings, 1 reply; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
This allows for QAPI functions to receive a variable-length argument
list. This is going to be used by device_add and netdev_add commands.
In the schema, the argument list is represented by type name '**',
like this example:
{ 'command': 'foo', 'data': { 'arg-list': '**' } }
Each argument is represented by the KeyValues type and the C
implementation should expect a KeyValuesList, like:
void qmp_foo(KeyValuesList *values_list, Error **errp);
XXX: This implementation is simple but very hacky. We just iterate
through all arguments and build the KeyValuesList list to be
passed to the QAPI function.
Maybe we could have a kwargs type, that does exactly this but
through a visitor instead?
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi-schema.json | 15 +++++++++++++++
scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
scripts/qapi.py | 2 ++
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..25bd487 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1701,3 +1701,18 @@
# Since: 1.1
##
{ 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
+
+##
+# @KeyValues:
+#
+# A generic representation of a key value pair.
+#
+# @key: the name of the item
+#
+# @value: the string representation of the item value. This typically follows
+# QEMU's command line parsing format. See the man pages for more
+# information.
+#
+# Since: 0.14.0
+##
+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 30a24d2..75a6e81 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
obj=obj)
for argname, argtype, optional, structured in parse_args(args):
- if optional:
+ if optional and not '**':
ret += mcgen('''
visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
if (has_%(c_name)s) {
''',
c_name=c_var(argname), name=argname)
push_indent()
- ret += mcgen('''
+ if argtype == '**':
+ if dealloc:
+ ret += mcgen('''
+qapi_free_KeyValuesList(%(obj)s);
+''',
+ obj=c_var(argname))
+ else:
+ ret += mcgen('''
+{
+ const QDictEntry *entry;
+ v = v; /* fix me baby */
+
+ for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
+ KeyValuesList *item = g_malloc0(sizeof(*item));
+ item->value = g_malloc0(sizeof(*item->value));
+ item->value->key = g_strdup(qdict_entry_key(entry));
+ item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
+
+ item->next = %(obj)s;
+ %(obj)s = item;
+ }
+}
+''',
+ obj=c_var(argname))
+ else:
+ ret += mcgen('''
%(visitor)s(v, &%(c_name)s, "%(name)s", errp);
''',
c_name=c_var(argname), name=argname, argtype=argtype,
visitor=type_visitor(argtype))
- if optional:
+ if optional and not '**':
pop_indent()
ret += mcgen('''
}
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..87b9ee6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -163,6 +163,8 @@ def c_type(name):
return 'bool'
elif name == 'number':
return 'double'
+ elif name == '**':
+ return 'KeyValuesList *'
elif type(name) == list:
return '%s *' % c_list_type(name[0])
elif is_enum(name):
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 17:26 ` [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list Luiz Capitulino
@ 2012-03-29 18:26 ` Anthony Liguori
2012-03-29 18:42 ` Luiz Capitulino
2012-03-29 19:28 ` Michael Roth
0 siblings, 2 replies; 24+ messages in thread
From: Anthony Liguori @ 2012-03-29 18:26 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, armbru, kraxel, qemu-devel, stefanha
On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> This allows for QAPI functions to receive a variable-length argument
> list. This is going to be used by device_add and netdev_add commands.
>
> In the schema, the argument list is represented by type name '**',
> like this example:
>
> { 'command': 'foo', 'data': { 'arg-list': '**' } }
>
> Each argument is represented by the KeyValues type and the C
> implementation should expect a KeyValuesList, like:
>
> void qmp_foo(KeyValuesList *values_list, Error **errp);
>
> XXX: This implementation is simple but very hacky. We just iterate
> through all arguments and build the KeyValuesList list to be
> passed to the QAPI function.
>
> Maybe we could have a kwargs type, that does exactly this but
> through a visitor instead?
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
What about just treating '**' as "marshal remaining arguments to a string" and
then pass that string to device_add? qmp_device_add can then parse that string
with QemuOpts.
It's a bit ugly, but that's how things worked. When we introduce qom_add, this
problem goes away because you would make multiple calls to qom_set to set all of
the properties.
Regards,
Anthony Liguori
> ---
> qapi-schema.json | 15 +++++++++++++++
> scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
> scripts/qapi.py | 2 ++
> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0d11d6e..25bd487 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1701,3 +1701,18 @@
> # Since: 1.1
> ##
> { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> +
> +##
> +# @KeyValues:
> +#
> +# A generic representation of a key value pair.
> +#
> +# @key: the name of the item
> +#
> +# @value: the string representation of the item value. This typically follows
> +# QEMU's command line parsing format. See the man pages for more
> +# information.
> +#
> +# Since: 0.14.0
> +##
> +{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 30a24d2..75a6e81 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
> obj=obj)
>
> for argname, argtype, optional, structured in parse_args(args):
> - if optional:
> + if optional and not '**':
> ret += mcgen('''
> visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
> if (has_%(c_name)s) {
> ''',
> c_name=c_var(argname), name=argname)
> push_indent()
> - ret += mcgen('''
> + if argtype == '**':
> + if dealloc:
> + ret += mcgen('''
> +qapi_free_KeyValuesList(%(obj)s);
> +''',
> + obj=c_var(argname))
> + else:
> + ret += mcgen('''
> +{
> + const QDictEntry *entry;
> + v = v; /* fix me baby */
> +
> + for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
> + KeyValuesList *item = g_malloc0(sizeof(*item));
> + item->value = g_malloc0(sizeof(*item->value));
> + item->value->key = g_strdup(qdict_entry_key(entry));
> + item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
> +
> + item->next = %(obj)s;
> + %(obj)s = item;
> + }
> +}
> +''',
> + obj=c_var(argname))
> + else:
> + ret += mcgen('''
> %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
> ''',
> c_name=c_var(argname), name=argname, argtype=argtype,
> visitor=type_visitor(argtype))
> - if optional:
> + if optional and not '**':
> pop_indent()
> ret += mcgen('''
> }
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index e062336..87b9ee6 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -163,6 +163,8 @@ def c_type(name):
> return 'bool'
> elif name == 'number':
> return 'double'
> + elif name == '**':
> + return 'KeyValuesList *'
> elif type(name) == list:
> return '%s *' % c_list_type(name[0])
> elif is_enum(name):
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 18:26 ` Anthony Liguori
@ 2012-03-29 18:42 ` Luiz Capitulino
2012-03-29 18:53 ` Anthony Liguori
2012-03-29 19:28 ` Michael Roth
1 sibling, 1 reply; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 18:42 UTC (permalink / raw)
To: Anthony Liguori; +Cc: mdroth, armbru, kraxel, qemu-devel, stefanha
On Thu, 29 Mar 2012 13:26:34 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:
> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> > This allows for QAPI functions to receive a variable-length argument
> > list. This is going to be used by device_add and netdev_add commands.
> >
> > In the schema, the argument list is represented by type name '**',
> > like this example:
> >
> > { 'command': 'foo', 'data': { 'arg-list': '**' } }
> >
> > Each argument is represented by the KeyValues type and the C
> > implementation should expect a KeyValuesList, like:
> >
> > void qmp_foo(KeyValuesList *values_list, Error **errp);
> >
> > XXX: This implementation is simple but very hacky. We just iterate
> > through all arguments and build the KeyValuesList list to be
> > passed to the QAPI function.
> >
> > Maybe we could have a kwargs type, that does exactly this but
> > through a visitor instead?
> >
> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>
> What about just treating '**' as "marshal remaining arguments to a string" and
> then pass that string to device_add? qmp_device_add can then parse that string
> with QemuOpts.
If this turns out to be simple enough, I'm fine with it.
> It's a bit ugly, but that's how things worked. When we introduce qom_add, this
> problem goes away because you would make multiple calls to qom_set to set all of
> the properties.
Just out of curiosity, is qom_add going to supersede device_add?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 18:42 ` Luiz Capitulino
@ 2012-03-29 18:53 ` Anthony Liguori
0 siblings, 0 replies; 24+ messages in thread
From: Anthony Liguori @ 2012-03-29 18:53 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, armbru, kraxel, qemu-devel, stefanha
On 03/29/2012 01:42 PM, Luiz Capitulino wrote:
> On Thu, 29 Mar 2012 13:26:34 -0500
> Anthony Liguori<aliguori@us.ibm.com> wrote:
>
>> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
>>> This allows for QAPI functions to receive a variable-length argument
>>> list. This is going to be used by device_add and netdev_add commands.
>>>
>>> In the schema, the argument list is represented by type name '**',
>>> like this example:
>>>
>>> { 'command': 'foo', 'data': { 'arg-list': '**' } }
>>>
>>> Each argument is represented by the KeyValues type and the C
>>> implementation should expect a KeyValuesList, like:
>>>
>>> void qmp_foo(KeyValuesList *values_list, Error **errp);
>>>
>>> XXX: This implementation is simple but very hacky. We just iterate
>>> through all arguments and build the KeyValuesList list to be
>>> passed to the QAPI function.
>>>
>>> Maybe we could have a kwargs type, that does exactly this but
>>> through a visitor instead?
>>>
>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>
>> What about just treating '**' as "marshal remaining arguments to a string" and
>> then pass that string to device_add? qmp_device_add can then parse that string
>> with QemuOpts.
>
> If this turns out to be simple enough, I'm fine with it.
I don't love doing this sort of double conversion but it's really the only
practical way to do it I think. device_add has a weird semantic where
printing/parsing is implied so I think it's unavoidable.
>> It's a bit ugly, but that's how things worked. When we introduce qom_add, this
>> problem goes away because you would make multiple calls to qom_set to set all of
>> the properties.
>
> Just out of curiosity, is qom_add going to supersede device_add?
Eventually...
I'd like to see qom_add as the low level interface but then I'd like to see nice
high level interfaces like 'block_add' which took a parameter of virtio-blk and
did all of the magic to create a block device in such a way that's compliant to
the current machine type.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 18:26 ` Anthony Liguori
2012-03-29 18:42 ` Luiz Capitulino
@ 2012-03-29 19:28 ` Michael Roth
2012-03-29 20:01 ` Anthony Liguori
2012-03-29 20:03 ` Paolo Bonzini
1 sibling, 2 replies; 24+ messages in thread
From: Michael Roth @ 2012-03-29 19:28 UTC (permalink / raw)
To: Anthony Liguori; +Cc: armbru, kraxel, qemu-devel, stefanha, Luiz Capitulino
On Thu, Mar 29, 2012 at 01:26:34PM -0500, Anthony Liguori wrote:
> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> >This allows for QAPI functions to receive a variable-length argument
> >list. This is going to be used by device_add and netdev_add commands.
> >
> >In the schema, the argument list is represented by type name '**',
> >like this example:
> >
> > { 'command': 'foo', 'data': { 'arg-list': '**' } }
> >
> >Each argument is represented by the KeyValues type and the C
> >implementation should expect a KeyValuesList, like:
> >
> > void qmp_foo(KeyValuesList *values_list, Error **errp);
> >
> >XXX: This implementation is simple but very hacky. We just iterate
> > through all arguments and build the KeyValuesList list to be
> > passed to the QAPI function.
> >
> > Maybe we could have a kwargs type, that does exactly this but
> > through a visitor instead?
> >
> >Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>
> What about just treating '**' as "marshal remaining arguments to a
> string" and then pass that string to device_add? qmp_device_add can
> then parse that string with QemuOpts.
Since currently we explicitly point qmp to the marshaller anyway, we
could also just treat '**' as an indicator to not generate a marshaller.
Then, we open-code the marshaller to process the QDict, rather than embedding
it in the script or passing it through to qmp_device_add().
>From the perspective of qmp_device_add() it then just looks like any
other qmp command.
>
> It's a bit ugly, but that's how things worked. When we introduce
> qom_add, this problem goes away because you would make multiple
> calls to qom_set to set all of the properties.
>
> Regards,
>
> Anthony Liguori
>
> >---
> > qapi-schema.json | 15 +++++++++++++++
> > scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
> > scripts/qapi.py | 2 ++
> > 3 files changed, 45 insertions(+), 3 deletions(-)
> >
> >diff --git a/qapi-schema.json b/qapi-schema.json
> >index 0d11d6e..25bd487 100644
> >--- a/qapi-schema.json
> >+++ b/qapi-schema.json
> >@@ -1701,3 +1701,18 @@
> > # Since: 1.1
> > ##
> > { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> >+
> >+##
> >+# @KeyValues:
> >+#
> >+# A generic representation of a key value pair.
> >+#
> >+# @key: the name of the item
> >+#
> >+# @value: the string representation of the item value. This typically follows
> >+# QEMU's command line parsing format. See the man pages for more
> >+# information.
> >+#
> >+# Since: 0.14.0
> >+##
> >+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
> >diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> >index 30a24d2..75a6e81 100644
> >--- a/scripts/qapi-commands.py
> >+++ b/scripts/qapi-commands.py
> >@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
> > obj=obj)
> >
> > for argname, argtype, optional, structured in parse_args(args):
> >- if optional:
> >+ if optional and not '**':
> > ret += mcgen('''
> > visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
> > if (has_%(c_name)s) {
> > ''',
> > c_name=c_var(argname), name=argname)
> > push_indent()
> >- ret += mcgen('''
> >+ if argtype == '**':
> >+ if dealloc:
> >+ ret += mcgen('''
> >+qapi_free_KeyValuesList(%(obj)s);
> >+''',
> >+ obj=c_var(argname))
> >+ else:
> >+ ret += mcgen('''
> >+{
> >+ const QDictEntry *entry;
> >+ v = v; /* fix me baby */
> >+
> >+ for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
> >+ KeyValuesList *item = g_malloc0(sizeof(*item));
> >+ item->value = g_malloc0(sizeof(*item->value));
> >+ item->value->key = g_strdup(qdict_entry_key(entry));
> >+ item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
> >+
> >+ item->next = %(obj)s;
> >+ %(obj)s = item;
> >+ }
> >+}
> >+''',
> >+ obj=c_var(argname))
> >+ else:
> >+ ret += mcgen('''
> > %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
> > ''',
> > c_name=c_var(argname), name=argname, argtype=argtype,
> > visitor=type_visitor(argtype))
> >- if optional:
> >+ if optional and not '**':
> > pop_indent()
> > ret += mcgen('''
> > }
> >diff --git a/scripts/qapi.py b/scripts/qapi.py
> >index e062336..87b9ee6 100644
> >--- a/scripts/qapi.py
> >+++ b/scripts/qapi.py
> >@@ -163,6 +163,8 @@ def c_type(name):
> > return 'bool'
> > elif name == 'number':
> > return 'double'
> >+ elif name == '**':
> >+ return 'KeyValuesList *'
> > elif type(name) == list:
> > return '%s *' % c_list_type(name[0])
> > elif is_enum(name):
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 19:28 ` Michael Roth
@ 2012-03-29 20:01 ` Anthony Liguori
2012-03-29 22:39 ` Michael Roth
2012-03-30 7:49 ` Paolo Bonzini
2012-03-29 20:03 ` Paolo Bonzini
1 sibling, 2 replies; 24+ messages in thread
From: Anthony Liguori @ 2012-03-29 20:01 UTC (permalink / raw)
To: Michael Roth; +Cc: armbru, kraxel, qemu-devel, stefanha, Luiz Capitulino
On 03/29/2012 02:28 PM, Michael Roth wrote:
> On Thu, Mar 29, 2012 at 01:26:34PM -0500, Anthony Liguori wrote:
>> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
>>> This allows for QAPI functions to receive a variable-length argument
>>> list. This is going to be used by device_add and netdev_add commands.
>>>
>>> In the schema, the argument list is represented by type name '**',
>>> like this example:
>>>
>>> { 'command': 'foo', 'data': { 'arg-list': '**' } }
>>>
>>> Each argument is represented by the KeyValues type and the C
>>> implementation should expect a KeyValuesList, like:
>>>
>>> void qmp_foo(KeyValuesList *values_list, Error **errp);
>>>
>>> XXX: This implementation is simple but very hacky. We just iterate
>>> through all arguments and build the KeyValuesList list to be
>>> passed to the QAPI function.
>>>
>>> Maybe we could have a kwargs type, that does exactly this but
>>> through a visitor instead?
>>>
>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>
>> What about just treating '**' as "marshal remaining arguments to a
>> string" and then pass that string to device_add? qmp_device_add can
>> then parse that string with QemuOpts.
>
> Since currently we explicitly point qmp to the marshaller anyway, we
> could also just treat '**' as an indicator to not generate a marshaller.
> Then, we open-code the marshaller to process the QDict, rather than embedding
> it in the script or passing it through to qmp_device_add().
You could also just do gen=False...
But I don't think open coding the marshaller is the right thing here. You have
to convert to strings and reparse anyway. The code needs to be shared between
device_add and netdev_add too.
Regards,
Anthony Liguori
>
>> From the perspective of qmp_device_add() it then just looks like any
> other qmp command.
>
>>
>> It's a bit ugly, but that's how things worked. When we introduce
>> qom_add, this problem goes away because you would make multiple
>> calls to qom_set to set all of the properties.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>> ---
>>> qapi-schema.json | 15 +++++++++++++++
>>> scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
>>> scripts/qapi.py | 2 ++
>>> 3 files changed, 45 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 0d11d6e..25bd487 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -1701,3 +1701,18 @@
>>> # Since: 1.1
>>> ##
>>> { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
>>> +
>>> +##
>>> +# @KeyValues:
>>> +#
>>> +# A generic representation of a key value pair.
>>> +#
>>> +# @key: the name of the item
>>> +#
>>> +# @value: the string representation of the item value. This typically follows
>>> +# QEMU's command line parsing format. See the man pages for more
>>> +# information.
>>> +#
>>> +# Since: 0.14.0
>>> +##
>>> +{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
>>> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
>>> index 30a24d2..75a6e81 100644
>>> --- a/scripts/qapi-commands.py
>>> +++ b/scripts/qapi-commands.py
>>> @@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
>>> obj=obj)
>>>
>>> for argname, argtype, optional, structured in parse_args(args):
>>> - if optional:
>>> + if optional and not '**':
>>> ret += mcgen('''
>>> visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
>>> if (has_%(c_name)s) {
>>> ''',
>>> c_name=c_var(argname), name=argname)
>>> push_indent()
>>> - ret += mcgen('''
>>> + if argtype == '**':
>>> + if dealloc:
>>> + ret += mcgen('''
>>> +qapi_free_KeyValuesList(%(obj)s);
>>> +''',
>>> + obj=c_var(argname))
>>> + else:
>>> + ret += mcgen('''
>>> +{
>>> + const QDictEntry *entry;
>>> + v = v; /* fix me baby */
>>> +
>>> + for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
>>> + KeyValuesList *item = g_malloc0(sizeof(*item));
>>> + item->value = g_malloc0(sizeof(*item->value));
>>> + item->value->key = g_strdup(qdict_entry_key(entry));
>>> + item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
>>> +
>>> + item->next = %(obj)s;
>>> + %(obj)s = item;
>>> + }
>>> +}
>>> +''',
>>> + obj=c_var(argname))
>>> + else:
>>> + ret += mcgen('''
>>> %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
>>> ''',
>>> c_name=c_var(argname), name=argname, argtype=argtype,
>>> visitor=type_visitor(argtype))
>>> - if optional:
>>> + if optional and not '**':
>>> pop_indent()
>>> ret += mcgen('''
>>> }
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index e062336..87b9ee6 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -163,6 +163,8 @@ def c_type(name):
>>> return 'bool'
>>> elif name == 'number':
>>> return 'double'
>>> + elif name == '**':
>>> + return 'KeyValuesList *'
>>> elif type(name) == list:
>>> return '%s *' % c_list_type(name[0])
>>> elif is_enum(name):
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 20:01 ` Anthony Liguori
@ 2012-03-29 22:39 ` Michael Roth
2012-03-29 22:56 ` Michael Roth
2012-03-30 7:49 ` Paolo Bonzini
1 sibling, 1 reply; 24+ messages in thread
From: Michael Roth @ 2012-03-29 22:39 UTC (permalink / raw)
To: Anthony Liguori; +Cc: armbru, kraxel, qemu-devel, stefanha, Luiz Capitulino
On Thu, Mar 29, 2012 at 03:01:16PM -0500, Anthony Liguori wrote:
> On 03/29/2012 02:28 PM, Michael Roth wrote:
> >On Thu, Mar 29, 2012 at 01:26:34PM -0500, Anthony Liguori wrote:
> >>On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> >>>This allows for QAPI functions to receive a variable-length argument
> >>>list. This is going to be used by device_add and netdev_add commands.
> >>>
> >>>In the schema, the argument list is represented by type name '**',
> >>>like this example:
> >>>
> >>> { 'command': 'foo', 'data': { 'arg-list': '**' } }
> >>>
> >>>Each argument is represented by the KeyValues type and the C
> >>>implementation should expect a KeyValuesList, like:
> >>>
> >>> void qmp_foo(KeyValuesList *values_list, Error **errp);
> >>>
> >>>XXX: This implementation is simple but very hacky. We just iterate
> >>> through all arguments and build the KeyValuesList list to be
> >>> passed to the QAPI function.
> >>>
> >>> Maybe we could have a kwargs type, that does exactly this but
> >>> through a visitor instead?
> >>>
> >>>Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >>
> >>What about just treating '**' as "marshal remaining arguments to a
> >>string" and then pass that string to device_add? qmp_device_add can
> >>then parse that string with QemuOpts.
> >
> >Since currently we explicitly point qmp to the marshaller anyway, we
> >could also just treat '**' as an indicator to not generate a marshaller.
> >Then, we open-code the marshaller to process the QDict, rather than embedding
> >it in the script or passing it through to qmp_device_add().
>
> You could also just do gen=False...
Ahh, yes we could. Nice :)
>
> But I don't think open coding the marshaller is the right thing
> here. You have to convert to strings and reparse anyway. The code
> needs to be shared between device_add and netdev_add too.
The only thing the marshallers need to do is call qemu_opts_from_qdict()
and pass them on to qdev_device_add()/net_client_init()/etc. We'd basically
be taking the current qmp implementations and modifying their call signatures
to be compatible with qmp-dispatch.c in the future. It's not really
qapi-ish, admittedly, but neither is a built-in varargs sort of type.
I'd just prefer not to bake legacy hooks into the code generators if we
don't have to. If we absolutely have to do this in the future, it would be more
sense to define such fields as being string arguments from the get-go.
>
> Regards,
>
> Anthony Liguori
>
> >
> >>From the perspective of qmp_device_add() it then just looks like any
> >other qmp command.
> >
> >>
> >>It's a bit ugly, but that's how things worked. When we introduce
> >>qom_add, this problem goes away because you would make multiple
> >>calls to qom_set to set all of the properties.
> >>
> >>Regards,
> >>
> >>Anthony Liguori
> >>
> >>>---
> >>> qapi-schema.json | 15 +++++++++++++++
> >>> scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
> >>> scripts/qapi.py | 2 ++
> >>> 3 files changed, 45 insertions(+), 3 deletions(-)
> >>>
> >>>diff --git a/qapi-schema.json b/qapi-schema.json
> >>>index 0d11d6e..25bd487 100644
> >>>--- a/qapi-schema.json
> >>>+++ b/qapi-schema.json
> >>>@@ -1701,3 +1701,18 @@
> >>> # Since: 1.1
> >>> ##
> >>> { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> >>>+
> >>>+##
> >>>+# @KeyValues:
> >>>+#
> >>>+# A generic representation of a key value pair.
> >>>+#
> >>>+# @key: the name of the item
> >>>+#
> >>>+# @value: the string representation of the item value. This typically follows
> >>>+# QEMU's command line parsing format. See the man pages for more
> >>>+# information.
> >>>+#
> >>>+# Since: 0.14.0
> >>>+##
> >>>+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
> >>>diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> >>>index 30a24d2..75a6e81 100644
> >>>--- a/scripts/qapi-commands.py
> >>>+++ b/scripts/qapi-commands.py
> >>>@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
> >>> obj=obj)
> >>>
> >>> for argname, argtype, optional, structured in parse_args(args):
> >>>- if optional:
> >>>+ if optional and not '**':
> >>> ret += mcgen('''
> >>> visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
> >>> if (has_%(c_name)s) {
> >>> ''',
> >>> c_name=c_var(argname), name=argname)
> >>> push_indent()
> >>>- ret += mcgen('''
> >>>+ if argtype == '**':
> >>>+ if dealloc:
> >>>+ ret += mcgen('''
> >>>+qapi_free_KeyValuesList(%(obj)s);
> >>>+''',
> >>>+ obj=c_var(argname))
> >>>+ else:
> >>>+ ret += mcgen('''
> >>>+{
> >>>+ const QDictEntry *entry;
> >>>+ v = v; /* fix me baby */
> >>>+
> >>>+ for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
> >>>+ KeyValuesList *item = g_malloc0(sizeof(*item));
> >>>+ item->value = g_malloc0(sizeof(*item->value));
> >>>+ item->value->key = g_strdup(qdict_entry_key(entry));
> >>>+ item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
> >>>+
> >>>+ item->next = %(obj)s;
> >>>+ %(obj)s = item;
> >>>+ }
> >>>+}
> >>>+''',
> >>>+ obj=c_var(argname))
> >>>+ else:
> >>>+ ret += mcgen('''
> >>> %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
> >>> ''',
> >>> c_name=c_var(argname), name=argname, argtype=argtype,
> >>> visitor=type_visitor(argtype))
> >>>- if optional:
> >>>+ if optional and not '**':
> >>> pop_indent()
> >>> ret += mcgen('''
> >>> }
> >>>diff --git a/scripts/qapi.py b/scripts/qapi.py
> >>>index e062336..87b9ee6 100644
> >>>--- a/scripts/qapi.py
> >>>+++ b/scripts/qapi.py
> >>>@@ -163,6 +163,8 @@ def c_type(name):
> >>> return 'bool'
> >>> elif name == 'number':
> >>> return 'double'
> >>>+ elif name == '**':
> >>>+ return 'KeyValuesList *'
> >>> elif type(name) == list:
> >>> return '%s *' % c_list_type(name[0])
> >>> elif is_enum(name):
> >>
> >
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 22:39 ` Michael Roth
@ 2012-03-29 22:56 ` Michael Roth
0 siblings, 0 replies; 24+ messages in thread
From: Michael Roth @ 2012-03-29 22:56 UTC (permalink / raw)
To: Anthony Liguori; +Cc: armbru, kraxel, qemu-devel, stefanha, Luiz Capitulino
On Thu, Mar 29, 2012 at 05:39:10PM -0500, Michael Roth wrote:
> On Thu, Mar 29, 2012 at 03:01:16PM -0500, Anthony Liguori wrote:
> > On 03/29/2012 02:28 PM, Michael Roth wrote:
> > >On Thu, Mar 29, 2012 at 01:26:34PM -0500, Anthony Liguori wrote:
> > >>On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> > >>>This allows for QAPI functions to receive a variable-length argument
> > >>>list. This is going to be used by device_add and netdev_add commands.
> > >>>
> > >>>In the schema, the argument list is represented by type name '**',
> > >>>like this example:
> > >>>
> > >>> { 'command': 'foo', 'data': { 'arg-list': '**' } }
> > >>>
> > >>>Each argument is represented by the KeyValues type and the C
> > >>>implementation should expect a KeyValuesList, like:
> > >>>
> > >>> void qmp_foo(KeyValuesList *values_list, Error **errp);
> > >>>
> > >>>XXX: This implementation is simple but very hacky. We just iterate
> > >>> through all arguments and build the KeyValuesList list to be
> > >>> passed to the QAPI function.
> > >>>
> > >>> Maybe we could have a kwargs type, that does exactly this but
> > >>> through a visitor instead?
> > >>>
> > >>>Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> > >>
> > >>What about just treating '**' as "marshal remaining arguments to a
> > >>string" and then pass that string to device_add? qmp_device_add can
> > >>then parse that string with QemuOpts.
> > >
> > >Since currently we explicitly point qmp to the marshaller anyway, we
> > >could also just treat '**' as an indicator to not generate a marshaller.
> > >Then, we open-code the marshaller to process the QDict, rather than embedding
> > >it in the script or passing it through to qmp_device_add().
> >
> > You could also just do gen=False...
>
> Ahh, yes we could. Nice :)
>
> >
> > But I don't think open coding the marshaller is the right thing
> > here. You have to convert to strings and reparse anyway. The code
> > needs to be shared between device_add and netdev_add too.
>
> The only thing the marshallers need to do is call qemu_opts_from_qdict()
> and pass them on to qdev_device_add()/net_client_init()/etc. We'd basically
> be taking the current qmp implementations and modifying their call signatures
> to be compatible with qmp-dispatch.c in the future. It's not really
Err, scratch that... I've been living in qemu-ga land too long. For
QMP this basically amounts to a no-op for now (other than documenting the
command in the schema and doing the error-handling cleanups), and when we
switch to the new qmp server/dispatch stuff we just massage the function
signature to match the marshaller qapi expects and continue to skip
marshaller generation.
> qapi-ish, admittedly, but neither is a built-in varargs sort of type.
>
> I'd just prefer not to bake legacy hooks into the code generators if we
> don't have to. If we absolutely have to do this in the future, it would be more
> sense to define such fields as being string arguments from the get-go.
>
> >
> > Regards,
> >
> > Anthony Liguori
> >
> > >
> > >>From the perspective of qmp_device_add() it then just looks like any
> > >other qmp command.
> > >
> > >>
> > >>It's a bit ugly, but that's how things worked. When we introduce
> > >>qom_add, this problem goes away because you would make multiple
> > >>calls to qom_set to set all of the properties.
> > >>
> > >>Regards,
> > >>
> > >>Anthony Liguori
> > >>
> > >>>---
> > >>> qapi-schema.json | 15 +++++++++++++++
> > >>> scripts/qapi-commands.py | 31 ++++++++++++++++++++++++++++---
> > >>> scripts/qapi.py | 2 ++
> > >>> 3 files changed, 45 insertions(+), 3 deletions(-)
> > >>>
> > >>>diff --git a/qapi-schema.json b/qapi-schema.json
> > >>>index 0d11d6e..25bd487 100644
> > >>>--- a/qapi-schema.json
> > >>>+++ b/qapi-schema.json
> > >>>@@ -1701,3 +1701,18 @@
> > >>> # Since: 1.1
> > >>> ##
> > >>> { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> > >>>+
> > >>>+##
> > >>>+# @KeyValues:
> > >>>+#
> > >>>+# A generic representation of a key value pair.
> > >>>+#
> > >>>+# @key: the name of the item
> > >>>+#
> > >>>+# @value: the string representation of the item value. This typically follows
> > >>>+# QEMU's command line parsing format. See the man pages for more
> > >>>+# information.
> > >>>+#
> > >>>+# Since: 0.14.0
> > >>>+##
> > >>>+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
> > >>>diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > >>>index 30a24d2..75a6e81 100644
> > >>>--- a/scripts/qapi-commands.py
> > >>>+++ b/scripts/qapi-commands.py
> > >>>@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
> > >>> obj=obj)
> > >>>
> > >>> for argname, argtype, optional, structured in parse_args(args):
> > >>>- if optional:
> > >>>+ if optional and not '**':
> > >>> ret += mcgen('''
> > >>> visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
> > >>> if (has_%(c_name)s) {
> > >>> ''',
> > >>> c_name=c_var(argname), name=argname)
> > >>> push_indent()
> > >>>- ret += mcgen('''
> > >>>+ if argtype == '**':
> > >>>+ if dealloc:
> > >>>+ ret += mcgen('''
> > >>>+qapi_free_KeyValuesList(%(obj)s);
> > >>>+''',
> > >>>+ obj=c_var(argname))
> > >>>+ else:
> > >>>+ ret += mcgen('''
> > >>>+{
> > >>>+ const QDictEntry *entry;
> > >>>+ v = v; /* fix me baby */
> > >>>+
> > >>>+ for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
> > >>>+ KeyValuesList *item = g_malloc0(sizeof(*item));
> > >>>+ item->value = g_malloc0(sizeof(*item->value));
> > >>>+ item->value->key = g_strdup(qdict_entry_key(entry));
> > >>>+ item->value->value = g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
> > >>>+
> > >>>+ item->next = %(obj)s;
> > >>>+ %(obj)s = item;
> > >>>+ }
> > >>>+}
> > >>>+''',
> > >>>+ obj=c_var(argname))
> > >>>+ else:
> > >>>+ ret += mcgen('''
> > >>> %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
> > >>> ''',
> > >>> c_name=c_var(argname), name=argname, argtype=argtype,
> > >>> visitor=type_visitor(argtype))
> > >>>- if optional:
> > >>>+ if optional and not '**':
> > >>> pop_indent()
> > >>> ret += mcgen('''
> > >>> }
> > >>>diff --git a/scripts/qapi.py b/scripts/qapi.py
> > >>>index e062336..87b9ee6 100644
> > >>>--- a/scripts/qapi.py
> > >>>+++ b/scripts/qapi.py
> > >>>@@ -163,6 +163,8 @@ def c_type(name):
> > >>> return 'bool'
> > >>> elif name == 'number':
> > >>> return 'double'
> > >>>+ elif name == '**':
> > >>>+ return 'KeyValuesList *'
> > >>> elif type(name) == list:
> > >>> return '%s *' % c_list_type(name[0])
> > >>> elif is_enum(name):
> > >>
> > >
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 20:01 ` Anthony Liguori
2012-03-29 22:39 ` Michael Roth
@ 2012-03-30 7:49 ` Paolo Bonzini
2012-03-30 13:01 ` Luiz Capitulino
1 sibling, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2012-03-30 7:49 UTC (permalink / raw)
To: Anthony Liguori
Cc: Michael Roth, armbru, qemu-devel, Luiz Capitulino, kraxel,
stefanha
Il 29/03/2012 22:01, Anthony Liguori ha scritto:
>> Then, we open-code the marshaller to process the QDict, rather than
>> embedding
>> it in the script or passing it through to qmp_device_add().
>
> You could also just do gen=False...
>
> But I don't think open coding the marshaller is the right thing here.
> You have to convert to strings and reparse anyway.
device_add expects strings, you don't have to convert no? There is a
single parse point.
(Instead, a qom_add would accept the right types, but then it would also
use a more QAPI-friendly syntax with lists insteads of varargs).
Regarding device_add ? and device_add foo,? I would implement it as
separate QMP commands hooking into QOM, such as qom_list_types (taking
the superclass as an optional argument) and qom_properties. But the
latter first needs static properties to move up from devices to objects.
I'll take a look at that.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-30 7:49 ` Paolo Bonzini
@ 2012-03-30 13:01 ` Luiz Capitulino
0 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-30 13:01 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Anthony Liguori, stefanha, armbru, qemu-devel, Michael Roth,
kraxel
On Fri, 30 Mar 2012 09:49:20 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Regarding device_add ? and device_add foo,? I would implement it as
> separate QMP commands hooking into QOM, such as qom_list_types (taking
> the superclass as an optional argument) and qom_properties. But the
> latter first needs static properties to move up from devices to objects.
Yes, after I suggested query-devices I remembered about qom_list_types.
I think it's ok for hmp.c to use it even if it's not declared stable yet,
right?
Anthony, you said you did it in your tree differently, did you? I think
using qom_list_types is the way to go here...
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list
2012-03-29 19:28 ` Michael Roth
2012-03-29 20:01 ` Anthony Liguori
@ 2012-03-29 20:03 ` Paolo Bonzini
1 sibling, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2012-03-29 20:03 UTC (permalink / raw)
To: Michael Roth
Cc: Anthony Liguori, stefanha, armbru, qemu-devel, Luiz Capitulino,
kraxel
Il 29/03/2012 21:28, Michael Roth ha scritto:
> Since currently we explicitly point qmp to the marshaller anyway, we
> could also just treat '**' as an indicator to not generate a marshaller.
> Then, we open-code the marshaller to process the QDict, rather than embedding
> it in the script or passing it through to qmp_device_add().
>
> From the perspective of qmp_device_add() it then just looks like any
> other qmp command.
That's what no_gen does, right?
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH 13/13] qapi: convert device_add
2012-03-29 17:26 [Qemu-devel] [RFC 00/13]: convert device_add to the qapi Luiz Capitulino
` (11 preceding siblings ...)
2012-03-29 17:26 ` [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list Luiz Capitulino
@ 2012-03-29 17:26 ` Luiz Capitulino
12 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-03-29 17:26 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, aliguori, kraxel, stefanha, armbru
FIXME: HMP's help text (device_add ?) is not implemented. We need a way to
export this information in QMP.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp-commands.hx | 3 +--
hmp.c | 24 ++++++++++++++++++
hmp.h | 1 +
hw/qdev-monitor.c | 70 +++++++++++++++++++++++++++++++++++++++--------------
hw/qdev.h | 2 +-
qapi-schema.json | 23 ++++++++++++++++++
qmp-commands.hx | 3 +--
vl.c | 3 ++-
8 files changed, 105 insertions(+), 24 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bd35a3e..376dc4d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -606,8 +606,7 @@ ETEXI
.args_type = "device:O",
.params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_device_add,
+ .mhandler.cmd = hmp_device_add,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 9cf2d13..23c06ec 100644
--- a/hmp.c
+++ b/hmp.c
@@ -934,3 +934,27 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
}
}
+
+void hmp_device_add(Monitor *mon, const QDict *qdict)
+{
+ KeyValuesList *opts_list = NULL;
+ const QDictEntry *entry;
+ Error *err = NULL;
+
+ for (entry = qdict_first(qdict); entry; entry = qdict_next(qdict, entry)) {
+ const char *key = qdict_entry_key(entry);
+ const char *value = qstring_get_str(qobject_to_qstring(qdict_entry_value(entry)));
+ KeyValuesList *kv;
+
+ kv = g_malloc0(sizeof(*kv));
+ kv->value = g_malloc0(sizeof(*kv->value));
+ kv->value->key = g_strdup(key);
+ kv->value->value = g_strdup(value);
+ kv->next = opts_list;
+ opts_list = kv;
+ }
+
+ qmp_device_add(opts_list, &err);
+ qapi_free_KeyValuesList(opts_list);
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 8807853..3e75d4c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,5 +60,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict);
void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
void hmp_migrate(Monitor *mon, const QDict *qdict);
+void hmp_device_add(Monitor *mon, const QDict *qdict);
#endif
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a310cc7..0661f55 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -19,6 +19,7 @@
#include "qdev.h"
#include "monitor.h"
+#include "qmp-commands.h"
/*
* Aliases were a bad idea from the start. Let's keep them
@@ -388,7 +389,7 @@ static BusState *qbus_find(const char *path)
}
}
-DeviceState *qdev_device_add(QemuOpts *opts)
+DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
{
ObjectClass *obj;
DeviceClass *k;
@@ -398,7 +399,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
driver = qemu_opt_get(opts, "driver");
if (!driver) {
- qerror_report(QERR_MISSING_PARAMETER, "driver");
+ error_set(errp, QERR_MISSING_PARAMETER, "driver");
return NULL;
}
@@ -414,7 +415,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
if (!obj) {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
return NULL;
}
@@ -428,20 +429,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (bus->info != k->bus_info) {
- qerror_report(QERR_BAD_BUS_FOR_DEVICE,
- driver, bus->info->name);
+ error_set(errp, QERR_BAD_BUS_FOR_DEVICE, driver, bus->info->name);
return NULL;
}
} else {
bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_info);
if (!bus) {
- qerror_report(QERR_NO_BUS_FOR_DEVICE,
- driver, k->bus_info->name);
+ error_set(errp, QERR_NO_BUS_FOR_DEVICE, driver, k->bus_info->name);
return NULL;
}
}
if (qdev_hotplug && !bus->allow_hotplug) {
- qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
+ error_set(errp, QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}
@@ -463,7 +462,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
return NULL;
}
if (qdev_init(qdev) < 0) {
- qerror_report(QERR_DEVICE_INIT_FAILED, driver);
+ error_set(errp, QERR_DEVICE_INIT_FAILED, driver);
return NULL;
}
if (qdev->id) {
@@ -555,23 +554,58 @@ void do_info_qdm(Monitor *mon)
object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, NULL);
}
-int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_device_add(KeyValuesList *opts, Error **errp)
{
- QemuOpts *opts;
+ Error *local_err = NULL;
+ QemuOptsList *opts_list;
+ QemuOpts *qopts = NULL;
+ const char *id = NULL;
+ KeyValuesList *kv;
+
+ for (kv = opts; kv; kv = kv->next) {
+ if (!strcmp(kv->value->key, "id")) {
+ id = kv->value->key;
+ break;
+ }
+ }
- opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict);
- if (!opts) {
- return -1;
+ opts_list = qemu_find_opts_err("device", &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
+ }
+
+ qopts = qemu_opts_create(opts_list, id, 1, &local_err);
+ if (local_err) {
+ goto exit_err;
+ }
+
+ for (kv = opts; kv; kv = kv->next) {
+ qemu_opt_set_err(qopts, kv->value->key, kv->value->value, &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
+ }
}
+
+#if 0
+ FIXME: move the help to the HMP counterpart
if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
qemu_opts_del(opts);
return 0;
}
- if (!qdev_device_add(opts)) {
- qemu_opts_del(opts);
- return -1;
+#endif
+
+ qdev_device_add(qopts, &local_err);
+ if (error_is_set(&local_err)) {
+ goto exit_err;
}
- return 0;
+
+ return;
+
+exit_err:
+ if (qopts) {
+ qemu_opts_del(qopts);
+ }
+ error_propagate(errp, local_err);
}
int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/hw/qdev.h b/hw/qdev.h
index 9cc3f98..3b938e2 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -143,7 +143,7 @@ DeviceState *qdev_create(BusState *bus, const char *name);
DeviceState *qdev_try_create(BusState *bus, const char *name);
bool qdev_exists(const char *name);
int qdev_device_help(QemuOpts *opts);
-DeviceState *qdev_device_add(QemuOpts *opts);
+DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
void qdev_init_nofail(DeviceState *dev);
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
diff --git a/qapi-schema.json b/qapi-schema.json
index 25bd487..68e2515 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1716,3 +1716,26 @@
# Since: 0.14.0
##
{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
+
+##
+# @device_add:
+#
+# Add a new device to the guest
+#
+# @device-optoins: a list of KeyValues of options specific to each device.
+# There are two driver indepedent options: 'driver' (which is
+# actually _required_) and 'id'.
+#
+# Returns: Nothing on success
+# If @driver is not a valid device type, DeviceNotFound
+# If @opts contains an invalid parameter for this device,
+# InvalidParameter
+#
+# Notes: The semantics of @opts is not well defined. Future commands will be
+# introduced that provide stronger typing for device creation.
+# This command performs hotplug of devices for guests. When the command
+# returns, the device may not be visible to the guest.
+#
+# Since: 0.14.0
+##
+{ 'command': 'device_add', 'data': {'device-options': '**' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c626ba8..829a043 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -279,8 +279,7 @@ EQMP
.args_type = "device:O",
.params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_device_add,
+ .mhandler.cmd_new = qmp_marshal_input_device_add,
},
SQMP
diff --git a/vl.c b/vl.c
index 6467d6b..976126f 100644
--- a/vl.c
+++ b/vl.c
@@ -1821,7 +1821,8 @@ static int device_init_func(QemuOpts *opts, void *opaque)
{
DeviceState *dev;
- dev = qdev_device_add(opts);
+ /* FIXME: */
+ dev = qdev_device_add(opts, NULL);
if (!dev)
return -1;
return 0;
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 24+ messages in thread