[Qemu-devel] [PATCH qmp-next v5 00/16]: qapi: convert netdev_add & netdev_del

[Qemu-devel] [PATCH qmp-next v5 00/16]: qapi: convert netdev_add & netdev_del

[Luiz Capitulino]

v5

- Simplify set_option_parameter() [Laszlo]
- Fix bad patch split in patch 15/16, hunk changing net_init_netdev()
  pertains to patch 14/16 [Laszlo]

 blockdev.c           |    2 +-
 hmp-commands.hx      |    6 +-
 hmp.c                |   30 +++++++++
 hmp.h                |    2 +
 hw/pci-hotplug.c     |    8 ++-
 hw/qdev-monitor.c    |    7 +-
 hw/usb/dev-network.c |    7 +-
 hw/usb/dev-storage.c |    2 +-
 hw/watchdog.c        |    2 +-
 net.c                |  104 ++++++++++++++++++++----------
 net.h                |    6 +-
 net/dump.c           |    2 +-
 net/dump.h           |    3 +-
 net/slirp.c          |    5 +-
 net/slirp.h          |    5 +-
 net/socket.c         |    8 +--
 net/socket.h         |    3 +-
 net/tap-win32.c      |    2 +-
 net/tap.c            |    9 ++-
 net/tap.h            |    5 +-
 net/vde.c            |    2 +-
 net/vde.h            |    2 +-
 qapi-schema.json     |   42 ++++++++++++
 qemu-char.c          |    8 ++-
 qemu-config.c        |   43 ++++++++++---
 qemu-config.h        |    3 +
 qemu-option.c        |  175 ++++++++++++++++++++++++++++++++++----------------
 qemu-option.h        |   11 +++-
 qemu-sockets.c       |    8 +--
 qerror.c             |    4 ++
 qerror.h             |    3 +
 qmp-commands.hx      |   10 +--
 vl.c                 |   22 ++++---
 33 files changed, 380 insertions(+), 171 deletions(-)

[Qemu-devel] [PATCH 01/16] qemu-option: qemu_opts_create(): use error_set()

[Luiz Capitulino]

This commit converts qemu_opts_create() from qerror_report() to
error_set().

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 keeps 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 67895b2..622ecba 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -569,7 +569,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 ae22fb1..a96c0b9 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -584,7 +584,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 fe1126f..0bd903f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2584,10 +2584,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 46c7619..2ae715d 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -461,7 +461,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, errp);
         if (sock != -1 && ostr) {
@@ -490,7 +490,7 @@ int inet_connect(const char *str, bool block, Error **errp)
     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) {
         if (block) {
             qemu_opt_set(opts, "block", "on");
@@ -589,7 +589,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) {
@@ -617,7 +617,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 23ab3a3..1485426 100644
--- a/vl.c
+++ b/vl.c
@@ -1786,7 +1786,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;
@@ -1921,7 +1921,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);
@@ -2035,14 +2035,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);
@@ -2065,7 +2065,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);
@@ -2813,7 +2813,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"));
@@ -2845,7 +2846,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"));
@@ -2857,14 +2859,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

[Qemu-devel] [PATCH 02/16] qemu-option: parse_option_number(): use error_set()

[Luiz Capitulino]

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

[Qemu-devel] [PATCH 03/16] qemu-option: parse_option_bool(): use error_set()

[Luiz Capitulino]

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 |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 72dcb56..b5da116 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,9 +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;
-        list->value.n = flag;
+        parse_option_bool(name, value, &flag, &local_err);
+        if (!error_is_set(&local_err)) {
+            list->value.n = flag;
+        }
         break;
 
     case OPT_STRING:
@@ -298,6 +299,12 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
         return -1;
     }
 
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+
     return 0;
 }
 
@@ -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

