[Qemu-devel] [PATCH 00/19] Port -net to QemuOpts

[Qemu-devel] [PATCH 00/19] Port -net to QemuOpts

[Mark McLoughlin]

Hi,
        Here's a fairly straightforward port of -net to QemuOpts.

        Chatting to Gerd, he favours:

  1) Adding explicit support to QemuOpts for type specific parameters so
     that it can be used by the chardev code too

  2) Adding a flags field which could be used to indicate that a parameter
     can be specified multiple times

        Both sound good, but IMHO would be fine after this -net series.

        Testing details:

  - I've just now rebased to Anthony's queue, and haven't completely
    re-done my testing. The conflicts were fairly trivial, though

  - Build tested all targets, also with mingw and with VDE enabled

  - Tried to excercise as many of the options as possible, including
    stuff like hostfwd

  - Tested monitor commands too

  - Runtime testing of basic net functionality

Cheers,
Mark.

[Qemu-devel] [PATCH 01/19] Suppress more more kraxelisms

[Mark McLoughlin]

Let's kick off this series with some of the more critical fixes.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/qdev.c     |    2 +-
 qemu-config.c |    2 +-
 qemu-option.c |    6 +++---
 vl.c          |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 6451b8a..c0ef55b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -137,7 +137,7 @@ static int set_property(const char *name, const char *value, void *opaque)
     if (strcmp(name, "bus") == 0)
         return 0;
 
-    if (-1 == qdev_prop_parse(dev, name, value)) {
+    if (qdev_prop_parse(dev, name, value) == -1) {
         qemu_error("can't set property \"%s\" to \"%s\" for \"%s\"\n",
                    name, value, dev->info->name);
         return -1;
diff --git a/qemu-config.c b/qemu-config.c
index f6f4cb4..61e6ed1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -186,7 +186,7 @@ int qemu_set_option(const char *str)
         return -1;
     }
 
-    if (-1 == qemu_opt_set(opts, arg, str+offset+1)) {
+    if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
         fprintf(stderr, "failed to set \"%s\" for %s \"%s\"\n",
                 arg, lists[i]->name, id);
         return -1;
diff --git a/qemu-option.c b/qemu-option.c
index 0c2101e..f1a666f 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -267,7 +267,7 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
     // Process parameter
     switch (list->type) {
     case OPT_FLAG:
-        if (-1 == parse_option_bool(name, value, &flag))
+        if (parse_option_bool(name, value, &flag) == -1)
             return -1;
         list->value.n = flag;
         break;
@@ -282,7 +282,7 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
         break;
 
     case OPT_SIZE:
-        if (-1 == parse_option_size(name, value, &list->value.n))
+        if (parse_option_size(name, value, &list->value.n) == -1)
             return -1;
         break;
 
@@ -745,7 +745,7 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
         }
         if (strcmp(option, "id") != 0) {
             /* store and parse */
-            if (-1 == qemu_opt_set(opts, option, value)) {
+            if (qemu_opt_set(opts, option, value) == -1) {
                 return -1;
             }
         }
diff --git a/vl.c b/vl.c
index 1dc3ffa..6da83a6 100644
--- a/vl.c
+++ b/vl.c
@@ -5153,7 +5153,7 @@ int main(int argc, char **argv, char **envp)
                     fprintf(stderr, "parse error: %s\n", optarg);
                     exit(1);
                 }
-                if (NULL == qemu_chr_open_opts(opts, NULL)) {
+                if (!qemu_chr_open_opts(opts, NULL)) {
                     exit(1);
                 }
                 break;
-- 
1.6.2.5

[Qemu-devel] [PATCH 02/19] Remove bogus error message from qemu_opts_set()

[Mark McLoughlin]

The only way qemu_opts_create() can fail is if a QemuOpts with that id
already exists and fail_if_exists=1. In that case, we already print
an error which makes more sense than the one in qemu_opts_set().

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-option.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index f1a666f..4d544c7 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -669,11 +669,8 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
     QemuOpts *opts;
 
     opts = qemu_opts_create(list, id, 1);
-    if (opts == NULL) {
-        fprintf(stderr, "id \"%s\" not found for \"%s\"\n",
-                id, list->name);
+    if (opts == NULL)
         return -1;
-    }
     return qemu_opt_set(opts, name, value);
 }
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 03/19] Remove double error message in qemu_option_set()

[Mark McLoughlin]

qemu_opt_set() prints an error message in all failure cases, so
qemu_set_option() doesn't need to print another error.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-config.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 61e6ed1..733cc24 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -186,11 +186,9 @@ int qemu_set_option(const char *str)
         return -1;
     }
 
-    if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
-        fprintf(stderr, "failed to set \"%s\" for %s \"%s\"\n",
-                arg, lists[i]->name, id);
+    if (qemu_opt_set(opts, arg, str+offset+1) == -1)
         return -1;
-    }
+
     return 0;
 }
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 04/19] Remove double error message for -device option parsing

[Mark McLoughlin]

qemu_opts_parse() gives a suitable error message in all failure cases
so we can remove the error message from the caller.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 vl.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 6da83a6..ed07d50 100644
--- a/vl.c
+++ b/vl.c
@@ -5251,11 +5251,8 @@ int main(int argc, char **argv, char **envp)
                 add_device_config(DEV_USB, optarg);
                 break;
             case QEMU_OPTION_device:
-                opts = qemu_opts_parse(&qemu_device_opts, optarg, "driver");
-                if (!opts) {
-                    fprintf(stderr, "parse error: %s\n", optarg);
+                if (!qemu_opts_parse(&qemu_device_opts, optarg, "driver"))
                     exit(1);
-                }
                 break;
             case QEMU_OPTION_smp:
                 smp_parse(optarg);
-- 
1.6.2.5

[Qemu-devel] [PATCH 05/19] Make qemu_opts_parse() handle empty strings

[Mark McLoughlin]

Rather than making callers explicitly handle empty strings by using
qemu_opts_create(), we can easily have qemu_opts_parse() handle
empty parameter strings.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-option.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 4d544c7..d312257 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -711,8 +711,7 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
     char option[128], value[128];
     const char *p,*pe,*pc;
 
-    p = params;
-    for(;;) {
+    for (p = params; *p != '\0'; p++) {
         pe = strchr(p, '=');
         pc = strchr(p, ',');
         if (!pe || (pc && pc < pe)) {
@@ -749,7 +748,6 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
         if (*p != ',') {
             break;
         }
-        p++;
     }
     return 0;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 06/19] Add qemu_opts_validate() for post parsing validation

[Mark McLoughlin]

Several qemu command line options have a parameter whose value affects
what other parameters are accepted for the option.

In these cases, we can have an empty description table in the
QemuOptsList and once the option has been parsed we can use a suitable
description table to validate the other parameters based on the value of
that parameter.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-option.c |   32 ++++++++++++++++++++++++++++++++
 qemu-option.h |    1 +
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index d312257..0f517d1 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -777,6 +777,38 @@ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *fi
     return opts;
 }
 
+/* Validate parsed opts against descriptions where no
+ * descriptions were provided in the QemuOptsList.
+ */
+int qemu_opts_validate(QemuOpts *opts, QemuOptDesc *desc)
+{
+    QemuOpt *opt;
+
+    assert(opts->list->desc[0].name == NULL);
+
+    TAILQ_FOREACH(opt, &opts->head, next) {
+        int i;
+
+        for (i = 0; desc[i].name != NULL; i++) {
+            if (strcmp(desc[i].name, opt->name) == 0) {
+                break;
+            }
+        }
+        if (desc[i].name == NULL) {
+            fprintf(stderr, "option \"%s\" is not valid for %s\n",
+                    opt->name, opts->list->name);
+            return -1;
+        }
+
+        opt->desc = &desc[i];
+
+        if (qemu_opt_parse(opt) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque,
                       int abort_on_failure)
 {
diff --git a/qemu-option.h b/qemu-option.h
index 9e52625..e816d95 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -114,6 +114,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, QemuOptDesc *desc);
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname);
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname);
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 07/19] Never overwrite a QemuOpt

[Mark McLoughlin]

Rather than overwriting a QemuOpt, just add a new one to the tail and
always do a reverse search for parameters to preserve the same
behaviour. We use this order so that foreach() iterates over the opts
in their original order.

This will allow us handle options where multiple values for the same
parameter is allowed - e.g. -net user,hostfwd=

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-option.c |   51 +++++++++++++++++++++++----------------------------
 1 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 0f517d1..376efc1 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -483,7 +483,7 @@ struct QemuOpt {
 struct QemuOpts {
     const char *id;
     QemuOptsList *list;
-    TAILQ_HEAD(, QemuOpt) head;
+    TAILQ_HEAD(QemuOptHead, QemuOpt) head;
     TAILQ_ENTRY(QemuOpts) next;
 };
 
@@ -491,7 +491,7 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
 
-    TAILQ_FOREACH(opt, &opts->head, next) {
+    TAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
         if (strcmp(opt->name, name) != 0)
             continue;
         return opt;
@@ -565,36 +565,31 @@ static void qemu_opt_del(QemuOpt *opt)
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
 {
     QemuOpt *opt;
+    QemuOptDesc *desc = opts->list->desc;
+    int i;
 
-    opt = qemu_opt_find(opts, name);
-    if (!opt) {
-        QemuOptDesc *desc = opts->list->desc;
-        int i;
-
-        for (i = 0; desc[i].name != NULL; i++) {
-            if (strcmp(desc[i].name, name) == 0) {
-                break;
-            }
-        }
-        if (desc[i].name == NULL) {
-            if (i == 0) {
-                /* empty list -> allow any */;
-            } else {
-                fprintf(stderr, "option \"%s\" is not valid for %s\n",
-                        name, opts->list->name);
-                return -1;
-            }
+    for (i = 0; desc[i].name != NULL; i++) {
+        if (strcmp(desc[i].name, name) == 0) {
+            break;
         }
-        opt = qemu_mallocz(sizeof(*opt));
-        opt->name = qemu_strdup(name);
-        opt->opts = opts;
-        TAILQ_INSERT_TAIL(&opts->head, opt, next);
-        if (desc[i].name != NULL) {
-            opt->desc = desc+i;
+    }
+    if (desc[i].name == NULL) {
+        if (i == 0) {
+            /* empty list -> allow any */;
+        } else {
+            fprintf(stderr, "option \"%s\" is not valid for %s\n",
+                    name, opts->list->name);
+            return -1;
         }
     }
-    qemu_free((/* !const */ char*)opt->str);
-    opt->str = NULL;
+
+    opt = qemu_mallocz(sizeof(*opt));
+    opt->name = qemu_strdup(name);
+    opt->opts = opts;
+    TAILQ_INSERT_TAIL(&opts->head, opt, next);
+    if (desc[i].name != NULL) {
+        opt->desc = desc+i;
+    }
     if (value) {
         opt->str = qemu_strdup(value);
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH 08/19] Add qemu_net_opts

[Mark McLoughlin]

The first step in porting -net to QemuOpts. We do not include parameter
descriptions in the QemuOptsList because we use the first parameter to
choose which descriptions validate against.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-config.c |   13 +++++++++++++
 qemu-config.h |    1 +
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 733cc24..275de4f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -151,10 +151,23 @@ QemuOptsList qemu_device_opts = {
     },
 };
 
+QemuOptsList qemu_net_opts = {
+    .name = "net",
+    .head = TAILQ_HEAD_INITIALIZER(qemu_net_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end if list */ }
+    },
+};
+
 static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
     &qemu_device_opts,
+    &qemu_net_opts,
     NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index 13b0f19..852fe52 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -4,6 +4,7 @@
 extern QemuOptsList qemu_drive_opts;
 extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
+extern QemuOptsList qemu_net_opts;
 
 int qemu_set_option(const char *str);
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 09/19] Port -net none and -net nic to QemuOpts

[Mark McLoughlin]

We use a table of network types to look up the initialization function
and parameter descriptions in net_client_init().

For now, we use QemuOpts for the 'none' and 'nic' types. Subsequent
patches port the other types too and the special casing is removed.

We're not parsing the full -net option string here as the type has
been stripped from the string, so we do not use qemu_opts_parse()
'firstname' facility. This will also be rectified in subsequent
patches.

No functional changes are introduced by this patch.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |  227 +++++++++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 150 insertions(+), 77 deletions(-)

diff --git a/net.c b/net.c
index 340177e..b002ae5 100644
--- a/net.c
+++ b/net.c
@@ -112,6 +112,7 @@
 #include "audio/audio.h"
 #include "qemu_socket.h"
 #include "qemu-log.h"
+#include "qemu-config.h"
 
 #include "slirp/libslirp.h"
 
@@ -2400,6 +2401,141 @@ static int net_handle_fd_param(Monitor *mon, const char *param)
     }
 }
 
+static int net_init_nic(QemuOpts *opts, Monitor *mon)
+{
+    int idx;
+    NICInfo *nd;
+
+    idx = nic_get_free_idx();
+    if (idx == -1 || nb_nics >= MAX_NICS) {
+        qemu_error("Too Many NICs\n");
+        return -1;
+    }
+
+    nd = &nd_table[idx];
+
+    memset(nd, 0, sizeof(*nd));
+
+    nd->vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    nd->id      = qemu_opts_id(opts);
+    nd->name    = qemu_opt_get(opts, "name");
+    nd->model   = qemu_opt_get(opts, "model");
+    nd->devaddr = qemu_opt_get(opts, "addr");
+
+    nd->macaddr[0] = 0x52;
+    nd->macaddr[1] = 0x54;
+    nd->macaddr[2] = 0x00;
+    nd->macaddr[3] = 0x12;
+    nd->macaddr[4] = 0x34;
+    nd->macaddr[5] = 0x56 + idx;
+
+    if (qemu_opt_get(opts, "macaddr") &&
+        parse_macaddr(nd->macaddr, qemu_opt_get(opts, "macaddr")) < 0) {
+        qemu_error("invalid syntax for ethernet address\n");
+        return -1;
+    }
+
+    nd->nvectors = qemu_opt_get_number(opts, "vectors", NIC_NVECTORS_UNSPECIFIED);
+    if (nd->nvectors != NIC_NVECTORS_UNSPECIFIED &&
+        (nd->nvectors < 0 || nd->nvectors > 0x7ffffff)) {
+        qemu_error("invalid # of vectors: %d\n", nd->nvectors);
+        return -1;
+    }
+
+    nd->used = 1;
+    nd->vlan->nb_guest_devs++;
+    nb_nics++;
+
+    return idx;
+}
+
+#define NET_COMMON_PARAMS_DESC                     \
+    {                                              \
+        .name = "type",                            \
+        .type = QEMU_OPT_STRING,                   \
+        .help = "net client type (nic, tap etc.)", \
+     }, {                                          \
+        .name = "vlan",                            \
+        .type = QEMU_OPT_NUMBER,                   \
+        .help = "vlan number",                     \
+     }, {                                          \
+        .name = "name",                            \
+        .type = QEMU_OPT_STRING,                   \
+        .help = "identifier for monitor commands", \
+     }
+
+typedef int (*net_client_init_func)(QemuOpts *opts, Monitor *mon);
+
+/* magic number, but compiler will warn if too small */
+#define NET_MAX_DESC 20
+
+static struct {
+    const char *type;
+    net_client_init_func init;
+    QemuOptDesc desc[NET_MAX_DESC];
+} net_client_types[] = {
+    {
+        .type = "none",
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            { /* end of list */ }
+        },
+    }, {
+        .type = "nic",
+        .init = net_init_nic,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "macaddr",
+                .type = QEMU_OPT_STRING,
+                .help = "MAC address",
+            }, {
+                .name = "model",
+                .type = QEMU_OPT_STRING,
+                .help = "device model (e1000, rtl8139, virtio etc.)",
+            }, {
+                .name = "addr",
+                .type = QEMU_OPT_STRING,
+                .help = "PCI device address",
+            }, {
+                .name = "vectors",
+                .type = QEMU_OPT_NUMBER,
+                .help = "number of MSI-x vectors, 0 to disable MSI-X",
+            },
+            { /* end of list */ }
+        },
+    },
+    { /* end of list */ }
+};
+
+static int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
+{
+    const char *type;
+    int i;
+
+    type = qemu_opt_get(opts, "type");
+    if (!type) {
+        qemu_error("No type specified for -net\n");
+        return -1;
+    }
+
+    for (i = 0; net_client_types[i].type != NULL; i++) {
+        if (!strcmp(net_client_types[i].type, type)) {
+            if (qemu_opts_validate(opts, &net_client_types[i].desc[0]) == -1)
+                return -1;
+
+            if (net_client_types[i].init)
+                return net_client_types[i].init(opts, NULL);
+            else
+                return 0;
+        }
+    }
+
+    qemu_error("Invalid -net type '%s'\n", type);
+    return -1;
+}
+
 int net_client_init(Monitor *mon, const char *device, const char *p)
 {
     char buf[1024];
@@ -2407,6 +2543,19 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     VLANState *vlan;
     char *name = NULL;
 
+    if (!strcmp(device, "none") ||
+        !strcmp(device, "nic")) {
+        QemuOpts *opts;
+
+        opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
+        if (!opts)
+            return -1;
+
+        qemu_opt_set(opts, "type", device);
+
+        return net_client_init_from_opts(mon, opts);
+    }
+
     vlan_id = 0;
     if (get_param_value(buf, sizeof(buf), "vlan", p)) {
         vlan_id = strtol(buf, NULL, 0);
@@ -2416,83 +2565,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     if (get_param_value(buf, sizeof(buf), "name", p)) {
         name = qemu_strdup(buf);
     }
-    if (!strcmp(device, "nic")) {
-        static const char * const nic_params[] = {
-            "vlan", "name", "macaddr", "model", "addr", "id", "vectors", NULL
-        };
-        NICInfo *nd;
-        uint8_t *macaddr;
-        int idx = nic_get_free_idx();
 
-        if (check_params(buf, sizeof(buf), nic_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        if (idx == -1 || nb_nics >= MAX_NICS) {
-            config_error(mon, "Too Many NICs\n");
-            ret = -1;
-            goto out;
-        }
-        nd = &nd_table[idx];
-        macaddr = nd->macaddr;
-        macaddr[0] = 0x52;
-        macaddr[1] = 0x54;
-        macaddr[2] = 0x00;
-        macaddr[3] = 0x12;
-        macaddr[4] = 0x34;
-        macaddr[5] = 0x56 + idx;
-
-        if (get_param_value(buf, sizeof(buf), "macaddr", p)) {
-            if (parse_macaddr(macaddr, buf) < 0) {
-                config_error(mon, "invalid syntax for ethernet address\n");
-                ret = -1;
-                goto out;
-            }
-        }
-        if (get_param_value(buf, sizeof(buf), "model", p)) {
-            nd->model = strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "addr", p)) {
-            nd->devaddr = strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "id", p)) {
-            nd->id = strdup(buf);
-        }
-        nd->nvectors = NIC_NVECTORS_UNSPECIFIED;
-        if (get_param_value(buf, sizeof(buf), "vectors", p)) {
-            char *endptr;
-            long vectors = strtol(buf, &endptr, 0);
-            if (*endptr) {
-                config_error(mon, "invalid syntax for # of vectors\n");
-                ret = -1;
-                goto out;
-            }
-            if (vectors < 0 || vectors > 0x7ffffff) {
-                config_error(mon, "invalid # of vectors\n");
-                ret = -1;
-                goto out;
-            }
-            nd->nvectors = vectors;
-        }
-        nd->vlan = vlan;
-        nd->name = name;
-        nd->used = 1;
-        name = NULL;
-        nb_nics++;
-        vlan->nb_guest_devs++;
-        ret = idx;
-    } else
-    if (!strcmp(device, "none")) {
-        if (*p != '\0') {
-            config_error(mon, "'none' takes no parameters\n");
-            ret = -1;
-            goto out;
-        }
-        /* does nothing. It is needed to signal that no network cards
-           are wanted */
-        ret = 0;
-    } else
 #ifdef CONFIG_SLIRP
     if (!strcmp(device, "user")) {
         static const char * const slirp_params[] = {
-- 
1.6.2.5

[Qemu-devel] [PATCH 10/19] Port -net user to QemuOpts

[Mark McLoughlin]

The handling of guestfwd and hostfwd requires the previous changes
to allow multiple values for each parameter. The only way to access
those multiple values is to use qemu_opt_foreach().

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |  250 +++++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 149 insertions(+), 101 deletions(-)

diff --git a/net.c b/net.c
index b002ae5..aa5a677 100644
--- a/net.c
+++ b/net.c
@@ -2450,6 +2450,90 @@ static int net_init_nic(QemuOpts *opts, Monitor *mon)
     return idx;
 }
 
+static int net_init_slirp_configs(const char *name, const char *value, void *opaque)
+{
+    struct slirp_config_str *config;
+
+    if (strcmp(name, "hostfwd") != 0 && strcmp(name, "guestfwd") != 0)
+        return 0;
+
+    config = qemu_mallocz(sizeof(*config));
+
+    pstrcpy(config->str, sizeof(config->str), value);
+
+    if (!strcmp(name, "hostfwd"))
+        config->flags = SLIRP_CFG_HOSTFWD;
+
+    config->next = slirp_configs;
+    slirp_configs = config;
+
+    return 0;
+}
+
+static int net_init_slirp(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    const char *name;
+    const char *vhost;
+    const char *vhostname;
+    const char *vdhcp_start;
+    const char *vnamesrv;
+    const char *tftp_export;
+    const char *bootfile;
+    const char *smb_export;
+    const char *vsmbsrv;
+    char *vnet = NULL;
+    int restricted = 0;
+    int ret;
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name = qemu_opt_get(opts, "name");
+
+    vhost       = qemu_opt_get(opts, "host");
+    vhostname   = qemu_opt_get(opts, "hostname");
+    vdhcp_start = qemu_opt_get(opts, "dhcpstart");
+    vnamesrv    = qemu_opt_get(opts, "dns");
+    tftp_export = qemu_opt_get(opts, "tftp");
+    bootfile    = qemu_opt_get(opts, "bootfile");
+    smb_export  = qemu_opt_get(opts, "smb");
+    vsmbsrv     = qemu_opt_get(opts, "smbserver");
+
+    if (qemu_opt_get(opts, "ip")) {
+        const char *ip = qemu_opt_get(opts, "ip");
+        int l = strlen(ip) + strlen("/24") + 1;
+
+        vnet = qemu_malloc(l);
+
+        /* emulate legacy ip= parameter */
+        pstrcpy(vnet, l, ip);
+        pstrcat(vnet, l, "/24");
+    }
+
+    if (qemu_opt_get(opts, "net")) {
+        if (vnet)
+            qemu_free(vnet);
+        vnet = qemu_strdup(qemu_opt_get(opts, "net"));
+    }
+
+    if (qemu_opt_get(opts, "restrict") &&
+        qemu_opt_get(opts, "restrict")[0] == 'y')
+        restricted = 1;
+
+    qemu_opt_foreach(opts, net_init_slirp_configs, NULL, 0);
+
+    ret = net_slirp_init(mon, vlan, "user", name, restricted, vnet, vhost,
+                         vhostname, tftp_export, bootfile, vdhcp_start,
+                         vnamesrv, smb_export, vsmbsrv);
+
+    if (ret != -1)
+        vlan->nb_host_devs++;
+
+    qemu_free(vnet);
+
+    return ret;
+}
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2505,6 +2589,68 @@ static struct {
             },
             { /* end of list */ }
         },
+#ifdef CONFIG_SLIRP
+    }, {
+        .type = "user",
+        .init = net_init_slirp,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "hostname",
+                .type = QEMU_OPT_STRING,
+                .help = "client hostname reported by the builtin DHCP server",
+            }, {
+                .name = "restrict",
+                .type = QEMU_OPT_STRING,
+                .help = "isolate the guest from the host (y|yes|n|no)",
+            }, {
+                .name = "ip",
+                .type = QEMU_OPT_STRING,
+                .help = "legacy parameter, use net= instead",
+            }, {
+                .name = "net",
+                .type = QEMU_OPT_STRING,
+                .help = "IP address and optional netmask",
+            }, {
+                .name = "host",
+                .type = QEMU_OPT_STRING,
+                .help = "guest-visible address of the host",
+            }, {
+                .name = "tftp",
+                .type = QEMU_OPT_STRING,
+                .help = "root directory of the built-in TFTP server",
+            }, {
+                .name = "bootfile",
+                .type = QEMU_OPT_STRING,
+                .help = "BOOTP filename, for use with tftp=",
+            }, {
+                .name = "dhcpstart",
+                .type = QEMU_OPT_STRING,
+                .help = "the first of the 16 IPs the built-in DHCP server can assign",
+            }, {
+                .name = "dns",
+                .type = QEMU_OPT_STRING,
+                .help = "guest-visible address of the virtual nameserver",
+            }, {
+                .name = "smb",
+                .type = QEMU_OPT_STRING,
+                .help = "root directory of the built-in SMB server",
+            }, {
+                .name = "smbserver",
+                .type = QEMU_OPT_STRING,
+                .help = "IP address of the built-in SMB server",
+            }, {
+                .name = "hostfwd",
+                .type = QEMU_OPT_STRING,
+                .help = "guest port number to forward incoming TCP or UDP connections",
+            }, {
+                .name = "guestfwd",
+                .type = QEMU_OPT_STRING,
+                .help = "IP address and port to forward guest TCP connections",
+            },
+            { /* end of list */ }
+        },
+#endif
     },
     { /* end of list */ }
 };