[Qemu-devel] [PATCH 04/16] qemu-option: parse_option_size(): use error_set()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qemu-option.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index b5da116..42bb685 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;
 }
 
 /*
@@ -290,8 +289,7 @@ 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);
         break;
 
     default:
@@ -602,7 +600,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

[Qemu-devel] [PATCH 05/16] qemu-option: qemu_opt_parse(): use error_set()

[Luiz Capitulino]

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 |   36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 42bb685..6d36970 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -581,38 +581,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)
@@ -628,6 +617,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++) {
@@ -658,10 +648,14 @@ 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)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         qemu_opt_del(opt);
         return -1;
     }
+
     return 0;
 }
 
@@ -1050,6 +1044,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);
 
@@ -1068,7 +1063,10 @@ int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
 
         opt->desc = &desc[i];
 
-        if (qemu_opt_parse(opt) < 0) {
+        qemu_opt_parse(opt, &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

[Qemu-devel] [PATCH 06/16] qemu-option: qemu_opts_validate(): use error_set()

[Luiz Capitulino]

net_client_init() propagates the error up by calling qerror_report_err(),
because its users expect QError semantics.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 net.c         |    6 +++++-
 qemu-option.c |   13 +++++--------
 qemu-option.h |    2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/net.c b/net.c
index 1922d8a..f5d9cc7 100644
--- a/net.c
+++ b/net.c
@@ -1136,10 +1136,14 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
     for (i = 0; i < NET_CLIENT_TYPE_MAX; i++) {
         if (net_client_types[i].type != NULL &&
             !strcmp(net_client_types[i].type, type)) {
+            Error *local_err = NULL;
             VLANState *vlan = NULL;
             int ret;
 
-            if (qemu_opts_validate(opts, &net_client_types[i].desc[0]) == -1) {
+            qemu_opts_validate(opts, &net_client_types[i].desc[0], &local_err);
+            if (error_is_set(&local_err)) {
+                qerror_report_err(local_err);
+                error_free(local_err);
                 return -1;
             }
 
diff --git a/qemu-option.c b/qemu-option.c
index 6d36970..eee3b45 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -1041,7 +1041,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
 /* Validate parsed opts against descriptions where no
  * descriptions were provided in the QemuOptsList.
  */
-int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
+void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
 {
     QemuOpt *opt;
     Error *local_err = NULL;
@@ -1057,21 +1057,18 @@ 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(errp, QERR_INVALID_PARAMETER, opt->name);
+            return;
         }
 
         opt->desc = &desc[i];
 
         qemu_opt_parse(opt, &local_err);
         if (error_is_set(&local_err)) {
-            qerror_report_err(local_err);
-            error_free(local_err);
-            return -1;
+            error_propagate(errp, local_err);
+            return;
         }
     }
-
-    return 0;
 }
 
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
diff --git a/qemu-option.h b/qemu-option.h
index 4d5b3d3..e9fbbb5 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -125,7 +125,7 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
                   const char *name, const char *value);
 const char *qemu_opts_id(QemuOpts *opts);
 void qemu_opts_del(QemuOpts *opts);
-int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc);
+void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp);
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev);
 void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 07/16] qemu-option: opt_set(): use error_set()

[Luiz Capitulino]

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 |   31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index eee3b45..7af2b50 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -612,8 +612,8 @@ 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;
@@ -629,8 +629,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(errp, QERR_INVALID_PARAMETER, name);
+            return;
         }
     }
 
@@ -650,18 +650,23 @@ static int opt_set(QemuOpts *opts, const char *name, const char *value,
     }
     qemu_opt_parse(opt, &local_err);
     if (error_is_set(&local_err)) {
-        qerror_report_err(local_err);
-        error_free(local_err);
+        error_propagate(errp, local_err);
         qemu_opt_del(opt);
-        return -1;
     }
-
-    return 0;
 }
 
 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)
@@ -847,6 +852,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, '=');
@@ -878,7 +884,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

[Qemu-devel] [PATCH 08/16] qemu-option: introduce qemu_opt_set_err()

[Luiz Capitulino]

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 7af2b50..1cddb9d 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -669,6 +669,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 e9fbbb5..c0e022b 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

[Qemu-devel] [PATCH 09/16] qemu-option: qemu_opts_from_qdict(): use error_set()

[Luiz Capitulino]

do_device_add() and do_netdev_add() call qerror_report_err() to maintain
their QError semantics.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/qdev-monitor.c |    7 +++++--
 net.c             |    5 ++++-
 qemu-option.c     |   31 ++++++++++++++++++++++++-------
 qemu-option.h     |    3 ++-
 4 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index eed781d..b01ef06 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -554,10 +554,13 @@ void do_info_qdm(Monitor *mon)
 
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
+    Error *local_err = NULL;
     QemuOpts *opts;
 
-    opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict);
-    if (!opts) {
+    opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return -1;
     }
     if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
diff --git a/net.c b/net.c
index f5d9cc7..246209f 100644
--- a/net.c
+++ b/net.c
@@ -1237,11 +1237,14 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict)
 
 int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
+    Error *local_err = NULL;
     QemuOpts *opts;
     int res;
 
-    opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict);
+    opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &local_err);
     if (!opts) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return -1;
     }
 
diff --git a/qemu-option.c b/qemu-option.c
index 1cddb9d..bb3886c 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -968,13 +968,19 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
     assert(opts);
 }
 
+typedef struct OptsFromQDictState {
+    QemuOpts *opts;
+    Error **errp;
+} OptsFromQDictState;
+
 static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
 {
+    OptsFromQDictState *state = opaque;
     char buf[32];
     const char *value;
     int n;
 
-    if (!strcmp(key, "id")) {
+    if (!strcmp(key, "id") || error_is_set(state->errp)) {
         return;
     }
 
@@ -1002,7 +1008,8 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
     default:
         return;
     }
-    qemu_opt_set(opaque, key, value);
+
+    qemu_opt_set_err(state->opts, key, value, state->errp);
 }
 
 /*
@@ -1011,21 +1018,31 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
  * Only QStrings, QInts, QFloats and QBools are copied.  Entries with
  * other types are silently ignored.
  */
-QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict)
+QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
+                               Error **errp)
 {
-    QemuOpts *opts;
+    OptsFromQDictState state;
     Error *local_err = NULL;
+    QemuOpts *opts;
 
     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);
+        error_propagate(errp, local_err);
         return NULL;
     }
 
     assert(opts != NULL);
-    qdict_iter(qdict, qemu_opts_from_qdict_1, opts);
+
+    state.errp = &local_err;
+    state.opts = opts;
+    qdict_iter(qdict, qemu_opts_from_qdict_1, &state);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
+        return NULL;
+    }
+
     return opts;
 }
 
diff --git a/qemu-option.h b/qemu-option.h
index c0e022b..951dec3 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -132,7 +132,8 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev);
 void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
                             int permit_abbrev);
-QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict);
+QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
+                               Error **errp);
 QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
 
 typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 10/16] qerror: introduce QERR_INVALID_OPTION_GROUP

[Luiz Capitulino]

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 5092fe7..92c4eff 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 4cbba48..b4c8758 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

[Qemu-devel] [PATCH 11/16] qemu-config: find_list(): use error_set()

[Luiz Capitulino]

Note that qemu_find_opts() and qemu_config_parse() need to call
error_report() to maintain their semantics on error.

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

[Qemu-devel] [PATCH 12/16] qemu-config: introduce qemu_find_opts_err()

[Luiz Capitulino]

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 6d7365d..e9f2ef4 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

[Qemu-devel] [PATCH 13/16] net: purge the monitor object from all init functions

[Luiz Capitulino]

The only backend that really uses it is the socket one, which calls
monitor_get_fd(). But it can use 'cur_mon' instead.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/pci-hotplug.c     |    2 +-
 hw/usb/dev-network.c |    2 +-
 net.c                |   18 +++++++-----------
 net.h                |    2 +-
 net/dump.c           |    2 +-
 net/dump.h           |    3 +--
 net/slirp.c          |    5 +----
 net/slirp.h          |    5 +----
 net/socket.c         |    8 +++-----
 net/socket.h         |    3 +--
 net/tap-win32.c      |    2 +-
 net/tap.c            |    9 ++++-----
 net/tap.h            |    5 ++---
 net/vde.c            |    2 +-
 net/vde.h            |    2 +-
 15 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c55d8b9..785eb3d 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -60,7 +60,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
     qemu_opt_set(opts, "type", "nic");
 
-    ret = net_client_init(mon, opts, 0);
+    ret = net_client_init(opts, 0);
     if (ret < 0)
         return NULL;
     if (nd_table[ret].devaddr) {
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index b238a09..4e26e64 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1367,7 +1367,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
     qemu_opt_set(opts, "type", "nic");
     qemu_opt_set(opts, "model", "usb");
 
-    idx = net_client_init(NULL, opts, 0);
+    idx = net_client_init(opts, 0);
     if (idx == -1) {
         return NULL;
     }
diff --git a/net.c b/net.c
index 246209f..adb7e20 100644
--- a/net.c
+++ b/net.c
@@ -745,10 +745,7 @@ int net_handle_fd_param(Monitor *mon, const char *param)
     return fd;
 }
 