@@ -2544,7 +2690,8 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     char *name = NULL;
 
     if (!strcmp(device, "none") ||
-        !strcmp(device, "nic")) {
+        !strcmp(device, "nic") ||
+        !strcmp(device, "user")) {
         QemuOpts *opts;
 
         opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
@@ -2567,106 +2714,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     }
 
 #ifdef CONFIG_SLIRP
-    if (!strcmp(device, "user")) {
-        static const char * const slirp_params[] = {
-            "vlan", "name", "hostname", "restrict", "ip", "net", "host",
-            "tftp", "bootfile", "dhcpstart", "dns", "smb", "smbserver",
-            "hostfwd", "guestfwd", NULL
-        };
-        struct slirp_config_str *config;
-        int restricted = 0;
-        char *vnet = NULL;
-        char *vhost = NULL;
-        char *vhostname = NULL;
-        char *tftp_export = NULL;
-        char *bootfile = NULL;
-        char *vdhcp_start = NULL;
-        char *vnamesrv = NULL;
-        char *smb_export = NULL;
-        char *vsmbsrv = NULL;
-        const char *q;
-
-        if (check_params(buf, sizeof(buf), slirp_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        if (get_param_value(buf, sizeof(buf), "ip", p)) {
-            int vnet_buflen = strlen(buf) + strlen("/24") + 1;
-            /* emulate legacy parameter */
-            vnet = qemu_malloc(vnet_buflen);
-            pstrcpy(vnet, vnet_buflen, buf);
-            pstrcat(vnet, vnet_buflen, "/24");
-        }
-        if (get_param_value(buf, sizeof(buf), "net", p)) {
-            vnet = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "host", p)) {
-            vhost = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "hostname", p)) {
-            vhostname = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "restrict", p)) {
-            restricted = (buf[0] == 'y') ? 1 : 0;
-        }
-        if (get_param_value(buf, sizeof(buf), "dhcpstart", p)) {
-            vdhcp_start = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "dns", p)) {
-            vnamesrv = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "tftp", p)) {
-            tftp_export = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "bootfile", p)) {
-            bootfile = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "smb", p)) {
-            smb_export = qemu_strdup(buf);
-            if (get_param_value(buf, sizeof(buf), "smbserver", p)) {
-                vsmbsrv = qemu_strdup(buf);
-            }
-        }
-        q = p;
-        while (1) {
-            config = qemu_malloc(sizeof(*config));
-            if (!get_next_param_value(config->str, sizeof(config->str),
-                                      "hostfwd", &q)) {
-                break;
-            }
-            config->flags = SLIRP_CFG_HOSTFWD;
-            config->next = slirp_configs;
-            slirp_configs = config;
-            config = NULL;
-        }
-        q = p;
-        while (1) {
-            config = qemu_malloc(sizeof(*config));
-            if (!get_next_param_value(config->str, sizeof(config->str),
-                                      "guestfwd", &q)) {
-                break;
-            }
-            config->flags = 0;
-            config->next = slirp_configs;
-            slirp_configs = config;
-            config = NULL;
-        }
-        qemu_free(config);
-        vlan->nb_host_devs++;
-        ret = net_slirp_init(mon, vlan, device, name, restricted, vnet, vhost,
-                             vhostname, tftp_export, bootfile, vdhcp_start,
-                             vnamesrv, smb_export, vsmbsrv);
-        qemu_free(vnet);
-        qemu_free(vhost);
-        qemu_free(vhostname);
-        qemu_free(tftp_export);
-        qemu_free(bootfile);
-        qemu_free(vdhcp_start);
-        qemu_free(vnamesrv);
-        qemu_free(smb_export);
-        qemu_free(vsmbsrv);
-    } else if (!strcmp(device, "channel")) {
+    if (!strcmp(device, "channel")) {
         if (TAILQ_EMPTY(&slirp_stacks)) {
             struct slirp_config_str *config;
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 11/19] Port -net tap to QemuOpts

[Mark McLoughlin]

Some parameters are not valid with fd=. Rather than having a separate
parameter description table for validating fd=, it's easir to just
check for those invalid parameters later.

Note, the need to possible lookup a file descriptor name from the
monitor is the reason why all these init functions are passed a Monitor
pointer.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |  219 +++++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 132 insertions(+), 87 deletions(-)

diff --git a/net.c b/net.c
index aa5a677..55792a5 100644
--- a/net.c
+++ b/net.c
@@ -1377,25 +1377,22 @@ static void tap_send(void *opaque)
  */
 #define TAP_DEFAULT_SNDBUF 1024*1024
 
-static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+static void tap_set_sndbuf(TAPState *s, QemuOpts *opts, Monitor *mon)
 {
-    int sndbuf = TAP_DEFAULT_SNDBUF;
-
-    if (sndbuf_str) {
-        sndbuf = atoi(sndbuf_str);
-    }
+    int sndbuf;
 
+    sndbuf = qemu_opt_get_size(opts, "sndbuf", TAP_DEFAULT_SNDBUF);
     if (!sndbuf) {
         sndbuf = INT_MAX;
     }
 
-    if (ioctl(s->fd, TUNSETSNDBUF, &sndbuf) == -1 && sndbuf_str) {
+    if (ioctl(s->fd, TUNSETSNDBUF, &sndbuf) == -1 && qemu_opt_get(opts, "sndbuf")) {
         config_error(mon, "TUNSETSNDBUF ioctl failed: %s\n",
                      strerror(errno));
     }
 }
 #else
-static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+static void tap_set_sndbuf(TAPState *s, QemuOpts *opts, Monitor *mon)
 {
     if (sndbuf_str) {
         config_error(mon, "No '-net tap,sndbuf=<nbytes>' support available\n");
@@ -2534,6 +2531,86 @@ static int net_init_slirp(QemuOpts *opts, Monitor *mon)
     return ret;
 }
 
+#ifdef _WIN32
+static int net_init_tap_win32(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    const char *name;
+    const char *ifname;
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name   = qemu_opt_get(opts, "name");
+    ifname = qemu_opt_get(opts, "ifname");
+
+    if (!ifname) {
+        qemu_error("tap: no interface name\n");
+        return -1;
+    }
+
+    if (tap_win32_init(vlan, "tap", name, ifname) == -1)
+        return -1;
+
+    vlan->nb_host_devs++;
+
+    return 0;
+}
+#elif !defined(_AIX)
+static int net_init_tap(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    const char *name;
+    TAPState *s;
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name = qemu_opt_get(opts, "name");
+
+    if (qemu_opt_get(opts, "fd")) {
+        int fd;
+
+        if (qemu_opt_get(opts, "ifname") ||
+            qemu_opt_get(opts, "script") ||
+            qemu_opt_get(opts, "downscript")) {
+            qemu_error("ifname=, script= and downscript= is invalid with fd=\n");
+            return -1;
+        }
+
+        fd = net_handle_fd_param(mon, qemu_opt_get(opts, "fd"));
+        if (fd == -1)
+            return -1;
+
+        fcntl(fd, F_SETFL, O_NONBLOCK);
+
+        s = net_tap_fd_init(vlan, "tap", name, fd);
+        if (!s)
+            close(fd);
+    } else {
+        const char *ifname, *script, *downscript;
+
+        ifname     = qemu_opt_get(opts, "ifname");
+        script     = qemu_opt_get(opts, "script");
+        downscript = qemu_opt_get(opts, "downscript");
+
+        if (!script)
+            script = DEFAULT_NETWORK_SCRIPT;
+        if (!downscript)
+            downscript = DEFAULT_NETWORK_DOWN_SCRIPT;
+
+        s = net_tap_init(vlan, "tap", name, ifname, script, downscript);
+    }
+
+    if (!s)
+        return -1;
+
+    tap_set_sndbuf(s, opts, mon);
+
+    vlan->nb_host_devs++;
+
+    return 0;
+}
+#endif
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2651,6 +2728,51 @@ static struct {
             { /* end of list */ }
         },
 #endif
+#ifdef _WIN32
+    }, {
+        .type = "tap",
+        .init = net_init_tap_win32,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "ifname",
+                .type = QEMU_OPT_STRING,
+                .help = "interface name",
+            },
+            { /* end of list */ }
+        },
+#elif !defined(_AIX)
+    }, {
+        .type = "tap",
+        .init = net_init_tap,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "fd",
+                .type = QEMU_OPT_STRING,
+                .help = "file descriptor of an already opened tap",
+            }, {
+                .name = "ifname",
+                .type = QEMU_OPT_STRING,
+                .help = "interface name",
+            }, {
+                .name = "script",
+                .type = QEMU_OPT_STRING,
+                .help = "script to initialize the interface",
+            }, {
+                .name = "downscript",
+                .type = QEMU_OPT_STRING,
+                .help = "script to shut down the interface",
+#ifdef TUNSETSNDBUF
+            }, {
+                .name = "sndbuf",
+                .type = QEMU_OPT_SIZE,
+                .help = "send buffer limit"
+#endif
+            },
+            { /* end of list */ }
+        },
+#endif
     },
     { /* end of list */ }
 };