-static int net_init_nic(QemuOpts *opts,
-                        Monitor *mon,
-                        const char *name,
-                        VLANState *vlan)
+static int net_init_nic(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     int idx;
     NICInfo *nd;
@@ -821,7 +818,6 @@ static int net_init_nic(QemuOpts *opts,
      }
 
 typedef int (*net_client_init_func)(QemuOpts *opts,
-                                    Monitor *mon,
                                     const char *name,
                                     VLANState *vlan);
 
@@ -1085,7 +1081,7 @@ static const struct {
 #endif /* CONFIG_NET_BRIDGE */
 };
 
-int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
+int net_client_init(QemuOpts *opts, int is_netdev)
 {
     const char *name;
     const char *type;
@@ -1156,7 +1152,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
 
             ret = 0;
             if (net_client_types[i].init) {
-                ret = net_client_types[i].init(opts, mon, name, vlan);
+                ret = net_client_types[i].init(opts, name, vlan);
                 if (ret < 0) {
                     /* TODO push error reporting into init() methods */
                     qerror_report(QERR_DEVICE_INIT_FAILED, type);
@@ -1213,7 +1209,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 
     qemu_opt_set(opts, "type", device);
 
-    if (net_client_init(mon, opts, 0) < 0) {
+    if (net_client_init(opts, 0) < 0) {
         monitor_printf(mon, "adding host network device %s failed\n", device);
     }
 }
@@ -1248,7 +1244,7 @@ int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }
 
-    res = net_client_init(mon, opts, 1);
+    res = net_client_init(opts, 1);
     if (res < 0) {
         qemu_opts_del(opts);
     }
@@ -1431,14 +1427,14 @@ void net_check_clients(void)
 
 static int net_init_client(QemuOpts *opts, void *dummy)
 {
-    if (net_client_init(NULL, opts, 0) < 0)
+    if (net_client_init(opts, 0) < 0)
         return -1;
     return 0;
 }
 
 static int net_init_netdev(QemuOpts *opts, void *dummy)
 {
-    return net_client_init(NULL, opts, 1);
+    return net_client_init(opts, 1);
 }
 
 int net_init_clients(void)
diff --git a/net.h b/net.h
index 64993b4..9d1ed93 100644
--- a/net.h
+++ b/net.h
@@ -163,7 +163,7 @@ struct HCIInfo *qemu_next_hci(void);
 extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
-int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev);
+int net_client_init(QemuOpts *opts, int is_netdev);
 int net_client_parse(QemuOptsList *opts_list, const char *str);
 int net_init_clients(void);
 void net_check_clients(void);
diff --git a/net/dump.c b/net/dump.c
index 4b48d48..f835c51 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -144,7 +144,7 @@ static int net_dump_init(VLANState *vlan, const char *device,
     return 0;
 }
 
-int net_init_dump(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan)
+int net_init_dump(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     int len;
     const char *file;
diff --git a/net/dump.h b/net/dump.h
index fdc91ad..2b5d9ba 100644
--- a/net/dump.h
+++ b/net/dump.h
@@ -27,7 +27,6 @@
 #include "net.h"
 #include "qemu-common.h"
 
-int net_init_dump(QemuOpts *opts, Monitor *mon,
-                  const char *name, VLANState *vlan);
+int net_init_dump(QemuOpts *opts, const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_DUMP_H */
diff --git a/net/slirp.c b/net/slirp.c
index 96f5032..37b6ccf 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -676,10 +676,7 @@ static int net_init_slirp_configs(const char *name, const char *value, void *opa
     return 0;
 }
 
-int net_init_slirp(QemuOpts *opts,
-                   Monitor *mon,
-                   const char *name,
-                   VLANState *vlan)
+int net_init_slirp(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     struct slirp_config_str *config;
     const char *vhost;
diff --git a/net/slirp.h b/net/slirp.h
index c17de8e..53fe95d 100644
--- a/net/slirp.h
+++ b/net/slirp.h
@@ -30,10 +30,7 @@
 
 #ifdef CONFIG_SLIRP
 
-int net_init_slirp(QemuOpts *opts,
-                   Monitor *mon,
-                   const char *name,
-                   VLANState *vlan);
+int net_init_slirp(QemuOpts *opts, const char *name, VLANState *vlan);
 
 void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict);
 void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict);
diff --git a/net/socket.c b/net/socket.c
index 0bcf229..fcd0a3c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -26,6 +26,7 @@
 #include "config-host.h"
 
 #include "net.h"
+#include "monitor.h"
 #include "qemu-char.h"
 #include "qemu-common.h"
 #include "qemu-error.h"
@@ -585,10 +586,7 @@ static int net_socket_udp_init(VLANState *vlan,
     return 0;
 }
 
-int net_init_socket(QemuOpts *opts,
-                    Monitor *mon,
-                    const char *name,
-                    VLANState *vlan)
+int net_init_socket(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     if (qemu_opt_get(opts, "fd")) {
         int fd;
@@ -601,7 +599,7 @@ int net_init_socket(QemuOpts *opts,
             return -1;
         }
 
-        fd = net_handle_fd_param(mon, qemu_opt_get(opts, "fd"));
+        fd = net_handle_fd_param(cur_mon, qemu_opt_get(opts, "fd"));
         if (fd == -1) {
             return -1;
         }
diff --git a/net/socket.h b/net/socket.h
index ea46f02..e1fe959 100644
--- a/net/socket.h
+++ b/net/socket.h
@@ -27,7 +27,6 @@
 #include "net.h"
 #include "qemu-common.h"
 
-int net_init_socket(QemuOpts *opts, Monitor *mon,
-                    const char *name, VLANState *vlan);
+int net_init_socket(QemuOpts *opts, const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_SOCKET_H */
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 596132e..a801a55 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -699,7 +699,7 @@ static int tap_win32_init(VLANState *vlan, const char *model,
     return 0;
 }
 
-int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan)
+int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     const char *ifname;
 
diff --git a/net/tap.c b/net/tap.c
index f240028..5ac4ba3 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -512,8 +512,7 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
     return -1;
 }
 
-int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name,
-                    VLANState *vlan)
+int net_init_bridge(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     TAPState *s;
     int fd, vnet_hdr;
@@ -583,7 +582,7 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr)
     return fd;
 }
 
-int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan)
+int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     TAPState *s;
     int fd, vnet_hdr = 0;
@@ -600,7 +599,7 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
             return -1;
         }
 