@@ -2691,7 +2813,8 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
 
     if (!strcmp(device, "none") ||
         !strcmp(device, "nic") ||
-        !strcmp(device, "user")) {
+        !strcmp(device, "user") ||
+        !strcmp(device, "tap")) {
         QemuOpts *opts;
 
         opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
@@ -2729,84 +2852,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         ret = 0;
     } else
 #endif
-#ifdef _WIN32
-    if (!strcmp(device, "tap")) {
-        static const char * const tap_params[] = {
-            "vlan", "name", "ifname", NULL
-        };
-        char ifname[64];
-
-        if (check_params(buf, sizeof(buf), tap_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        if (get_param_value(ifname, sizeof(ifname), "ifname", p) <= 0) {
-            config_error(mon, "tap: no interface name\n");
-            ret = -1;
-            goto out;
-        }
-        vlan->nb_host_devs++;
-        ret = tap_win32_init(vlan, device, name, ifname);
-    } else
-#elif defined (_AIX)
-#else
-    if (!strcmp(device, "tap")) {
-        char ifname[64], chkbuf[64];
-        char setup_script[1024], down_script[1024];
-        TAPState *s;
-        int fd;
-        vlan->nb_host_devs++;
-        if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
-            static const char * const fd_params[] = {
-                "vlan", "name", "fd", "sndbuf", NULL
-            };
-            ret = -1;
-            if (check_params(chkbuf, sizeof(chkbuf), fd_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                goto out;
-            }
-            fd = net_handle_fd_param(mon, buf);
-            if (fd == -1) {
-                goto out;
-            }
-            fcntl(fd, F_SETFL, O_NONBLOCK);
-            s = net_tap_fd_init(vlan, device, name, fd);
-            if (!s) {
-                close(fd);
-            }
-        } else {
-            static const char * const tap_params[] = {
-                "vlan", "name", "ifname", "script", "downscript", "sndbuf", NULL
-            };
-            if (check_params(chkbuf, sizeof(chkbuf), tap_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                ret = -1;
-                goto out;
-            }
-            if (get_param_value(ifname, sizeof(ifname), "ifname", p) <= 0) {
-                ifname[0] = '\0';
-            }
-            if (get_param_value(setup_script, sizeof(setup_script), "script", p) == 0) {
-                pstrcpy(setup_script, sizeof(setup_script), DEFAULT_NETWORK_SCRIPT);
-            }
-            if (get_param_value(down_script, sizeof(down_script), "downscript", p) == 0) {
-                pstrcpy(down_script, sizeof(down_script), DEFAULT_NETWORK_DOWN_SCRIPT);
-            }
-            s = net_tap_init(vlan, device, name, ifname, setup_script, down_script);
-        }
-        if (s != NULL) {
-            const char *sndbuf_str = NULL;
-            if (get_param_value(buf, sizeof(buf), "sndbuf", p)) {
-                sndbuf_str = buf;
-            }
-            tap_set_sndbuf(s, sndbuf_str, mon);
-            ret = 0;
-        } else {
-            ret = -1;
-        }
-    } else
-#endif
     if (!strcmp(device, "socket")) {
         char chkbuf[64];
         if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
-- 
1.6.2.5

[Qemu-devel] [PATCH 12/19] Port -net socket to QemuOpts

[Mark McLoughlin]

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |  168 ++++++++++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 109 insertions(+), 59 deletions(-)

diff --git a/net.c b/net.c
index 55792a5..c3414eb 100644
--- a/net.c
+++ b/net.c
@@ -2611,6 +2611,89 @@ static int net_init_tap(QemuOpts *opts, Monitor *mon)
 }
 #endif
 
+static int net_init_socket(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    const char *name;
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name = qemu_opt_get(opts, "name");
+
+    if (qemu_opt_get(opts, "fd")) {
+        int fd;
+
+        if (qemu_opt_get(opts, "listen") ||
+            qemu_opt_get(opts, "connect") ||
+            qemu_opt_get(opts, "mcast")) {
+            qemu_error("listen=, connect= and mcast= is invalid with fd=\n");
+            return -1;
+        }
+
+        fd = net_handle_fd_param(mon, qemu_opt_get(opts, "fd"));
+        if (fd == -1) {
+            return -1;
+        }
+
+        if (!net_socket_fd_init(vlan, "socket", name, fd, 1)) {
+            close(fd);
+            return -1;
+        }
+    } else if (qemu_opt_get(opts, "listen")) {
+        const char *listen;
+
+        if (qemu_opt_get(opts, "fd") ||
+            qemu_opt_get(opts, "connect") ||
+            qemu_opt_get(opts, "mcast")) {
+            qemu_error("fd=, connect= and mcast= is invalid with listen=\n");
+            return -1;
+        }
+
+        listen = qemu_opt_get(opts, "listen");
+
+        if (net_socket_listen_init(vlan, "socket", name, listen) == -1) {
+            return -1;
+        }
+    } else if (qemu_opt_get(opts, "connect")) {
+        const char *connect;
+
+        if (qemu_opt_get(opts, "fd") ||
+            qemu_opt_get(opts, "listen") ||
+            qemu_opt_get(opts, "mcast")) {
+            qemu_error("fd=, listen= and mcast= is invalid with connect=\n");
+            return -1;
+        }
+
+        connect = qemu_opt_get(opts, "connect");
+
+        if (net_socket_connect_init(vlan, "socket", name, connect) == -1) {
+            return -1;
+        }
+    } else if (qemu_opt_get(opts, "mcast")) {
+        const char *mcast;
+
+        if (qemu_opt_get(opts, "fd") ||
+            qemu_opt_get(opts, "connect") ||
+            qemu_opt_get(opts, "listen")) {
+            qemu_error("fd=, connect= and listen= is invalid with mcast=\n");
+            return -1;
+        }
+
+        mcast = qemu_opt_get(opts, "mcast");
+
+        if (net_socket_mcast_init(vlan, "socket", name, mcast) == -1) {
+            return -1;
+        }
+    } else {
+        qemu_error("-socket requires fd=, listen=, connect= or mcast=\n");
+        return -1;
+    }
+
+    vlan->nb_host_devs++;
+
+    return 0;
+}
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2773,6 +2856,30 @@ static struct {
             { /* end of list */ }
         },
 #endif
+    }, {
+        .type = "socket",
+        .init = net_init_socket,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "fd",
+                .type = QEMU_OPT_STRING,
+                .help = "file descriptor of an already opened socket",
+            }, {
+                .name = "listen",
+                .type = QEMU_OPT_STRING,
+                .help = "port number, and optional hostname, to listen on",
+            }, {
+                .name = "connect",
+                .type = QEMU_OPT_STRING,
+                .help = "port number, and optional hostname, to connect to",
+            }, {
+                .name = "mcast",
+                .type = QEMU_OPT_STRING,
+                .help = "UDP multicast address and port number",
+            },
+            { /* end of list */ }
+        },
     },
     { /* end of list */ }
 };
@@ -2814,7 +2921,8 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     if (!strcmp(device, "none") ||
         !strcmp(device, "nic") ||
         !strcmp(device, "user") ||
-        !strcmp(device, "tap")) {
+        !strcmp(device, "tap") ||
+        !strcmp(device, "socket")) {
         QemuOpts *opts;
 
         opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
@@ -2852,64 +2960,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         ret = 0;
     } else
 #endif
-    if (!strcmp(device, "socket")) {
-        char chkbuf[64];
-        if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
-            static const char * const fd_params[] = {
-                "vlan", "name", "fd", NULL
-            };
-            int fd;
-            ret = -1;
-            if (check_params(chkbuf, sizeof(chkbuf), fd_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                goto out;
-            }
-            fd = net_handle_fd_param(mon, buf);
-            if (fd == -1) {
-                goto out;
-            }
-            if (!net_socket_fd_init(vlan, device, name, fd, 1)) {
-                close(fd);
-                goto out;
-            }
-            ret = 0;
-        } else if (get_param_value(buf, sizeof(buf), "listen", p) > 0) {
-            static const char * const listen_params[] = {
-                "vlan", "name", "listen", NULL
-            };
-            if (check_params(chkbuf, sizeof(chkbuf), listen_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                ret = -1;
-                goto out;
-            }
-            ret = net_socket_listen_init(vlan, device, name, buf);
-        } else if (get_param_value(buf, sizeof(buf), "connect", p) > 0) {
-            static const char * const connect_params[] = {
-                "vlan", "name", "connect", NULL
-            };
-            if (check_params(chkbuf, sizeof(chkbuf), connect_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                ret = -1;
-                goto out;
-            }
-            ret = net_socket_connect_init(vlan, device, name, buf);
-        } else if (get_param_value(buf, sizeof(buf), "mcast", p) > 0) {
-            static const char * const mcast_params[] = {
-                "vlan", "name", "mcast", NULL
-            };
-            if (check_params(chkbuf, sizeof(chkbuf), mcast_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
-                ret = -1;
-                goto out;
-            }
-            ret = net_socket_mcast_init(vlan, device, name, buf);
-        } else {
-            config_error(mon, "Unknown socket options: %s\n", p);
-            ret = -1;
-            goto out;
-        }
-        vlan->nb_host_devs++;
-    } else
 #ifdef CONFIG_VDE
     if (!strcmp(device, "vde")) {
         static const char * const vde_params[] = {
-- 
1.6.2.5

[Qemu-devel] [PATCH 13/19] Port -net vde to QemuOpts

[Mark McLoughlin]

The net_vde_init() change is needed because we now pass NULL pointers
instead of empty strings for group/sock if they're not set.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   93 +++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 57 insertions(+), 36 deletions(-)

diff --git a/net.c b/net.c
index c3414eb..3297cb1 100644
--- a/net.c
+++ b/net.c
@@ -1745,8 +1745,8 @@ static int net_vde_init(VLANState *vlan, const char *model,
                         int port, const char *group, int mode)
 {
     VDEState *s;
-    char *init_group = strlen(group) ? (char *)group : NULL;
-    char *init_sock = strlen(sock) ? (char *)sock : NULL;
+    char *init_group = (char *)group;
+    char *init_sock = (char *)sock;
 
     struct vde_open_args args = {
         .port = port,
@@ -2694,6 +2694,33 @@ static int net_init_socket(QemuOpts *opts, Monitor *mon)
     return 0;
 }
 
+#ifdef CONFIG_VDE
+static int net_init_vde(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    const char *name;
+    const char *sock;
+    const char *group;
+    int port, mode;
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name  = qemu_opt_get(opts, "name");
+    sock  = qemu_opt_get(opts, "sock");
+    group = qemu_opt_get(opts, "group");
+
+    port = qemu_opt_get_number(opts, "port", 0);
+    mode = qemu_opt_get_number(opts, "mode", 0700);
+
+    if (net_vde_init(vlan, "vde", name, sock, port, group, mode) == -1)
+        return -1;
+
+    vlan->nb_host_devs++;
+
+    return 0;
+}
+#endif
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2880,6 +2907,32 @@ static struct {
             },
             { /* end of list */ }
         },
+#ifdef CONFIG_VDE
+    }, {
+        .type = "vde",
+        .init = net_init_vde,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "sock",
+                .type = QEMU_OPT_STRING,
+                .help = "socket path",
+            }, {
+                .name = "port",
+                .type = QEMU_OPT_NUMBER,
+                .help = "port number",
+            }, {
+                .name = "group",
+                .type = QEMU_OPT_STRING,
+                .help = "group owner of socket",
+            }, {
+                .name = "mode",
+                .type = QEMU_OPT_NUMBER,
+                .help = "permissions for socket",
+            },
+            { /* end of list */ }
+        },
+#endif
     },
     { /* end of list */ }
 };
@@ -2922,7 +2975,8 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         !strcmp(device, "nic") ||
         !strcmp(device, "user") ||
         !strcmp(device, "tap") ||
-        !strcmp(device, "socket")) {
+        !strcmp(device, "socket") ||
+        !strcmp(device, "vde")) {
         QemuOpts *opts;
 
         opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
@@ -2960,39 +3014,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         ret = 0;
     } else
 #endif
-#ifdef CONFIG_VDE
-    if (!strcmp(device, "vde")) {
-        static const char * const vde_params[] = {
-            "vlan", "name", "sock", "port", "group", "mode", NULL
-        };
-        char vde_sock[1024], vde_group[512];
-	int vde_port, vde_mode;
-
-        if (check_params(buf, sizeof(buf), vde_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        vlan->nb_host_devs++;
-        if (get_param_value(vde_sock, sizeof(vde_sock), "sock", p) <= 0) {
-	    vde_sock[0] = '\0';
-	}
-	if (get_param_value(buf, sizeof(buf), "port", p) > 0) {
-	    vde_port = strtol(buf, NULL, 10);
-	} else {
-	    vde_port = 0;
-	}
-	if (get_param_value(vde_group, sizeof(vde_group), "group", p) <= 0) {
-	    vde_group[0] = '\0';
-	}
-	if (get_param_value(buf, sizeof(buf), "mode", p) > 0) {
-	    vde_mode = strtol(buf, NULL, 8);
-	} else {
-	    vde_mode = 0700;
-	}
-	ret = net_vde_init(vlan, device, name, vde_sock, vde_port, vde_group, vde_mode);
-    } else
-#endif
     if (!strcmp(device, "dump")) {
         int len = 65536;
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 14/19] Port -net dump to QemuOpts

[Mark McLoughlin]

Note, not incrementing nb_host_devs in net_init_dump() is intentional.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   54 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/net.c b/net.c
index 3297cb1..54483b4 100644
--- a/net.c
+++ b/net.c
@@ -2721,6 +2721,29 @@ static int net_init_vde(QemuOpts *opts, Monitor *mon)
 }
 #endif
 
+static int net_init_dump(QemuOpts *opts, Monitor *mon)
+{
+    VLANState *vlan;
+    int len;
+    const char *file;
+    const char *name;
+    char def_file[128];
+
+    vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1);
+
+    name = qemu_opt_get(opts, "name");
+
+    file = qemu_opt_get(opts, "file");
+    if (!file) {
+        snprintf(def_file, sizeof(def_file), "qemu-vlan%d.pcap", vlan->id);
+        file = def_file;
+    }
+
+    len = qemu_opt_get_size(opts, "len", 65536);
+
+    return net_dump_init(mon, vlan, "dump", name, file, len);
+}
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2933,6 +2956,22 @@ static struct {
             { /* end of list */ }
         },
 #endif
+    }, {
+        .type = "dump",
+        .init = net_init_dump,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "len",
+                .type = QEMU_OPT_SIZE,
+                .help = "per-packet size limit (64k default)",
+            }, {
+                .name = "file",
+                .type = QEMU_OPT_STRING,
+                .help = "dump file path (default is qemu-vlan0.pcap)",
+            },
+            { /* end of list */ }
+        },
     },
     { /* end of list */ }
 };
@@ -2976,7 +3015,8 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         !strcmp(device, "user") ||
         !strcmp(device, "tap") ||
         !strcmp(device, "socket") ||
-        !strcmp(device, "vde")) {
+        !strcmp(device, "vde") ||
+        !strcmp(device, "dump")) {
         QemuOpts *opts;
 
         opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
@@ -3014,17 +3054,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         ret = 0;
     } else
 #endif