-        fd = net_handle_fd_param(mon, qemu_opt_get(opts, "fd"));
+        fd = net_handle_fd_param(cur_mon, qemu_opt_get(opts, "fd"));
         if (fd == -1) {
             return -1;
         }
@@ -687,7 +686,7 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
         int vhostfd, r;
         bool force = qemu_opt_get_bool(opts, "vhostforce", false);
         if (qemu_opt_get(opts, "vhostfd")) {
-            r = net_handle_fd_param(mon, qemu_opt_get(opts, "vhostfd"));
+            r = net_handle_fd_param(cur_mon, qemu_opt_get(opts, "vhostfd"));
             if (r == -1) {
                 return -1;
             }
diff --git a/net/tap.h b/net/tap.h
index 56c591f..b2a9450 100644
--- a/net/tap.h
+++ b/net/tap.h
@@ -32,7 +32,7 @@
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
 
-int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan);
+int net_init_tap(QemuOpts *opts, const char *name, VLANState *vlan);
 
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required);
 
@@ -57,7 +57,6 @@ int tap_get_fd(VLANClientState *vc);
 struct vhost_net;
 struct vhost_net *tap_get_vhost_net(VLANClientState *vc);
 
-int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name,
-                    VLANState *vlan);
+int net_init_bridge(QemuOpts *opts, const char *name, VLANState *vlan);
 
 #endif /* QEMU_NET_TAP_H */
diff --git a/net/vde.c b/net/vde.c
index ac48ab2..6b9d452 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -110,7 +110,7 @@ static int net_vde_init(VLANState *vlan, const char *model,
     return 0;
 }
 
-int net_init_vde(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan)
+int net_init_vde(QemuOpts *opts, const char *name, VLANState *vlan)
 {
     const char *sock;
     const char *group;
diff --git a/net/vde.h b/net/vde.h
index 3e6ca3e..732e575 100644
--- a/net/vde.h
+++ b/net/vde.h
@@ -29,7 +29,7 @@
 
 #ifdef CONFIG_VDE
 
-int net_init_vde(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan);
+int net_init_vde(QemuOpts *opts, const char *name, VLANState *vlan);
 
 #endif /* CONFIG_VDE */
 
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 14/16] net: net_client_init(): use error_set()

[Luiz Capitulino]

Callers are changed to use qerror_report_err() to keep their QError
semantics.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/pci-hotplug.c     |    8 ++++++--
 hw/usb/dev-network.c |    7 +++++--
 net.c                |   54 +++++++++++++++++++++++++++++++++++---------------
 net.h                |    2 +-
 4 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 785eb3d..61257f4 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -39,6 +39,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
                                        const char *devaddr,
                                        const char *opts_str)
 {
+    Error *local_err = NULL;
     QemuOpts *opts;
     PCIBus *bus;
     int ret, devfn;
@@ -60,9 +61,12 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
     qemu_opt_set(opts, "type", "nic");
 
-    ret = net_client_init(opts, 0);
-    if (ret < 0)
+    ret = net_client_init(opts, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return NULL;
+    }
     if (nd_table[ret].devaddr) {
         monitor_printf(mon, "Parameter addr not supported\n");
         return NULL;
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 4e26e64..5d2f098 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1356,6 +1356,7 @@ static int usb_net_initfn(USBDevice *dev)
 
 static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
 {
+    Error *local_err = NULL;
     USBDevice *dev;
     QemuOpts *opts;
     int idx;
@@ -1367,8 +1368,10 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
     qemu_opt_set(opts, "type", "nic");
     qemu_opt_set(opts, "model", "usb");
 
-    idx = net_client_init(opts, 0);
-    if (idx == -1) {
+    idx = net_client_init(opts, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return NULL;
     }
 
diff --git a/net.c b/net.c
index adb7e20..8f7cb40 100644
--- a/net.c
+++ b/net.c
@@ -1081,7 +1081,7 @@ static const struct {
 #endif /* CONFIG_NET_BRIDGE */
 };
 
-int net_client_init(QemuOpts *opts, int is_netdev)
+int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
 {
     const char *name;
     const char *type;
@@ -1089,7 +1089,7 @@ int net_client_init(QemuOpts *opts, int is_netdev)
 
     type = qemu_opt_get(opts, "type");
     if (!type) {
-        qerror_report(QERR_MISSING_PARAMETER, "type");
+        error_set(errp, QERR_MISSING_PARAMETER, "type");
         return -1;
     }
 
@@ -1105,21 +1105,21 @@ int net_client_init(QemuOpts *opts, int is_netdev)
             strcmp(type, "vde") != 0 &&
 #endif
             strcmp(type, "socket") != 0) {
-            qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
-                          "a netdev backend type");
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                      "a netdev backend type");
             return -1;
         }
 
         if (qemu_opt_get(opts, "vlan")) {
-            qerror_report(QERR_INVALID_PARAMETER, "vlan");
+            error_set(errp, QERR_INVALID_PARAMETER, "vlan");
             return -1;
         }
         if (qemu_opt_get(opts, "name")) {
-            qerror_report(QERR_INVALID_PARAMETER, "name");
+            error_set(errp, QERR_INVALID_PARAMETER, "name");
             return -1;
         }
         if (!qemu_opts_id(opts)) {
-            qerror_report(QERR_MISSING_PARAMETER, "id");
+            error_set(errp, QERR_MISSING_PARAMETER, "id");
             return -1;
         }
     }
@@ -1138,8 +1138,7 @@ int net_client_init(QemuOpts *opts, int is_netdev)
 
             qemu_opts_validate(opts, &net_client_types[i].desc[0], &local_err);
             if (error_is_set(&local_err)) {
-                qerror_report_err(local_err);
-                error_free(local_err);
+                error_propagate(errp, local_err);
                 return -1;
             }
 
@@ -1155,7 +1154,7 @@ int net_client_init(QemuOpts *opts, int is_netdev)
                 ret = net_client_types[i].init(opts, name, vlan);
                 if (ret < 0) {
                     /* TODO push error reporting into init() methods */
-                    qerror_report(QERR_DEVICE_INIT_FAILED, type);
+                    error_set(errp, QERR_DEVICE_INIT_FAILED, type);
                     return -1;
                 }
             }