-    if (!strcmp(device, "dump")) {
-        int len = 65536;
-
-        if (get_param_value(buf, sizeof(buf), "len", p) > 0) {
-            len = strtol(buf, NULL, 0);
-        }
-        if (!get_param_value(buf, sizeof(buf), "file", p)) {
-            snprintf(buf, sizeof(buf), "qemu-vlan%d.pcap", vlan_id);
-        }
-        ret = net_dump_init(mon, vlan, device, name, buf, len);
-    } else {
+    {
         config_error(mon, "Unknown network device: %s\n", device);
         ret = -1;
         goto out;
-- 
1.6.2.5

[Qemu-devel] [PATCH 15/19] Clean up legacy code in net_client_init()

[Mark McLoughlin]

Now that we've ported everything over to QemuOpts, we can kill off
all the cruft in net_client_init().

Note, the 'channel' type requires special handling as it uses a
format that QemuOpts can't parse

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   56 +++++++++++---------------------------------------------
 1 files changed, 11 insertions(+), 45 deletions(-)

diff --git a/net.c b/net.c
index 54483b4..14271ff 100644
--- a/net.c
+++ b/net.c
@@ -3005,38 +3005,7 @@ static int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
 
 int net_client_init(Monitor *mon, const char *device, const char *p)
 {
-    char buf[1024];
-    int vlan_id, ret;
-    VLANState *vlan;
-    char *name = NULL;
-
-    if (!strcmp(device, "none") ||
-        !strcmp(device, "nic") ||
-        !strcmp(device, "user") ||
-        !strcmp(device, "tap") ||
-        !strcmp(device, "socket") ||
-        !strcmp(device, "vde") ||
-        !strcmp(device, "dump")) {
-        QemuOpts *opts;
-
-        opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
-        if (!opts)
-            return -1;
-
-        qemu_opt_set(opts, "type", device);
-
-        return net_client_init_from_opts(mon, opts);
-    }
-
-    vlan_id = 0;
-    if (get_param_value(buf, sizeof(buf), "vlan", p)) {
-        vlan_id = strtol(buf, NULL, 0);
-    }
-    vlan = qemu_find_vlan(vlan_id, 1);
-
-    if (get_param_value(buf, sizeof(buf), "name", p)) {
-        name = qemu_strdup(buf);
-    }
+    QemuOpts *opts;
 
 #ifdef CONFIG_SLIRP
     if (!strcmp(device, "channel")) {
@@ -3051,20 +3020,17 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         } else {
             slirp_guestfwd(TAILQ_FIRST(&slirp_stacks), mon, p, 1);
         }
-        ret = 0;
-    } else
-#endif
-    {
-        config_error(mon, "Unknown network device: %s\n", device);
-        ret = -1;
-        goto out;
-    }
-    if (ret < 0) {
-        config_error(mon, "Could not initialize device '%s'\n", device);
+        return 0;
     }
-out:
-    qemu_free(name);
-    return ret;
+#endif
+
+    opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
+    if (!opts)
+      return -1;
+
+    qemu_opt_set(opts, "type", device);
+
+    return net_client_init_from_opts(mon, opts);
 }
 
 void net_client_uninit(NICInfo *nd)
-- 
1.6.2.5

[Qemu-devel] [PATCH 16/19] Port host_net_add monitor command to QemuOpts

[Mark McLoughlin]

Here is where we rely on qemu_opts_parse() to handle an empty string.
We could alternatively explicitly handle this here by using
qemu_opts_create() when we're not supplied any parameters, but its
cleaner this way.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/net.c b/net.c
index 14271ff..7812018 100644
--- a/net.c
+++ b/net.c
@@ -3064,13 +3064,24 @@ static int net_host_check_device(const char *device)
 void net_host_device_add(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
-    const char *opts = qdict_get_try_str(qdict, "opts");
+    const char *opts_str = qdict_get_try_str(qdict, "opts");
+    QemuOpts *opts;
 
     if (!net_host_check_device(device)) {
         monitor_printf(mon, "invalid host network device %s\n", device);
         return;
     }
-    if (net_client_init(mon, device, opts ? opts : "") < 0) {
+
+    opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", NULL);
+    if (!opts) {
+        monitor_printf(mon, "parsing network options '%s' failed\n",
+                       opts_str ? opts_str : "");
+        return;
+    }
+
+    qemu_opt_set(opts, "type", device);
+
+    if (net_client_init_from_opts(mon, opts) < 0) {
         monitor_printf(mon, "adding host network device %s failed\n", device);
     }
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 17/19] Port usb net to QemuOpts

[Mark McLoughlin]

We need net_client_init_from_opts() exported for this

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |    2 +-
 net.h |    2 ++
 vl.c  |   17 +++++++++++++----
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index 7812018..bd6c305 100644
--- a/net.c
+++ b/net.c
@@ -2976,7 +2976,7 @@ static struct {
     { /* end of list */ }
 };
 
-static int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
+int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
 {
     const char *type;
     int i;
diff --git a/net.h b/net.h
index f46e6c6..5833958 100644
--- a/net.h
+++ b/net.h
@@ -4,6 +4,7 @@
 #include "sys-queue.h"
 #include "qemu-common.h"
 #include "qdict.h"
+#include "qemu-option.h"
 
 /* VLANs support */
 
@@ -135,6 +136,7 @@ extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
 int net_client_init(Monitor *mon, const char *device, const char *p);
+int net_client_init_from_opts(Monitor *mon, QemuOpts *opts);
 void net_client_uninit(NICInfo *nd);
 int net_client_parse(const char *str);
 void net_slirp_smb(const char *exported_dir);
diff --git a/vl.c b/vl.c
index ed07d50..18f115a 100644
--- a/vl.c
+++ b/vl.c
@@ -2506,12 +2506,21 @@ static int usb_device_add(const char *devname, int is_hotplug)
         dev = usb_baum_init();
 #endif
     } else if (strstart(devname, "net:", &p)) {
-        int nic = nb_nics;
+        QemuOpts *opts;
+        int idx;
 
-        if (net_client_init(NULL, "nic", p) < 0)
+        opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
+        if (!opts)
             return -1;
-        nd_table[nic].model = "usb";
-        dev = usb_net_init(&nd_table[nic]);
+
+        qemu_opt_set(opts, "type", "nic");
+
+        idx = net_client_init_from_opts(NULL, opts);
+        if (idx == -1)
+            return -1;
+
+        nd_table[idx].model = "usb";
+        dev = usb_net_init(&nd_table[idx]);
     } else if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
         dev = usb_bt_init(devname[2] ? hci_init(p) :
                         bt_new_hci(qemu_find_bt_vlan(0)));
-- 
1.6.2.5

[Qemu-devel] [PATCH 18/19] Port PCI NIC hotplug to QemuOpts

[Mark McLoughlin]

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/pci-hotplug.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 5348dd1..930a3eb 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -32,14 +32,26 @@
 #include "block_int.h"
 #include "scsi-disk.h"
 #include "virtio-blk.h"
+#include "qemu-config.h"
 
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
-                                       const char *devaddr, const char *opts)
+                                       const char *devaddr,
+                                       const char *opts_str)
 {
+    QemuOpts *opts;
     int ret;
 
-    ret = net_client_init(mon, "nic", opts);
+    opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", NULL);
+    if (!opts) {
+        monitor_printf(mon, "parsing network options '%s' failed\n",
+                       opts_str ? opts_str : "");
+        return NULL;
+    }
+
+    qemu_opt_set(opts, "type", "nic");
+
+    ret = net_client_init_from_opts(mon, opts);
     if (ret < 0)
         return NULL;
     if (nd_table[ret].devaddr) {
-- 
1.6.2.5

[Qemu-devel] [PATCH 19/19] Final net cleanup after conversion to QemuOpts

[Mark McLoughlin]

Now that net_client_init() has no users, kill it off and rename
net_client_init_from_opts().

There is no further need for the old code in net_client_parse() either.
We use qemu_opts_parse() 'firstname' facitity for that. Instead, move
the special handling of the 'vmchannel' type there.

Simplify the vl.c code into merely call net_client_parse() for each
-net command line option and then calling net_init_clients() later
to iterate over the options and create the clients.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/pci-hotplug.c |    2 +-
 net.c            |  106 +++++++++++++++++++++++++++---------------------------
 net.h            |    5 +--
 vl.c             |   30 ++-------------
 4 files changed, 60 insertions(+), 83 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 930a3eb..253dd59 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -51,7 +51,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
     qemu_opt_set(opts, "type", "nic");
 
-    ret = net_client_init_from_opts(mon, opts);
+    ret = net_client_init(mon, opts);
     if (ret < 0)
         return NULL;
     if (nd_table[ret].devaddr) {
diff --git a/net.c b/net.c
index bd6c305..041b814 100644
--- a/net.c
+++ b/net.c
@@ -2976,7 +2976,7 @@ static struct {
     { /* end of list */ }
 };
 
-int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
+int net_client_init(Monitor *mon, QemuOpts *opts)
 {
     const char *type;
     int i;
@@ -3003,36 +3003,6 @@ int net_client_init_from_opts(Monitor *mon, QemuOpts *opts)
     return -1;
 }
 
-int net_client_init(Monitor *mon, const char *device, const char *p)
-{
-    QemuOpts *opts;
-
-#ifdef CONFIG_SLIRP
-    if (!strcmp(device, "channel")) {
-        if (TAILQ_EMPTY(&slirp_stacks)) {
-            struct slirp_config_str *config;
-
-            config = qemu_malloc(sizeof(*config));
-            pstrcpy(config->str, sizeof(config->str), p);
-            config->flags = SLIRP_CFG_LEGACY;
-            config->next = slirp_configs;
-            slirp_configs = config;
-        } else {
-            slirp_guestfwd(TAILQ_FIRST(&slirp_stacks), mon, p, 1);
-        }
-        return 0;
-    }
-#endif
-
-    opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
-    if (!opts)
-      return -1;
-
-    qemu_opt_set(opts, "type", device);
-
-    return net_client_init_from_opts(mon, opts);
-}
-
 void net_client_uninit(NICInfo *nd)
 {
     nd->vlan->nb_guest_devs--;
@@ -3081,7 +3051,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 
     qemu_opt_set(opts, "type", device);
 
-    if (net_client_init_from_opts(mon, opts) < 0) {
+    if (net_client_init(mon, opts) < 0) {
         monitor_printf(mon, "adding host network device %s failed\n", device);
     }
 }
@@ -3103,26 +3073,6 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict)
     qemu_del_vlan_client(vc);
 }
 
-int net_client_parse(const char *str)
-{
-    const char *p;
-    char *q;
-    char device[64];
-
-    p = str;
-    q = device;
-    while (*p != '\0' && *p != ',') {
-        if ((q - device) < sizeof(device) - 1)
-            *q++ = *p;
-        p++;
-    }
-    *q = '\0';
-    if (*p == ',')
-        p++;
-
-    return net_client_init(NULL, device, p);
-}
-
 void net_set_boot_mask(int net_boot_mask)
 {
     int i;
@@ -3203,7 +3153,7 @@ void net_cleanup(void)
     }
 }
 
-void net_client_check(void)
+static void net_check_clients(void)
 {
     VLANState *vlan;
 
@@ -3218,3 +3168,53 @@ void net_client_check(void)
                     vlan->id);
     }
 }