@@ -1163,8 +1162,8 @@ int net_client_init(QemuOpts *opts, int is_netdev)
         }
     }
 
-    qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
-                  "a network client type");
+    error_set(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+              "a network client type");
     return -1;
 }
 
@@ -1195,6 +1194,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
     const char *opts_str = qdict_get_try_str(qdict, "opts");
+    Error *local_err = NULL;
     QemuOpts *opts;
 
     if (!net_host_check_device(device)) {
@@ -1209,7 +1209,10 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 
     qemu_opt_set(opts, "type", device);
 
-    if (net_client_init(opts, 0) < 0) {
+    net_client_init(opts, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         monitor_printf(mon, "adding host network device %s failed\n", device);
     }
 }
@@ -1244,8 +1247,10 @@ int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }
 
-    res = net_client_init(opts, 1);
+    res = net_client_init(opts, 1, &local_err);
     if (res < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         qemu_opts_del(opts);
     }
 
@@ -1427,14 +1432,31 @@ void net_check_clients(void)
 
 static int net_init_client(QemuOpts *opts, void *dummy)
 {
-    if (net_client_init(opts, 0) < 0)
+    Error *local_err = NULL;
+
+    net_client_init(opts, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return -1;
+    }
+
     return 0;
 }
 
 static int net_init_netdev(QemuOpts *opts, void *dummy)
 {
-    return net_client_init(opts, 1);
+    Error *local_err = NULL;
+    int ret;
+
+    ret = net_client_init(opts, 1, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+
+    return ret;
 }
 
 int net_init_clients(void)
diff --git a/net.h b/net.h
index 9d1ed93..7ee97e9 100644
--- a/net.h
+++ b/net.h
@@ -163,7 +163,7 @@ struct HCIInfo *qemu_next_hci(void);
 extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
-int net_client_init(QemuOpts *opts, int is_netdev);
+int net_client_init(QemuOpts *opts, int is_netdev, Error **errp);
 int net_client_parse(QemuOptsList *opts_list, const char *str);
 int net_init_clients(void);
 void net_check_clients(void);
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 15/16] qapi: convert netdev_add

[Luiz Capitulino]

This is not a full QAPI conversion, but an intermediate step.