+
+static int net_init_client(QemuOpts *opts, void *dummy)
+{
+    return net_client_init(NULL, opts);
+}
+
+int net_init_clients(void)
+{
+    if (TAILQ_EMPTY(&qemu_net_opts.head)) {
+        /* if no clients, we use a default config */
+        qemu_opts_set(&qemu_net_opts, NULL, "type", "nic");
+#ifdef CONFIG_SLIRP
+        qemu_opts_set(&qemu_net_opts, NULL, "type", "user");
+#endif
+    }
+
+    if (qemu_opts_foreach(&qemu_net_opts, net_init_client, NULL, 1) == -1)
+        return -1;
+
+    net_check_clients();
+
+    return 0;
+}
+
+int net_client_parse(const char *optarg)
+{
+    /* handle legacy -net channel,port:chr */
+    if (!strncmp(optarg, "channel,", strlen("channel,"))) {
+        optarg += strlen("channel,");
+
+        if (TAILQ_EMPTY(&slirp_stacks)) {
+            struct slirp_config_str *config;
+
+            config = qemu_malloc(sizeof(*config));
+            pstrcpy(config->str, sizeof(config->str), optarg);
+            config->flags = SLIRP_CFG_LEGACY;
+            config->next = slirp_configs;
+            slirp_configs = config;
+        } else {
+            slirp_guestfwd(TAILQ_FIRST(&slirp_stacks), NULL, optarg, 1);
+        }
+
+        return 0;
+    }
+
+    if (!qemu_opts_parse(&qemu_net_opts, optarg, "type"))
+        return -1;
+
+    return 0;
+}
diff --git a/net.h b/net.h
index 5833958..9812aad 100644
--- a/net.h
+++ b/net.h
@@ -135,16 +135,15 @@ void net_checksum_calculate(uint8_t *data, int length);
 extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
-int net_client_init(Monitor *mon, const char *device, const char *p);
-int net_client_init_from_opts(Monitor *mon, QemuOpts *opts);
+int net_client_init(Monitor *mon, QemuOpts *opts);
 void net_client_uninit(NICInfo *nd);
 int net_client_parse(const char *str);
+int net_init_clients(void);
 void net_slirp_smb(const char *exported_dir);
 void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict);
 void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict);
 void net_slirp_redir(const char *redir_str);
 void net_cleanup(void);
-void net_client_check(void);
 void net_set_boot_mask(int boot_mask);
 void net_host_device_add(Monitor *mon, const QDict *qdict);
 void net_host_device_remove(Monitor *mon, const QDict *qdict);
diff --git a/vl.c b/vl.c
index 18f115a..9285d6b 100644
--- a/vl.c
+++ b/vl.c
@@ -2515,7 +2515,7 @@ static int usb_device_add(const char *devname, int is_hotplug)
 
         qemu_opt_set(opts, "type", "nic");
 
-        idx = net_client_init_from_opts(NULL, opts);
+        idx = net_client_init(NULL, opts);
         if (idx == -1)
             return -1;
 
@@ -4435,8 +4435,6 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
     return 0;
 }
 
-#define MAX_NET_CLIENTS 32
-
 #ifndef _WIN32
 
 static void termsig_handler(int signal)
@@ -4640,8 +4638,6 @@ int main(int argc, char **argv, char **envp)
     DisplayState *ds;
     DisplayChangeListener *dcl;
     int cyls, heads, secs, translation;
-    const char *net_clients[MAX_NET_CLIENTS];
-    int nb_net_clients;
     QemuOpts *hda_opts = NULL, *opts;
     int optind;
     const char *r, *optarg;
@@ -4742,7 +4738,6 @@ int main(int argc, char **argv, char **envp)
         node_cpumask[i] = 0;
     }
 
-    nb_net_clients = 0;
     nb_numa_nodes = 0;
     nb_nics = 0;
 
@@ -4988,12 +4983,8 @@ int main(int argc, char **argv, char **envp)
                 break;
 #endif
             case QEMU_OPTION_net:
-                if (nb_net_clients >= MAX_NET_CLIENTS) {
-                    fprintf(stderr, "qemu: too many network clients\n");
+                if (net_client_parse(optarg) == -1)
                     exit(1);
-                }
-                net_clients[nb_net_clients] = optarg;
-                nb_net_clients++;
                 break;
 #ifdef CONFIG_SLIRP
             case QEMU_OPTION_tftp:
@@ -5578,25 +5569,12 @@ int main(int argc, char **argv, char **envp)
     socket_init();
 #endif
 
-    /* init network clients */
-    if (nb_net_clients == 0) {
-        /* if no clients, we use a default config */
-        net_clients[nb_net_clients++] = "nic";
-#ifdef CONFIG_SLIRP
-        net_clients[nb_net_clients++] = "user";
-#endif
-    }
-
-    for(i = 0;i < nb_net_clients; i++) {
-        if (net_client_parse(net_clients[i]) < 0)
-            exit(1);
-    }
+    if (net_init_clients() < 0)
+        exit(1);
 
     net_boot = (boot_devices_bitmap >> ('n' - 'a')) & 0xF;
     net_set_boot_mask(net_boot);
 
-    net_client_check();
-
     /* init the bluetooth world */
     if (foreach_device_config(DEV_BT, bt_parse))
         exit(1);
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 02/19] Remove bogus error message from qemu_opts_set()

[Michael S. Tsirkin]

On Thu, Sep 10, 2009 at 04:18:44PM +0100, Mark McLoughlin wrote:
> The only way qemu_opts_create() can fail is if a QemuOpts with that id
> already exists and fail_if_exists=1. In that case, we already print
> an error which makes more sense than the one in qemu_opts_set().
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  qemu-option.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-option.c b/qemu-option.c
> index f1a666f..4d544c7 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -669,11 +669,8 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
>      QemuOpts *opts;
>  
>      opts = qemu_opts_create(list, id, 1);
> -    if (opts == NULL) {
> -        fprintf(stderr, "id \"%s\" not found for \"%s\"\n",
> -                id, list->name);
> +    if (opts == NULL)
>          return -1;
> -    }

I thought we are supposed to keep {} around?
Yes I hate this too.


>      return qemu_opt_set(opts, name, value);
>  }
>  
> -- 
> 1.6.2.5
> 
> 

Re: [Qemu-devel] [PATCH 01/19] Suppress more more kraxelisms

[Michael S. Tsirkin]

On Thu, Sep 10, 2009 at 04:18:43PM +0100, Mark McLoughlin wrote:
> Let's kick off this series with some of the more critical fixes.
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>


Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/qdev.c     |    2 +-
>  qemu-config.c |    2 +-
>  qemu-option.c |    6 +++---
>  vl.c          |    2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 6451b8a..c0ef55b 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -137,7 +137,7 @@ static int set_property(const char *name, const char *value, void *opaque)
>      if (strcmp(name, "bus") == 0)
>          return 0;
>  
> -    if (-1 == qdev_prop_parse(dev, name, value)) {
> +    if (qdev_prop_parse(dev, name, value) == -1) {
>          qemu_error("can't set property \"%s\" to \"%s\" for \"%s\"\n",
>                     name, value, dev->info->name);
>          return -1;
> diff --git a/qemu-config.c b/qemu-config.c
> index f6f4cb4..61e6ed1 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -186,7 +186,7 @@ int qemu_set_option(const char *str)
>          return -1;
>      }
>  
> -    if (-1 == qemu_opt_set(opts, arg, str+offset+1)) {
> +    if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
>          fprintf(stderr, "failed to set \"%s\" for %s \"%s\"\n",
>                  arg, lists[i]->name, id);
>          return -1;
> diff --git a/qemu-option.c b/qemu-option.c
> index 0c2101e..f1a666f 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -267,7 +267,7 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
>      // Process parameter
>      switch (list->type) {
>      case OPT_FLAG:
> -        if (-1 == parse_option_bool(name, value, &flag))
> +        if (parse_option_bool(name, value, &flag) == -1)
>              return -1;
>          list->value.n = flag;
>          break;
> @@ -282,7 +282,7 @@ int set_option_parameter(QEMUOptionParameter *list, const char *name,
>          break;
>  
>      case OPT_SIZE:
> -        if (-1 == parse_option_size(name, value, &list->value.n))
> +        if (parse_option_size(name, value, &list->value.n) == -1)
>              return -1;
>          break;
>  
> @@ -745,7 +745,7 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
>          }
>          if (strcmp(option, "id") != 0) {
>              /* store and parse */
> -            if (-1 == qemu_opt_set(opts, option, value)) {
> +            if (qemu_opt_set(opts, option, value) == -1) {
>                  return -1;
>              }
>          }
> diff --git a/vl.c b/vl.c
> index 1dc3ffa..6da83a6 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5153,7 +5153,7 @@ int main(int argc, char **argv, char **envp)
>                      fprintf(stderr, "parse error: %s\n", optarg);
>                      exit(1);
>                  }
> -                if (NULL == qemu_chr_open_opts(opts, NULL)) {
> +                if (!qemu_chr_open_opts(opts, NULL)) {
>                      exit(1);
>                  }
>                  break;
> -- 
> 1.6.2.5
> 
> 

Re: [Qemu-devel] [PATCH 03/19] Remove double error message in qemu_option_set()

[Michael S. Tsirkin]

On Thu, Sep 10, 2009 at 04:18:45PM +0100, Mark McLoughlin wrote:
> qemu_opt_set() prints an error message in all failure cases, so
> qemu_set_option() doesn't need to print another error.
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>

Same comment about {} here

> ---
>  qemu-config.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-config.c b/qemu-config.c
> index 61e6ed1..733cc24 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -186,11 +186,9 @@ int qemu_set_option(const char *str)
>          return -1;
>      }
>  
> -    if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
> -        fprintf(stderr, "failed to set \"%s\" for %s \"%s\"\n",
> -                arg, lists[i]->name, id);
> +    if (qemu_opt_set(opts, arg, str+offset+1) == -1)
>          return -1;
> -    }
> +
>      return 0;
>  }
>  
> -- 
> 1.6.2.5
> 
> 

[Qemu-devel] Re: [PATCH 04/19] Remove double error message for -device option parsing

[Mark McLoughlin]

On Thu, 2009-09-10 at 18:25 +0200, Juan Quintela wrote:
> Mark McLoughlin <markmc@redhat.com> wrote:
> > qemu_opts_parse() gives a suitable error message in all failure cases
> > so we can remove the error message from the caller.
> >
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > ---
> >  vl.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 6da83a6..ed07d50 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -5251,11 +5251,8 @@ int main(int argc, char **argv, char **envp)
> >                  add_device_config(DEV_USB, optarg);
> >                  break;
> >              case QEMU_OPTION_device:
> > -                opts = qemu_opts_parse(&qemu_device_opts, optarg, "driver");
> > -                if (!opts) {
> > -                    fprintf(stderr, "parse error: %s\n", optarg);
> > +                if (!qemu_opts_parse(&qemu_device_opts, optarg, "driver"))
> >                      exit(1);
> > -                }
> >                  break;
> >              case QEMU_OPTION_smp:
> >                  smp_parse(optarg);
> 
> This was the only use of opts variable, you have to remove it.

I did, but then re-added it when I re-based to Anthony's queue. The
chardev stuff uses it.

> And you shouldn't remove the braces (read the qemu coding style).

Yeah, I try not to, it was a reflex, I guess.

(It's also not helped that a lot of the existing code doesn't follow
this aspect of the coding style)

Cheers,
Mark.

Re: braces [was Re: [Qemu-devel] [PATCH 03/19] Remove double error message in qemu_option_set()]

[Mark McLoughlin]

On Thu, 2009-09-10 at 21:03 +0300, Michael S. Tsirkin wrote:
> On Thu, Sep 10, 2009 at 04:18:45PM +0100, Mark McLoughlin wrote:
> > qemu_opt_set() prints an error message in all failure cases, so
> > qemu_set_option() doesn't need to print another error.
> > 
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> 
> Same comment about {} here

That's 3 of these you and Juan have found. And, genuinely, I try not to
do this. Just shows how much of reflex it is.

IMHO,

  if (blaa(foo, bar, doodah) == NULL)
      return -1;

is far nicer than:

  if (blaa(foo, bar, doodah) == NULL) {
      return -1;
  }

but OTOH, this:

  if (blaa(foo, bar, doodah) == NULL) {
      return doodah(foo, bar, blaa);
  }

*is* arguably better than:

  if (blaa(foo, bar, doodah) == NULL)
      return doodah(foo, bar, blaa);

There's enough of both ways in the code that I think either should be
acceptable.

Cheers,
Mark.

Re: braces [was Re: [Qemu-devel] [PATCH 03/19] Remove double error message in qemu_option_set()]

[Michael S. Tsirkin]

On Thu, Sep 10, 2009 at 07:36:09PM +0100, Mark McLoughlin wrote:
> On Thu, 2009-09-10 at 21:03 +0300, Michael S. Tsirkin wrote:
> > On Thu, Sep 10, 2009 at 04:18:45PM +0100, Mark McLoughlin wrote:
> > > qemu_opt_set() prints an error message in all failure cases, so
> > > qemu_set_option() doesn't need to print another error.
> > > 
> > > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > 
> > Same comment about {} here
> 
> That's 3 of these you and Juan have found. And, genuinely, I try not to
> do this. Just shows how much of reflex it is.
> 
> IMHO,
> 
>   if (blaa(foo, bar, doodah) == NULL)
>       return -1;
> 
> is far nicer than:
> 
>   if (blaa(foo, bar, doodah) == NULL) {
>       return -1;
>   }
> 
> but OTOH, this:
> 
>   if (blaa(foo, bar, doodah) == NULL) {
>       return doodah(foo, bar, blaa);
>   }
> 
> *is* arguably better than:
> 
>   if (blaa(foo, bar, doodah) == NULL)
>       return doodah(foo, bar, blaa);

And 
  if (!blaa(foo, bar, doodah))
is even better :)