In essence, do_netdev_add() is split into three functions:

 1. netdev_add(): performs the actual work. This function is fully
    converted to Error (thus, it's "qapi-friendly")

 2. qmp_netdev_add(): the QMP front-end for netdev_add(). This is
    coded by hand and not auto-generated (gen=no in the schema). The
    reason for this it's a lot easier and simpler to with QemuOpts
    this way

 3. hmp_netdev_add(): HMP front-end.

This design was suggested by Paolo Bonzini.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx  |    3 +--
 hmp.c            |   21 +++++++++++++++++++++
 hmp.h            |    1 +
 net.c            |   36 ++++++++++++++++++++++++------------
 net.h            |    3 ++-
 qapi-schema.json |   28 ++++++++++++++++++++++++++++
 qmp-commands.hx  |    5 +----
 7 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81723c8..d0ce6a5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1037,8 +1037,7 @@ ETEXI
         .args_type  = "netdev:O",
         .params     = "[user|tap|socket],id=str[,prop=value][,...]",
         .help       = "add host network device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_netdev_add,
+        .mhandler.cmd = hmp_netdev_add,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 42ced2a..7a4e25f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -14,6 +14,8 @@
  */
 
 #include "hmp.h"
+#include "net.h"
+#include "qemu-option.h"
 #include "qemu-timer.h"
 #include "qmp-commands.h"
 
@@ -969,3 +971,22 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
                           &errp);
     hmp_handle_error(mon, &errp);
 }
+
+void hmp_netdev_add(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    QemuOpts *opts;
+
+    opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
+    if (error_is_set(&err)) {
+        goto out;
+    }
+
+    netdev_add(opts, &err);
+    if (error_is_set(&err)) {
+        qemu_opts_del(opts);
+    }
+
+out:
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 5cf3241..017df87 100644
--- a/hmp.h
+++ b/hmp.h
@@ -62,5 +62,6 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
+void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/net.c b/net.c
index 8f7cb40..5f0c53c 100644
--- a/net.c
+++ b/net.c
@@ -1234,27 +1234,39 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict)
     qemu_del_vlan_client(vc);
 }
 
-int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void netdev_add(QemuOpts *opts, Error **errp)
+{
+    net_client_init(opts, 1, errp);
+}
+
+int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
 {
     Error *local_err = NULL;
+    QemuOptsList *opts_list;
     QemuOpts *opts;
-    int res;
 
-    opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &local_err);
-    if (!opts) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return -1;
+    opts_list = qemu_find_opts_err("netdev", &local_err);
+    if (error_is_set(&local_err)) {
+        goto exit_err;
     }
 
-    res = net_client_init(opts, 1, &local_err);
-    if (res < 0) {
-        qerror_report_err(local_err);
-        error_free(local_err);
+    opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
+    if (error_is_set(&local_err)) {
+        goto exit_err;
+    }
+
+    netdev_add(opts, &local_err);
+    if (error_is_set(&local_err)) {
         qemu_opts_del(opts);
+        goto exit_err;
     }
 
-    return res;
+    return 0;
+
+exit_err:
+    qerror_report_err(local_err);
+    error_free(local_err);
+    return -1;
 }
 
 int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/net.h b/net.h
index 7ee97e9..1eb9280 100644
--- a/net.h
+++ b/net.h
@@ -170,7 +170,8 @@ void net_check_clients(void);
 void net_cleanup(void);
 void net_host_device_add(Monitor *mon, const QDict *qdict);
 void net_host_device_remove(Monitor *mon, const QDict *qdict);
-int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+void netdev_add(QemuOpts *opts, Error **errp);
+int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret);
 int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
diff --git a/qapi-schema.json b/qapi-schema.json
index dfa0e1f..69fcd8e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1798,3 +1798,31 @@
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
             '*length': 'int' } }
+##
+# @netdev_add:
+#
+# Add a network backend.
+#
+# @type: the type of network backend.  Current valid values are 'user', 'tap',
+#        'vde', 'socket', 'dump' and 'bridge'
+#
+# @id: the name of the new network backend
+#
+# @props: #optional a list of properties to be passed to the backend in
+#         the format 'name=value', like 'ifname=tap0,script=no'
+#
+# Notes: The semantics of @props is not well defined.  Future commands will be
+#        introduced that provide stronger typing for backend creation.
+#
+# Since: 0.14.0
+#
+# Returns: Nothing on success
+#          If @type is not a valid network backend, DeviceNotFound
+#          If @id is not a valid identifier, InvalidParameterValue
+#          if @id already exists, DuplicateId
+#          If @props contains an invalid parameter for this backend,
+#            InvalidParameter
+##
+{ 'command': 'netdev_add',
+  'data': {'type': 'str', 'id': 'str', '*props': '**'},
+  'gen': 'no' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2aa64ad..f6550cb 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -642,10 +642,7 @@ EQMP
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .params     = "[user|tap|socket],id=str[,prop=value][,...]",
-        .help       = "add host network device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_netdev_add,
+        .mhandler.cmd_new = qmp_netdev_add,
     },
 
 SQMP
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 16/16] qapi: convert netdev_del

[Luiz Capitulino]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx  |    3 +--
 hmp.c            |    9 +++++++++
 hmp.h            |    1 +
 net.c            |   11 +++++------
 net.h            |    1 -
 qapi-schema.json |   14 ++++++++++++++
 qmp-commands.hx  |    5 +----
 7 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0ce6a5..f5d9d91 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1051,8 +1051,7 @@ ETEXI
         .args_type  = "id:s",
         .params     = "id",
         .help       = "remove host network device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_netdev_del,
+        .mhandler.cmd = hmp_netdev_del,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 7a4e25f..2ce8cb9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -990,3 +990,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
 out:
     hmp_handle_error(mon, &err);
 }
+
+void hmp_netdev_del(Monitor *mon, const QDict *qdict)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    Error *err = NULL;
+
+    qmp_netdev_del(id, &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 017df87..79d138d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -63,5 +63,6 @@ void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
+void hmp_netdev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/net.c b/net.c
index 5f0c53c..4aa416c 100644
--- a/net.c
+++ b/net.c
@@ -1269,19 +1269,18 @@ exit_err:
     return -1;
 }
 
-int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_netdev_del(const char *id, Error **errp)
 {
-    const char *id = qdict_get_str(qdict, "id");
     VLANClientState *vc;
 
     vc = qemu_find_netdev(id);
     if (!vc) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, id);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+        return;
     }
+
     qemu_del_vlan_client(vc);
-    qemu_opts_del(qemu_opts_find(qemu_find_opts("netdev"), id));
-    return 0;
+    qemu_opts_del(qemu_opts_find(qemu_find_opts_err("netdev", errp), id));
 }
 
 static void print_net_client(Monitor *mon, VLANClientState *vc)
diff --git a/net.h b/net.h
index 1eb9280..bdc2a06 100644
--- a/net.h
+++ b/net.h
@@ -172,7 +172,6 @@ void net_host_device_add(Monitor *mon, const QDict *qdict);
 void net_host_device_remove(Monitor *mon, const QDict *qdict);
 void netdev_add(QemuOpts *opts, Error **errp);
 int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret);
-int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
diff --git a/qapi-schema.json b/qapi-schema.json
index 69fcd8e..bb1f806 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1826,3 +1826,17 @@
 { 'command': 'netdev_add',
   'data': {'type': 'str', 'id': 'str', '*props': '**'},
   'gen': 'no' }
+
+##
+# @netdev_del:
+#
+# Remove a network backend.
+#
+# @id: the name of the network backend to remove
+#
+# Returns: Nothing on success
+#          If @id is not a valid network backend, DeviceNotFound
+#
+# Since: 0.14.0
+##
+{ 'command': 'netdev_del', 'data': {'id': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f6550cb..57ea803 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -671,10 +671,7 @@ EQMP
     {
         .name       = "netdev_del",
         .args_type  = "id:s",
-        .params     = "id",
-        .help       = "remove host network device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_netdev_del,
+        .mhandler.cmd_new = qmp_marshal_input_netdev_del,
     },
 
 SQMP
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [PATCH qmp-next v5 00/16]: qapi: convert netdev_add & netdev_del

[Laszlo Ersek]

On 05/21/12 19:41, Luiz Capitulino wrote:
> v5
> 
> - Simplify set_option_parameter() [Laszlo]

patches changed: 3 & 4

> - Fix bad patch split in patch 15/16, hunk changing net_init_netdev()
>   pertains to patch 14/16 [Laszlo]

hunk squashed from 15 into 14

> 
>  blockdev.c           |    2 +-
>  hmp-commands.hx      |    6 +-
>  hmp.c                |   30 +++++++++
>  hmp.h                |    2 +
>  hw/pci-hotplug.c     |    8 ++-
>  hw/qdev-monitor.c    |    7 +-
>  hw/usb/dev-network.c |    7 +-
>  hw/usb/dev-storage.c |    2 +-
>  hw/watchdog.c        |    2 +-
>  net.c                |  104 ++++++++++++++++++++----------
>  net.h                |    6 +-
>  net/dump.c           |    2 +-
>  net/dump.h           |    3 +-
>  net/slirp.c          |    5 +-
>  net/slirp.h          |    5 +-
>  net/socket.c         |    8 +--
>  net/socket.h         |    3 +-
>  net/tap-win32.c      |    2 +-
>  net/tap.c            |    9 ++-
>  net/tap.h            |    5 +-
>  net/vde.c            |    2 +-
>  net/vde.h            |    2 +-
>  qapi-schema.json     |   42 ++++++++++++
>  qemu-char.c          |    8 ++-
>  qemu-config.c        |   43 ++++++++++---
>  qemu-config.h        |    3 +
>  qemu-option.c        |  175 ++++++++++++++++++++++++++++++++++----------------
>  qemu-option.h        |   11 +++-
>  qemu-sockets.c       |    8 +--
>  qerror.c             |    4 ++
>  qerror.h             |    3 +
>  qmp-commands.hx      |   10 +--
>  vl.c                 |   22 ++++---
>  33 files changed, 380 insertions(+), 171 deletions(-)

Reviewed-By: Laszlo Ersek <lersek@redhat.com>