> There's enough of both ways in the code that I think either should be
> acceptable.
> 
> Cheers,
> Mark.

Re: braces [was Re: [Qemu-devel] [PATCH 03/19] Remove double error message in qemu_option_set()]

[Amit Shah]

On (Thu) Sep 10 2009 [19:36:09], Mark McLoughlin wrote:
> On Thu, 2009-09-10 at 21:03 +0300, Michael S. Tsirkin wrote:
> > On Thu, Sep 10, 2009 at 04:18:45PM +0100, Mark McLoughlin wrote:
> > > qemu_opt_set() prints an error message in all failure cases, so
> > > qemu_set_option() doesn't need to print another error.
> > > 
> > > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > 
> > Same comment about {} here
> 
> That's 3 of these you and Juan have found. And, genuinely, I try not to
> do this. Just shows how much of reflex it is.
> 
> IMHO,
> 
>   if (blaa(foo, bar, doodah) == NULL)
>       return -1;
> 
> is far nicer than:
> 
>   if (blaa(foo, bar, doodah) == NULL) {
>       return -1;
>   }

BTW in patch 1 of the series you convert

    if (foo == NULL)

to

    if (!foo)

so maybe you'd want to be consistent in the rest of the series as well?

		Amit

Re: [Qemu-devel] [PATCH 06/19] Add qemu_opts_validate() for post parsing validation

[Gerd Hoffmann]

On 09/10/09 17:18, Mark McLoughlin wrote:
> Several qemu command line options have a parameter whose value affects
> what other parameters are accepted for the option.
>
> In these cases, we can have an empty description table in the
> QemuOptsList and once the option has been parsed we can use a suitable
> description table to validate the other parameters based on the value of
> that parameter.

Hmm.  The "empty description" mode is intended to be used in case the 
verification isn't done by QemuOpts at all (i.e. for -devices, where the 
qdev property code will do the checks).  I really like to have the 
option description hooked into QemuOptsList.

I can think of two possible ways to do that:

(1) We can stick all possible values info QemuOptsList->desc.  Then
     have separate data structures to describe which fields are allowed
     in which cases (and, while being at it, which fields are mandatory).

(2) We have multiple QemuOptDesc lists for the different cases.

I tend to prefer the first.  We can do the option parsing 
unconditionally in qemu_opt_set() then and don't have two different 
possible code paths for that.

Comments?  Other ideas?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 07/19] Never overwrite a QemuOpt

[Gerd Hoffmann]

On 09/10/09 17:18, Mark McLoughlin wrote:
> Rather than overwriting a QemuOpt, just add a new one to the tail and
> always do a reverse search for parameters to preserve the same
> behaviour. We use this order so that foreach() iterates over the opts
> in their original order.

I think we should simply add a flag to QemuOptsDesc saying 'this can 
have multiple instances' instead of doing these reverse search tricks to 
maintain backward compatibility.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 09/19] Port -net none and -net nic to QemuOpts

[Gerd Hoffmann]

   Hi,

> +#define NET_COMMON_PARAMS_DESC                     \

> +        .name = "name",                            \
> +        .type = QEMU_OPT_STRING,                   \
> +        .help = "identifier for monitor commands", \

QemuOpts has id= which IMHO should be used here instead for consistency 
reasons.  I suspect we have to support name= as well for backward 
compatibility ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 06/19] Add qemu_opts_validate() for post parsing validation

[Mark McLoughlin]

On Fri, 2009-09-11 at 09:47 +0200, Gerd Hoffmann wrote:
> On 09/10/09 17:18, Mark McLoughlin wrote:
> > Several qemu command line options have a parameter whose value affects
> > what other parameters are accepted for the option.
> >
> > In these cases, we can have an empty description table in the
> > QemuOptsList and once the option has been parsed we can use a suitable
> > description table to validate the other parameters based on the value of
> > that parameter.
> 
> Hmm.  The "empty description" mode is intended to be used in case the 
> verification isn't done by QemuOpts at all (i.e. for -devices, where the 
> qdev property code will do the checks).  I really like to have the 
> option description hooked into QemuOptsList.
> 
> I can think of two possible ways to do that:
> 
> (1) We can stick all possible values info QemuOptsList->desc.  Then
>      have separate data structures to describe which fields are allowed
>      in which cases (and, while being at it, which fields are mandatory).

You'd need to make it part of the QemuOptDesc to make it easy to figure
out from reading the code which parameters apply to which types.

e.g. a QemuOptDesc::firstname_values field

> (2) We have multiple QemuOptDesc lists for the different cases.

As with what I did, you'll have some parameters which are common to
different types.

> I tend to prefer the first.  We can do the option parsing 
> unconditionally in qemu_opt_set() then and don't have two different 
> possible code paths for that.

Sounds reasonable, except it is nice that right now we have the
parameter descriptions associated with, and close to, the init function.

Cheers,
Mark.

Re: [Qemu-devel] [PATCH 09/19] Port -net none and -net nic to QemuOpts

[Mark McLoughlin]

On Fri, 2009-09-11 at 09:57 +0200, Gerd Hoffmann wrote:
> Hi,
> 
> > +#define NET_COMMON_PARAMS_DESC                     \
> 
> > +        .name = "name",                            \
> > +        .type = QEMU_OPT_STRING,                   \
> > +        .help = "identifier for monitor commands", \
> 
> QemuOpts has id= which IMHO should be used here instead for consistency 
> reasons.  I suspect we have to support name= as well for backward 
> compatibility ...

Yes, libvirt uses name= already

Cheers,
Mark.

Re: [Qemu-devel] [PATCH 06/19] Add qemu_opts_validate() for post parsing validation

[Gerd Hoffmann]

On 09/11/09 14:38, Mark McLoughlin wrote:
> On Fri, 2009-09-11 at 09:47 +0200, Gerd Hoffmann wrote:
>> (1) We can stick all possible values info QemuOptsList->desc.  Then
>>       have separate data structures to describe which fields are allowed
>>       in which cases (and, while being at it, which fields are mandatory).
>
> You'd need to make it part of the QemuOptDesc to make it easy to figure
> out from reading the code which parameters apply to which types.
>
> e.g. a QemuOptDesc::firstname_values field

Yes, that would work as well.

>> (2) We have multiple QemuOptDesc lists for the different cases.
>
> As with what I did, you'll have some parameters which are common to
> different types.

One of the reasons I'd prefer (1).

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 00/19] Port -net to QemuOpts

[Mark McLoughlin]

Hi Anthony,

On Thu, 2009-09-10 at 16:18 +0100, Mark McLoughlin wrote:
> Hi,
>         Here's a fairly straightforward port of -net to QemuOpts.

	What happened to this patch series? AFAIR, it was in your staging tree
for a while but never got pushed upstream?

Cheers,
Mark.