All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts
@ 2009-09-23 10:23 Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 01/24] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
                   ` (24 more replies)
  0 siblings, 25 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:23 UTC (permalink / raw)
  To: qemu-devel


Hi,
        I'm still not sure what happened the first series, but it
needs rebasing now anyway, so ...

        Changes since v1:

  - Rebase to latest git, conflicts mainly with the qemu-queue.h
    changes

  - Fix some coding style problems pointed out by people

  - Pull in Glauber's NICInfo fix, since it would also conflict;
    also add some cleanups related to that fix

        Original posting here:

  http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00617.html

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 01/24] Use qemu_strdup() for NICInfo string fields
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 02/24] Don't assign a static string to NICInfo::model Mark McLoughlin
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/net.c b/net.c
index 3fdf1e6..c276d0a 100644
--- a/net.c
+++ b/net.c
@@ -2364,7 +2364,7 @@ void qemu_check_nic_model_list(NICInfo *nd, const char * const *models,
     int i, exit_status = 0;
 
     if (!nd->model)
-        nd->model = strdup(default_model);
+        nd->model = qemu_strdup(default_model);
 
     if (strcmp(nd->model, "?") != 0) {
         for (i = 0 ; models[i]; i++)
@@ -2450,13 +2450,13 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             }
         }
         if (get_param_value(buf, sizeof(buf), "model", p)) {
-            nd->model = strdup(buf);
+            nd->model = qemu_strdup(buf);
         }
         if (get_param_value(buf, sizeof(buf), "addr", p)) {
-            nd->devaddr = strdup(buf);
+            nd->devaddr = qemu_strdup(buf);
         }
         if (get_param_value(buf, sizeof(buf), "id", p)) {
-            nd->id = strdup(buf);
+            nd->id = qemu_strdup(buf);
         }
         nd->nvectors = NIC_NVECTORS_UNSPECIFIED;
         if (get_param_value(buf, sizeof(buf), "vectors", p)) {
@@ -2804,7 +2804,7 @@ void net_client_uninit(NICInfo *nd)
     nd->vlan->nb_guest_devs--;
     nb_nics--;
     nd->used = 0;
-    free((void *)nd->model);
+    qemu_free((void *)nd->model);
 }
 
 static int net_host_check_device(const char *device)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 02/24] Don't assign a static string to NICInfo::model
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 01/24] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 03/24] Make NICInfo string fields non-const Mark McLoughlin
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/vl.c b/vl.c
index eb01da7..f978944 100644
--- a/vl.c
+++ b/vl.c
@@ -2499,7 +2499,7 @@ static int usb_device_add(const char *devname, int is_hotplug)
 
         if (net_client_init(NULL, "nic", p) < 0)
             return -1;
-        nd_table[nic].model = "usb";
+        nd_table[nic].model = qemu_strdup("usb");
         dev = usb_net_init(&nd_table[nic]);
     } else if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
         dev = usb_bt_init(devname[2] ? hci_init(p) :
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 03/24] Make NICInfo string fields non-const
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 01/24] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 02/24] Don't assign a static string to NICInfo::model Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 04/24] Correctly free nd structure Mark McLoughlin
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

We now only assign strdup()ed strings to these fields, never static
strings.

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

diff --git a/net.c b/net.c
index c276d0a..d04b6bd 100644
--- a/net.c
+++ b/net.c
@@ -2804,7 +2804,7 @@ void net_client_uninit(NICInfo *nd)
     nd->vlan->nb_guest_devs--;
     nb_nics--;
     nd->used = 0;
-    qemu_free((void *)nd->model);
+    qemu_free(nd->model);
 }
 
 static int net_host_check_device(const char *device)
diff --git a/net.h b/net.h
index 1479826..d48c9b8 100644
--- a/net.h
+++ b/net.h
@@ -94,10 +94,10 @@ enum {
 
 struct NICInfo {
     uint8_t macaddr[6];
-    const char *model;
-    const char *name;
-    const char *devaddr;
-    const char *id;
+    char *model;
+    char *name;
+    char *devaddr;
+    char *id;
     VLANState *vlan;
     VLANClientState *vc;
     void *private;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 04/24] Correctly free nd structure
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (2 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 03/24] Make NICInfo string fields non-const Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 05/24] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Glauber Costa

From: Glauber Costa <glommer@redhat.com>

When we "free" a NICInfo structure, we can leak pointers, since we don't do
much more than setting used = 0.

We free() the model parameter, but we don't set it to NULL. This means that
a new user of this structure will see garbage in there. It was not noticed
before because reusing a NICInfo is not that common, but it can be, for
users of device pci hotplug.

A user hit it, described at https://bugzilla.redhat.com/524022

This patch memset's the whole structure, guaranteeing that anyone reusing it
will see a fresh NICinfo. Also, we free some other strings that are currently
leaking.

This codebase is quite old, so this patch should feed all stable trees.

Signed-off-by: Glauber Costa <glommer@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index d04b6bd..422ef4c 100644
--- a/net.c
+++ b/net.c
@@ -2434,6 +2434,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             goto out;
         }
         nd = &nd_table[idx];
+        memset(nd, 0, sizeof(*nd));
         macaddr = nd->macaddr;
         macaddr[0] = 0x52;
         macaddr[1] = 0x54;
@@ -2803,8 +2804,13 @@ void net_client_uninit(NICInfo *nd)
 {
     nd->vlan->nb_guest_devs--;
     nb_nics--;
-    nd->used = 0;
+
     qemu_free(nd->model);
+    qemu_free(nd->name);
+    qemu_free(nd->devaddr);
+    qemu_free(nd->id);
+
+    nd->used = 0;
 }
 
 static int net_host_check_device(const char *device)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 05/24] Use qemu_strdup() for VLANClientState string fields
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (3 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 04/24] Correctly free nd structure Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 06/24] Fix coding style issue Mark McLoughlin
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/net.c b/net.c
index 422ef4c..9a34030 100644
--- a/net.c
+++ b/net.c
@@ -296,7 +296,7 @@ static char *assign_name(VLANClientState *vc1, const char *model)
 
     snprintf(buf, sizeof(buf), "%s.%d", model, id);
 
-    return strdup(buf);
+    return qemu_strdup(buf);
 }
 
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
@@ -310,9 +310,9 @@ VLANClientState *qemu_new_vlan_client(VLANState *vlan,
 {
     VLANClientState *vc, **pvc;
     vc = qemu_mallocz(sizeof(VLANClientState));
-    vc->model = strdup(model);
+    vc->model = qemu_strdup(model);
     if (name)
-        vc->name = strdup(name);
+        vc->name = qemu_strdup(name);
     else
         vc->name = assign_name(vc, model);
     vc->can_receive = can_receive;
@@ -340,8 +340,8 @@ void qemu_del_vlan_client(VLANClientState *vc)
             if (vc->cleanup) {
                 vc->cleanup(vc);
             }
-            free(vc->name);
-            free(vc->model);
+            qemu_free(vc->name);
+            qemu_free(vc->model);
             qemu_free(vc);
             break;
         } else
@@ -2127,8 +2127,8 @@ static int net_socket_listen_init(VLANState *vlan,
         return -1;
     }
     s->vlan = vlan;
-    s->model = strdup(model);
-    s->name = name ? strdup(name) : NULL;
+    s->model = qemu_strdup(model);
+    s->name = name ? qemu_strdup(name) : NULL;
     s->fd = fd;
     qemu_set_fd_handler(fd, net_socket_accept, NULL, s);
     return 0;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 06/24] Fix coding style issue
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (4 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 05/24] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 07/24] Remove bogus error message from qemu_opts_set() Mark McLoughlin
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Avi Kivity, Gerd Hoffmann

Replace:

  if (-1 == foo())

with:

  if (foo() == -1)

While this coding style is not in direct contravention of our currently
ratified CODING_STYLE treaty, it could be argued that the Article 3 of
the European Convention on Human Rights (prohibiting torture and "inhuman
or degrading treatment") reads on the matter.

[This commit message was brought to you without humour, as is evidenced
by the absence of any emoticons]

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: Gerd Hoffmann <kraxel@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 43b1beb..a589d72 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 39bf6a9..555c7ba 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 88298e4..9fe1f95 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 f978944..4ebd242 100644
--- a/vl.c
+++ b/vl.c
@@ -5142,7 +5142,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) == NULL) {
                     exit(1);
                 }
                 break;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 07/24] Remove bogus error message from qemu_opts_set()
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (5 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 06/24] Fix coding style issue Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 08/24] Remove double error message in qemu_option_set() Mark McLoughlin
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 08/24] Remove double error message in qemu_option_set()
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (6 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 07/24] Remove bogus error message from qemu_opts_set() Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 09/24] Remove double error message for -device option parsing Mark McLoughlin
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 555c7ba..31e7d61 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -187,8 +187,6 @@ int qemu_set_option(const char *str)
     }
 
     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;
     }
     return 0;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 09/24] Remove double error message for -device option parsing
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (7 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 08/24] Remove double error message in qemu_option_set() Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 10/24] Make qemu_opts_parse() handle empty strings Mark McLoughlin
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 4ebd242..0f2e2f6 100644
--- a/vl.c
+++ b/vl.c
@@ -5240,9 +5240,7 @@ 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;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 10/24] Make qemu_opts_parse() handle empty strings
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (8 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 09/24] Remove double error message for -device option parsing Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 11/24] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 246f211..390444f 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -712,8 +712,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)) {
@@ -750,7 +749,6 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
         if (*p != ',') {
             break;
         }
-        p++;
     }
     return 0;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 11/24] Add qemu_opts_validate() for post parsing validation
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (9 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 10/24] Make qemu_opts_parse() handle empty strings Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 12/24] Never overwrite a QemuOpt Mark McLoughlin
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |   33 +++++++++++++++++++++++++++++++++
 qemu-option.h |    1 +
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 390444f..6cb5f50 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -778,6 +778,39 @@ 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);
+
+    QTAILQ_FOREACH(opt, &opts->head, next) {
+        int i;
+
+        for (i = 0; desc[i].name != NULL; i++) {
+            if (strcmp(desc[i].name, opt->name) == 0) {
+                break;
+            }
+        }
+        if (desc[i].name == NULL) {
+            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 525b9b6..666b666 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 12/24] Never overwrite a QemuOpt
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (10 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 11/24] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 13/24] Add qemu_net_opts Mark McLoughlin
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 6cb5f50..ffc369a 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -483,7 +483,7 @@ struct QemuOpt {
 struct QemuOpts {
     const char *id;
     QemuOptsList *list;
-    QTAILQ_HEAD(, QemuOpt) head;
+    QTAILQ_HEAD(QemuOptHead, QemuOpt) head;
     QTAILQ_ENTRY(QemuOpts) next;
 };
 
@@ -491,7 +491,7 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
 
-    QTAILQ_FOREACH(opt, &opts->head, next) {
+    QTAILQ_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;
-        QTAILQ_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;
+    QTAILQ_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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 13/24] Add qemu_net_opts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (11 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 12/24] Never overwrite a QemuOpt Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 14/24] Port -net none and -net nic to QemuOpts Mark McLoughlin
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 31e7d61..cf4aee8 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -151,10 +151,23 @@ QemuOptsList qemu_device_opts = {
     },
 };
 
+QemuOptsList qemu_net_opts = {
+    .name = "net",
+    .head = QTAILQ_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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 14/24] Port -net none and -net nic to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (12 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 13/24] Add qemu_net_opts Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 15/24] Port -net user " Mark McLoughlin
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  239 ++++++++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 161 insertions(+), 78 deletions(-)

diff --git a/net.c b/net.c
index 9a34030..e03dd8c 100644
--- a/net.c
+++ b/net.c
@@ -110,6 +110,7 @@
 #include "audio/audio.h"
 #include "qemu_socket.h"
 #include "qemu-log.h"
+#include "qemu-config.h"
 
 #include "slirp/libslirp.h"
 #include "qemu-queue.h"
@@ -2399,6 +2400,151 @@ 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);
+
+    if (qemu_opts_id(opts)) {
+        nd->id = qemu_strdup(qemu_opts_id(opts));
+    }
+    if (qemu_opt_get(opts, "name")) {
+        nd->name = qemu_strdup(qemu_opt_get(opts, "name"));
+    }
+    if (qemu_opt_get(opts, "model")) {
+        nd->model = qemu_strdup(qemu_opt_get(opts, "model"));
+    }
+    if (qemu_opt_get(opts, "addr")) {
+        nd->devaddr = qemu_strdup(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];
@@ -2406,6 +2552,20 @@ 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);
@@ -2415,84 +2575,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];
-        memset(nd, 0, sizeof(*nd));
-        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 = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "addr", p)) {
-            nd->devaddr = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "id", p)) {
-            nd->id = qemu_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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 15/24] Port -net user to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (13 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 14/24] Port -net none and -net nic to QemuOpts Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 16/24] Port -net tap " Mark McLoughlin
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  255 +++++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 154 insertions(+), 101 deletions(-)

diff --git a/net.c b/net.c
index e03dd8c..5cc72b2 100644
--- a/net.c
+++ b/net.c
@@ -2457,6 +2457,95 @@ 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",                            \
@@ -2512,6 +2601,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 */ }
 };
@@ -2553,7 +2704,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);
@@ -2577,106 +2729,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 (QTAILQ_EMPTY(&slirp_stacks)) {
             struct slirp_config_str *config;
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 16/24] Port -net tap to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (14 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 15/24] Port -net user " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-30 19:41   ` Anthony Liguori
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 17/24] Port -net socket " Mark McLoughlin
                   ` (8 subsequent siblings)
  24 siblings, 1 reply; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  225 ++++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 138 insertions(+), 87 deletions(-)

diff --git a/net.c b/net.c
index 5cc72b2..468d25c 100644
--- a/net.c
+++ b/net.c
@@ -1376,25 +1376,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");
@@ -2546,6 +2543,92 @@ 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",                            \
@@ -2663,6 +2746,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 */ }
 };
@@ -2705,7 +2833,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);
@@ -2744,84 +2873,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 17/24] Port -net socket to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (15 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 16/24] Port -net tap " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 18/24] Port -net vde " Mark McLoughlin
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 468d25c..f2bc9f8 100644
--- a/net.c
+++ b/net.c
@@ -2629,6 +2629,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",                            \
@@ -2791,6 +2874,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 */ }
 };
@@ -2834,7 +2941,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);
@@ -2873,64 +2981,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 18/24] Port -net vde to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (16 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 17/24] Port -net socket " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 19/24] Port -net dump " Mark McLoughlin
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |   94 ++++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 36 deletions(-)

diff --git a/net.c b/net.c
index f2bc9f8..e439b59 100644
--- a/net.c
+++ b/net.c
@@ -1744,8 +1744,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,
@@ -2712,6 +2712,34 @@ 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",                            \
@@ -2898,6 +2926,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 */ }
 };
@@ -2942,7 +2996,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);
@@ -2981,39 +3036,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 19/24] Port -net dump to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (17 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 18/24] Port -net vde " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 20/24] Clean up legacy code in net_client_init() Mark McLoughlin
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 e439b59..1ba9a3d 100644
--- a/net.c
+++ b/net.c
@@ -2740,6 +2740,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",                            \
@@ -2952,6 +2975,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 */ }
 };
@@ -2997,7 +3036,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);
@@ -3036,17 +3076,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 20/24] Clean up legacy code in net_client_init()
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (18 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 19/24] Port -net dump " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 21/24] Port host_net_add monitor command to QemuOpts Mark McLoughlin
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 1ba9a3d..8da5eba 100644
--- a/net.c
+++ b/net.c
@@ -3026,39 +3026,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")) {
@@ -3073,20 +3041,18 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         } else {
             slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), mon, p, 1);
         }
-        ret = 0;
-    } else
-#endif
-    {
-        config_error(mon, "Unknown network device: %s\n", device);
-        ret = -1;
-        goto out;
+        return 0;
     }
-    if (ret < 0) {
-        config_error(mon, "Could not initialize device '%s'\n", device);
+#endif
+
+    opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
+    if (!opts) {
+      return -1;
     }
-out:
-    qemu_free(name);
-    return ret;
+
+    qemu_opt_set(opts, "type", device);
+
+    return net_client_init_from_opts(mon, opts);
 }
 
 void net_client_uninit(NICInfo *nd)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 21/24] Port host_net_add monitor command to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (19 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 20/24] Clean up legacy code in net_client_init() Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 22/24] Port usb net " Mark McLoughlin
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 8da5eba..f66df04 100644
--- a/net.c
+++ b/net.c
@@ -3091,13 +3091,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 22/24] Port usb net to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (20 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 21/24] Port host_net_add monitor command to QemuOpts Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 23/24] Port PCI NIC hotplug " Mark McLoughlin
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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  |   19 +++++++++++++++----
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index f66df04..a192761 100644
--- a/net.c
+++ b/net.c
@@ -2995,7 +2995,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 d48c9b8..1b849b4 100644
--- a/net.h
+++ b/net.h
@@ -4,6 +4,7 @@
 #include "qemu-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 0f2e2f6..f9e60ed 100644
--- a/vl.c
+++ b/vl.c
@@ -2495,12 +2495,23 @@ 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 = qemu_strdup("usb");
-        dev = usb_net_init(&nd_table[nic]);
+        }
+
+        qemu_opt_set(opts, "type", "nic");
+        qemu_opt_set(opts, "model", "usb");
+
+        idx = net_client_init_from_opts(NULL, opts);
+        if (idx == -1) {
+            return -1;
+        }
+
+        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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 23/24] Port PCI NIC hotplug to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (21 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 22/24] Port usb net " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 24/24] Final net cleanup after conversion " Mark McLoughlin
  2009-09-23 15:58 ` [Qemu-devel] [PATCH 00/19 v2] Port -net " Mark McLoughlin
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 f3dc421..4905157 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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 24/24] Final net cleanup after conversion to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (22 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 23/24] Port PCI NIC hotplug " Mark McLoughlin
@ 2009-09-23 10:24 ` Mark McLoughlin
  2009-09-23 15:58 ` [Qemu-devel] [PATCH 00/19 v2] Port -net " Mark McLoughlin
  24 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 10:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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            |  109 +++++++++++++++++++++++++++---------------------------
 net.h            |    5 +-
 vl.c             |   28 ++------------
 4 files changed, 62 insertions(+), 82 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 4905157..e1a4ed6 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 a192761..71b57ed 100644
--- a/net.c
+++ b/net.c
@@ -2995,7 +2995,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;
@@ -3024,37 +3024,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 (QTAILQ_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(QTAILQ_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--;
@@ -3108,7 +3077,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);
     }
 }
@@ -3130,26 +3099,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;
@@ -3230,7 +3179,7 @@ void net_cleanup(void)
     }
 }
 
-void net_client_check(void)
+static void net_check_clients(void)
 {
     VLANState *vlan;
 
@@ -3245,3 +3194,55 @@ 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 (QTAILQ_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 (QTAILQ_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(QTAILQ_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 1b849b4..2669287 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 f9e60ed..684f981 100644
--- a/vl.c
+++ b/vl.c
@@ -2506,7 +2506,7 @@ static int usb_device_add(const char *devname, int is_hotplug)
         qemu_opt_set(opts, "type", "nic");
         qemu_opt_set(opts, "model", "usb");
 
-        idx = net_client_init_from_opts(NULL, opts);
+        idx = net_client_init(NULL, opts);
         if (idx == -1) {
             return -1;
         }
@@ -4426,8 +4426,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)
@@ -4631,8 +4629,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;
@@ -4733,7 +4729,6 @@ int main(int argc, char **argv, char **envp)
         node_cpumask[i] = 0;
     }
 
-    nb_net_clients = 0;
     nb_numa_nodes = 0;
     nb_nics = 0;
 
@@ -4979,12 +4974,9 @@ 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:
@@ -5570,25 +5562,13 @@ 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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* Re: [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts
  2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
                   ` (23 preceding siblings ...)
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 24/24] Final net cleanup after conversion " Mark McLoughlin
@ 2009-09-23 15:58 ` Mark McLoughlin
  2009-09-30 10:33   ` Mark McLoughlin
  24 siblings, 1 reply; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-23 15:58 UTC (permalink / raw)
  To: qemu-devel

On Wed, 2009-09-23 at 11:23 +0100, Mark McLoughlin wrote:
> Hi,
>         I'm still not sure what happened the first series, but it
> needs rebasing now anyway, so ...
> 
>         Changes since v1:
> 
>   - Rebase to latest git, conflicts mainly with the qemu-queue.h
>     changes
> 
>   - Fix some coding style problems pointed out by people
> 
>   - Pull in Glauber's NICInfo fix, since it would also conflict;
>     also add some cleanups related to that fix
> 
>         Original posting here:
> 
>   http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00617.html

FWIW, I've pushed these here also:

  http://git.et.redhat.com/?p=qemu-net.git

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts
  2009-09-23 15:58 ` [Qemu-devel] [PATCH 00/19 v2] Port -net " Mark McLoughlin
@ 2009-09-30 10:33   ` Mark McLoughlin
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
  0 siblings, 1 reply; 58+ messages in thread
From: Mark McLoughlin @ 2009-09-30 10:33 UTC (permalink / raw)
  To: qemu-devel

On Wed, 2009-09-23 at 16:58 +0100, Mark McLoughlin wrote:
> On Wed, 2009-09-23 at 11:23 +0100, Mark McLoughlin wrote:
> > Hi,
> >         I'm still not sure what happened the first series, but it
> > needs rebasing now anyway, so ...
> > 
> >         Changes since v1:
> > 
> >   - Rebase to latest git, conflicts mainly with the qemu-queue.h
> >     changes
> > 
> >   - Fix some coding style problems pointed out by people
> > 
> >   - Pull in Glauber's NICInfo fix, since it would also conflict;
> >     also add some cleanups related to that fix
> > 
> >         Original posting here:
> > 
> >   http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00617.html
> 
> FWIW, I've pushed these here also:
> 
>   http://git.et.redhat.com/?p=qemu-net.git

I've rebased this series again, this time on top of Markus's changes to
not exit() on error

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [Qemu-devel] [PATCH 16/24] Port -net tap to QemuOpts
  2009-09-23 10:24 ` [Qemu-devel] [PATCH 16/24] Port -net tap " Mark McLoughlin
@ 2009-09-30 19:41   ` Anthony Liguori
  2009-10-01  6:43     ` Mark McLoughlin
  0 siblings, 1 reply; 58+ messages in thread
From: Anthony Liguori @ 2009-09-30 19:41 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: qemu-devel

Mark McLoughlin wrote:
> -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");
> @@ -2546,6 +2543,92 @@ static int net_init_slirp(QemuOpts *opts, Monitor *mon)
>      return ret;
>  }
>   
Breaks the build.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [Qemu-devel] [PATCH 16/24] Port -net tap to QemuOpts
  2009-09-30 19:41   ` Anthony Liguori
@ 2009-10-01  6:43     ` Mark McLoughlin
  0 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-01  6:43 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On Wed, 2009-09-30 at 14:41 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > -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");
> > @@ -2546,6 +2543,92 @@ static int net_init_slirp(QemuOpts *opts, Monitor *mon)
> >      return ret;
> >  }
> >   
> Breaks the build.

Yes, on older linux hosts.

Fixed in my tree now; the check isn't needed at all because the sndbuf
QemuOpt isn't defined if TUNSETSNDBUF isn't defined

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH 00/19 v3] Port -net to QemuOpts
  2009-09-30 10:33   ` Mark McLoughlin
@ 2009-10-06 11:16     ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Register rtc options for -set Mark McLoughlin
                         ` (26 more replies)
  0 siblings, 27 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel

On Wed, 2009-09-30 at 11:33 +0100, Mark McLoughlin wrote:
> On Wed, 2009-09-23 at 16:58 +0100, Mark McLoughlin wrote:
> > On Wed, 2009-09-23 at 11:23 +0100, Mark McLoughlin wrote:
> > > Hi,
> > >         I'm still not sure what happened the first series, but it
> > > needs rebasing now anyway, so ...
> > > 
> > >         Changes since v1:
> > > 
> > >   - Rebase to latest git, conflicts mainly with the qemu-queue.h
> > >     changes
> > > 
> > >   - Fix some coding style problems pointed out by people
> > > 
> > >   - Pull in Glauber's NICInfo fix, since it would also conflict;
> > >     also add some cleanups related to that fix
> > > 
> > >         Original posting here:
> > > 
> > >   http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00617.html
> > 
> > FWIW, I've pushed these here also:
> > 
> >   http://git.et.redhat.com/?p=qemu-net.git
> 
> I've rebased this series again, this time on top of Markus's changes to
> not exit() on error

Another rebase:

  - Pulled in Jan's rtc_opts patch because it conflicts with the 
    net_opts patch

  - Glauber's "correctly free nd structure" is merged now, 05/27 is a 
    minor tweak to that patch

  - Fix broken build in 19/27 against older linux lacking TUNSETSNDBUF

  - Fix merge conflicts with rtc_opts and 'switch qemu-config to 
    qemu_error'

Build tested all targets on f11 (with and without TUNSETSNDBUF) and
mingw. Basic sanity (re-)checking of networking.

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Register rtc options for -set
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
                         ` (25 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jan Kiszka, Mark McLoughlin

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu-config.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index ca93dc7..2e396ae 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -176,6 +176,7 @@ static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
     &qemu_device_opts,
+    &qemu_rtc_opts,
     NULL,
 };
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Use qemu_strdup() for NICInfo string fields
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Register rtc options for -set Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't assign a static string to NICInfo::model Mark McLoughlin
                         ` (24 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/net.c b/net.c
index dc274c7..f2b954c 100644
--- a/net.c
+++ b/net.c
@@ -2380,7 +2380,7 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
     int i;
 
     if (!nd->model)
-        nd->model = strdup(default_model);
+        nd->model = qemu_strdup(default_model);
 
     for (i = 0 ; models[i]; i++) {
         if (strcmp(nd->model, models[i]) == 0)
@@ -2459,13 +2459,13 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             }
         }
         if (get_param_value(buf, sizeof(buf), "model", p)) {
-            nd->model = strdup(buf);
+            nd->model = qemu_strdup(buf);
         }
         if (get_param_value(buf, sizeof(buf), "addr", p)) {
-            nd->devaddr = strdup(buf);
+            nd->devaddr = qemu_strdup(buf);
         }
         if (get_param_value(buf, sizeof(buf), "id", p)) {
-            nd->id = strdup(buf);
+            nd->id = qemu_strdup(buf);
         }
         nd->nvectors = NIC_NVECTORS_UNSPECIFIED;
         if (get_param_value(buf, sizeof(buf), "vectors", p)) {
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Don't assign a static string to NICInfo::model
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Register rtc options for -set Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make NICInfo string fields non-const Mark McLoughlin
                         ` (23 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/vl.c b/vl.c
index be0776f..95868e7 100644
--- a/vl.c
+++ b/vl.c
@@ -2597,7 +2597,7 @@ static int usb_device_add(const char *devname, int is_hotplug)
 
         if (net_client_init(NULL, "nic", p) < 0)
             return -1;
-        nd_table[nic].model = "usb";
+        nd_table[nic].model = qemu_strdup("usb");
         dev = usb_net_init(&nd_table[nic]);
     } else if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
         dev = usb_bt_init(devname[2] ? hci_init(p) :
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Make NICInfo string fields non-const
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (2 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't assign a static string to NICInfo::model Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 19:19         ` Anthony Liguori
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Move memset() from net_client_uninit() to net_client_init() Mark McLoughlin
                         ` (22 subsequent siblings)
  26 siblings, 1 reply; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

We now only assign strdup()ed strings to these fields, never static
strings.

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

diff --git a/net.c b/net.c
index f2b954c..76e44c9 100644
--- a/net.c
+++ b/net.c
@@ -2813,10 +2813,10 @@ void net_client_uninit(NICInfo *nd)
     nd->vlan->nb_guest_devs--;
     nb_nics--;
 
-    qemu_free((void *)nd->model);
-    qemu_free((void *)nd->name);
-    qemu_free((void *)nd->devaddr);
-    qemu_free((void *)nd->id);
+    qemu_free(nd->model);
+    qemu_free(nd->name);
+    qemu_free(nd->devaddr);
+    qemu_free(nd->id);
 
     memset(nd, 0, sizeof(*nd));
 }
diff --git a/net.h b/net.h
index dfce8d6..a36df45 100644
--- a/net.h
+++ b/net.h
@@ -95,10 +95,10 @@ enum {
 
 struct NICInfo {
     uint8_t macaddr[6];
-    const char *model;
-    const char *name;
-    const char *devaddr;
-    const char *id;
+    char *model;
+    char *name;
+    char *devaddr;
+    char *id;
     VLANState *vlan;
     VLANClientState *vc;
     void *private;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Move memset() from net_client_uninit() to net_client_init()
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (3 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make NICInfo string fields non-const Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
                         ` (21 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

zeroing a structure before using it is more common than zeroing after
using it. Also makes the setting of nd->used more obvious.

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

diff --git a/net.c b/net.c
index 76e44c9..f2b472d 100644
--- a/net.c
+++ b/net.c
@@ -2443,6 +2443,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             goto out;
         }
         nd = &nd_table[idx];
+        memset(nd, 0, sizeof(*nd));
         macaddr = nd->macaddr;
         macaddr[0] = 0x52;
         macaddr[1] = 0x54;
@@ -2818,7 +2819,7 @@ void net_client_uninit(NICInfo *nd)
     qemu_free(nd->devaddr);
     qemu_free(nd->id);
 
-    memset(nd, 0, sizeof(*nd));
+    nd->used = 0;
 }
 
 static int net_host_check_device(const char *device)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Use qemu_strdup() for VLANClientState string fields
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (4 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Move memset() from net_client_uninit() to net_client_init() Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make net_client_init() consume slirp_configs even on error Mark McLoughlin
                         ` (20 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin

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

diff --git a/net.c b/net.c
index f2b472d..0e5afa9 100644
--- a/net.c
+++ b/net.c
@@ -296,7 +296,7 @@ static char *assign_name(VLANClientState *vc1, const char *model)
 
     snprintf(buf, sizeof(buf), "%s.%d", model, id);
 
-    return strdup(buf);
+    return qemu_strdup(buf);
 }
 
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
@@ -310,9 +310,9 @@ VLANClientState *qemu_new_vlan_client(VLANState *vlan,
 {
     VLANClientState *vc, **pvc;
     vc = qemu_mallocz(sizeof(VLANClientState));
-    vc->model = strdup(model);
+    vc->model = qemu_strdup(model);
     if (name)
-        vc->name = strdup(name);
+        vc->name = qemu_strdup(name);
     else
         vc->name = assign_name(vc, model);
     vc->can_receive = can_receive;
@@ -340,8 +340,8 @@ void qemu_del_vlan_client(VLANClientState *vc)
             if (vc->cleanup) {
                 vc->cleanup(vc);
             }
-            free(vc->name);
-            free(vc->model);
+            qemu_free(vc->name);
+            qemu_free(vc->model);
             qemu_free(vc);
             break;
         } else
@@ -2127,8 +2127,8 @@ static int net_socket_listen_init(VLANState *vlan,
         return -1;
     }
     s->vlan = vlan;
-    s->model = strdup(model);
-    s->name = name ? strdup(name) : NULL;
+    s->model = qemu_strdup(model);
+    s->name = name ? qemu_strdup(name) : NULL;
     s->fd = fd;
     qemu_set_fd_handler(fd, net_socket_accept, NULL, s);
     return 0;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Make net_client_init() consume slirp_configs even on error
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (5 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't exit() in config_error() Mark McLoughlin
                         ` (19 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

net_slirp_init() walks slirp_configs, and stops when it encounters one
that doesn't work.  Instead of consuming slirp_configs members there,
consume them in the sole caller.  This makes sure all are consumed.
Before, the tail starting with the non-working one was left in place,
where it made the next net_slirp_init() fail again.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index 0e5afa9..a2507f9 100644
--- a/net.c
+++ b/net.c
@@ -761,6 +761,7 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
     uint32_t addr;
     int shift;
     char *end;
+    struct slirp_config_str *config;
 
     if (!tftp_export) {
         tftp_export = legacy_tftp_prefix;
@@ -845,9 +846,7 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
                           tftp_export, bootfile, dhcp, dns, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
-    while (slirp_configs) {
-        struct slirp_config_str *config = slirp_configs;
-
+    for (config = slirp_configs; config; config = config->next) {
         if (config->flags & SLIRP_CFG_HOSTFWD) {
             slirp_hostfwd(s, mon, config->str,
                           config->flags & SLIRP_CFG_LEGACY);
@@ -855,8 +854,6 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
             slirp_guestfwd(s, mon, config->str,
                            config->flags & SLIRP_CFG_LEGACY);
         }
-        slirp_configs = config->next;
-        qemu_free(config);
     }
 #ifndef _WIN32
     if (!smb_export) {
@@ -2593,6 +2590,11 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         ret = net_slirp_init(mon, vlan, device, name, restricted, vnet, vhost,
                              vhostname, tftp_export, bootfile, vdhcp_start,
                              vnamesrv, smb_export, vsmbsrv);
+        while (slirp_configs) {
+            config = slirp_configs;
+            slirp_configs = config->next;
+            qemu_free(config);
+        }
         qemu_free(vnet);
         qemu_free(vhost);
         qemu_free(vhostname);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Don't exit() in config_error()
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (6 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make net_client_init() consume slirp_configs even on error Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Drop config_error(), use qemu_error() instead Mark McLoughlin
                         ` (18 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

Propagating errors up the call chain is tedious.  In startup code, we
can take a shortcut: terminate the program.  This is wrong elsewhere,
the monitor in particular.

config_error() tries to cater for both customers: it terminates the
program unless its mon parameter tells it it's working for the
monitor.

Its users need to return status anyway (unless passing a null mon
argument, which none do), which their users need to check.  So this
automatic exit buys us exactly nothing useful.  Only the dangerous
delusion that we can get away without returning status.  Some of its
users fell for that.  Their callers continue executing after failure
when working for the monitor.

This bites monitor command host_net_add in two places:

* net_slirp_init() continues after slirp_hostfwd(), slirp_guestfwd(),
  or slirp_smb() failed, and may end up reporting success.  This
  happens for "host_net_add user guestfwd=foo": it complains about the
  invalid guest forwarding rule, then happily creates the user network
  without guest forwarding.

* net_client_init() can't detect slirp_guestfwd() failure, and gets
  fooled by net_slirp_init() lying about success.  Suppresses its
  "Could not initialize device" message.

Add the missing error reporting, make sure errors are checked, and
drop the exit() from config_error().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |   83 ++++++++++++++++++++++++++++++++++++----------------------------
 net.h |    4 +-
 vl.c  |    9 ++++--
 3 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/net.c b/net.c
index a2507f9..8e4a752 100644
--- a/net.c
+++ b/net.c
@@ -650,7 +650,6 @@ static void config_error(Monitor *mon, const char *fmt, ...)
     } else {
         fprintf(stderr, "qemu: ");
         vfprintf(stderr, fmt, ap);
-        exit(1);
     }
     va_end(ap);
 }
@@ -684,16 +683,16 @@ const char *legacy_bootp_filename;
 static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
     QTAILQ_HEAD_INITIALIZER(slirp_stacks);
 
-static void slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
+static int slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
+                         int legacy_format);
+static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
                           int legacy_format);
-static void slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
-                           int legacy_format);
 
 #ifndef _WIN32
 static const char *legacy_smb_export;
 
-static void slirp_smb(SlirpState *s, Monitor *mon, const char *exported_dir,
-                      struct in_addr vserver_addr);
+static int slirp_smb(SlirpState *s, Monitor *mon, const char *exported_dir,
+                     struct in_addr vserver_addr);
 static void slirp_smb_cleanup(SlirpState *s);
 #else
 static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -848,11 +847,13 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
 
     for (config = slirp_configs; config; config = config->next) {
         if (config->flags & SLIRP_CFG_HOSTFWD) {
-            slirp_hostfwd(s, mon, config->str,
-                          config->flags & SLIRP_CFG_LEGACY);
+            if (slirp_hostfwd(s, mon, config->str,
+                              config->flags & SLIRP_CFG_LEGACY) < 0)
+                return -1;
         } else {
-            slirp_guestfwd(s, mon, config->str,
-                           config->flags & SLIRP_CFG_LEGACY);
+            if (slirp_guestfwd(s, mon, config->str,
+                               config->flags & SLIRP_CFG_LEGACY) < 0)
+                return -1;
         }
     }
 #ifndef _WIN32
@@ -860,7 +861,8 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
         smb_export = legacy_smb_export;
     }
     if (smb_export) {
-        slirp_smb(s, mon, smb_export, smbsrv);
+        if (slirp_smb(s, mon, smb_export, smbsrv) < 0)
+            return -1;
     }
 #endif
 
@@ -953,8 +955,8 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "invalid format\n");
 }
 
-static void slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
-                          int legacy_format)
+static int slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
+                         int legacy_format)
 {
     struct in_addr host_addr = { .s_addr = INADDR_ANY };
     struct in_addr guest_addr = { .s_addr = 0 };
@@ -1009,11 +1011,13 @@ static void slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
                           guest_port) < 0) {
         config_error(mon, "could not set up host forwarding rule '%s'\n",
                      redir_str);
+        return -1;
     }
-    return;
+    return 0;
 
  fail_syntax:
     config_error(mon, "invalid host forwarding rule '%s'\n", redir_str);
+    return -1;
 }
 
 void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict)
@@ -1037,7 +1041,7 @@ void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict)
 
 }
 
-void net_slirp_redir(const char *redir_str)
+int net_slirp_redir(const char *redir_str)
 {
     struct slirp_config_str *config;
 
@@ -1047,10 +1051,10 @@ void net_slirp_redir(const char *redir_str)
         config->flags = SLIRP_CFG_HOSTFWD | SLIRP_CFG_LEGACY;
         config->next = slirp_configs;
         slirp_configs = config;
-        return;
+        return 0;
     }
 
-    slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), NULL, redir_str, 1);
+    return slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), NULL, redir_str, 1);
 }
 
 #ifndef _WIN32
@@ -1067,8 +1071,8 @@ static void slirp_smb_cleanup(SlirpState *s)
     }
 }
 
-static void slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
-                      struct in_addr vserver_addr)
+static int slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
+                     struct in_addr vserver_addr)
 {
     static int instance;
     char smb_conf[128];
@@ -1080,7 +1084,7 @@ static void slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
     if (mkdir(s->smb_dir, 0700) < 0) {
         config_error(mon, "could not create samba server dir '%s'\n",
                      s->smb_dir);
-        return;
+        return -1;
     }
     snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
 
@@ -1089,7 +1093,7 @@ static void slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
         slirp_smb_cleanup(s);
         config_error(mon, "could not create samba server "
                      "configuration file '%s'\n", smb_conf);
-        return;
+        return -1;
     }
     fprintf(f,
             "[global]\n"
@@ -1120,23 +1124,26 @@ static void slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
     if (slirp_add_exec(s->slirp, 0, smb_cmdline, &vserver_addr, 139) < 0) {
         slirp_smb_cleanup(s);
         config_error(mon, "conflicting/invalid smbserver address\n");
+        return -1;
     }
+    return 0;
 }
 
 /* automatic user mode samba server configuration (legacy interface) */
-void net_slirp_smb(const char *exported_dir)
+int net_slirp_smb(const char *exported_dir)
 {
     struct in_addr vserver_addr = { .s_addr = 0 };
 
     if (legacy_smb_export) {
         fprintf(stderr, "-smb given twice\n");
-        exit(1);
+        return -1;
     }
     legacy_smb_export = exported_dir;
     if (!QTAILQ_EMPTY(&slirp_stacks)) {
-        slirp_smb(QTAILQ_FIRST(&slirp_stacks), NULL, exported_dir,
-                  vserver_addr);
+        return slirp_smb(QTAILQ_FIRST(&slirp_stacks), NULL, exported_dir,
+                         vserver_addr);
     }
+    return 0;
 }
 
 #endif /* !defined(_WIN32) */
@@ -1160,8 +1167,8 @@ static void guestfwd_read(void *opaque, const uint8_t *buf, int size)
     slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size);
 }
 
-static void slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
-                           int legacy_format)
+static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
+                          int legacy_format)
 {
     struct in_addr server = { .s_addr = 0 };
     struct GuestFwd *fwd;
@@ -1204,14 +1211,14 @@ static void slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
         config_error(mon, "could not open guest forwarding device '%s'\n",
                      buf);
         qemu_free(fwd);
-        return;
+        return -1;
     }
 
     if (slirp_add_exec(s->slirp, 3, fwd->hd, &server, port) < 0) {
         config_error(mon, "conflicting/invalid host:port in guest forwarding "
                      "rule '%s'\n", config_str);
         qemu_free(fwd);
-        return;
+        return -1;
     }
     fwd->server = server;
     fwd->port = port;
@@ -1219,10 +1226,11 @@ static void slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
 
     qemu_chr_add_handlers(fwd->hd, guestfwd_can_read, guestfwd_read,
                           NULL, fwd);
-    return;
+    return 0;
 
  fail_syntax:
     config_error(mon, "invalid guest forwarding rule '%s'\n", config_str);
+    return -1;
 }
 
 void do_info_usernet(Monitor *mon)
@@ -1372,7 +1380,7 @@ 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 int tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
 {
     int sndbuf = TAP_DEFAULT_SNDBUF;
 
@@ -1387,14 +1395,18 @@ static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
     if (ioctl(s->fd, TUNSETSNDBUF, &sndbuf) == -1 && sndbuf_str) {
         config_error(mon, "TUNSETSNDBUF ioctl failed: %s\n",
                      strerror(errno));
+        return -1;
     }
+    return 0;
 }
 #else
-static void tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
 {
     if (sndbuf_str) {
         config_error(mon, "No '-net tap,sndbuf=<nbytes>' support available\n");
+        return -1;
     }
+    return 0;
 }
 #endif /* TUNSETSNDBUF */
 
@@ -2613,10 +2625,10 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             config->flags = SLIRP_CFG_LEGACY;
             config->next = slirp_configs;
             slirp_configs = config;
+            ret = 0;
         } else {
-            slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), mon, p, 1);
+            ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), mon, p, 1);
         }
-        ret = 0;
     } else
 #endif
 #ifdef _WIN32
@@ -2690,8 +2702,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             if (get_param_value(buf, sizeof(buf), "sndbuf", p)) {
                 sndbuf_str = buf;
             }
-            tap_set_sndbuf(s, sndbuf_str, mon);
-            ret = 0;
+            ret = tap_set_sndbuf(s, sndbuf_str, mon);
         } else {
             ret = -1;
         }
diff --git a/net.h b/net.h
index a36df45..a2d5b3c 100644
--- a/net.h
+++ b/net.h
@@ -138,10 +138,10 @@ extern const char *legacy_bootp_filename;
 int net_client_init(Monitor *mon, const char *device, const char *p);
 void net_client_uninit(NICInfo *nd);
 int net_client_parse(const char *str);
-void net_slirp_smb(const char *exported_dir);
+int 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);
+int net_slirp_redir(const char *redir_str);
 void net_cleanup(void);
 void net_client_check(void);
 void net_set_boot_mask(int boot_mask);
diff --git a/vl.c b/vl.c
index 95868e7..b7d370b 100644
--- a/vl.c
+++ b/vl.c
@@ -5096,11 +5096,13 @@ int main(int argc, char **argv, char **envp)
                 break;
 #ifndef _WIN32
             case QEMU_OPTION_smb:
-                net_slirp_smb(optarg);
+                if (net_slirp_smb(optarg) < 0)
+                    exit(1);
                 break;
 #endif
             case QEMU_OPTION_redir:
-                net_slirp_redir(optarg);
+                if (net_slirp_redir(optarg) < 0)
+                    exit(1);
                 break;
 #endif
             case QEMU_OPTION_bt:
@@ -5844,7 +5846,8 @@ int main(int argc, char **argv, char **envp)
 
     /* init USB devices */
     if (usb_enabled) {
-        foreach_device_config(DEV_USB, usb_parse);
+        if (foreach_device_config(DEV_USB, usb_parse) < 0)
+            exit(1);
     }
 
     /* init generic devices */
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Drop config_error(), use qemu_error() instead
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (7 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't exit() in config_error() Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Remove bogus error message from qemu_opts_set() Mark McLoughlin
                         ` (17 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

Diagnostic output goes to stderr, except when we're in a monitor
command, when it goes to the monitor instead.

config_error() implements this with a monitor argument: if it's
non-null, report there, else to stderr.  This obliges us to pass the
monitor down various call chains, to make it available to
config_error().

The recently created qemu_error() doesn't need a monitor argument to
route output.  Use it.

There's one user-visible difference: config_error() prepended "qemu: "
to a message bound for stderr.  qemu_error() doesn't, which means the
prefix goes away with this commit.  If such a prefix is desired for
stderr, then I figure it should be slapped on all error messages, not
just the ones that used to go through config_error().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net.c |  127 ++++++++++++++++++++++++++++-------------------------------------
 1 files changed, 55 insertions(+), 72 deletions(-)

diff --git a/net.c b/net.c
index 8e4a752..2a95935 100644
--- a/net.c
+++ b/net.c
@@ -640,20 +640,6 @@ qemu_sendv_packet(VLANClientState *vc, const struct iovec *iov, int iovcnt)
     return qemu_sendv_packet_async(vc, iov, iovcnt, NULL);
 }
 
-static void config_error(Monitor *mon, const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    if (mon) {
-        monitor_vprintf(mon, fmt, ap);
-    } else {
-        fprintf(stderr, "qemu: ");
-        vfprintf(stderr, fmt, ap);
-    }
-    va_end(ap);
-}
-
 #if defined(CONFIG_SLIRP)
 
 /* slirp network adapter */
@@ -683,15 +669,15 @@ const char *legacy_bootp_filename;
 static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
     QTAILQ_HEAD_INITIALIZER(slirp_stacks);
 
-static int slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
+static int slirp_hostfwd(SlirpState *s, const char *redir_str,
                          int legacy_format);
-static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
+static int slirp_guestfwd(SlirpState *s, const char *config_str,
                           int legacy_format);
 
 #ifndef _WIN32
 static const char *legacy_smb_export;
 
-static int slirp_smb(SlirpState *s, Monitor *mon, const char *exported_dir,
+static int slirp_smb(SlirpState *s, const char *exported_dir,
                      struct in_addr vserver_addr);
 static void slirp_smb_cleanup(SlirpState *s);
 #else
@@ -738,7 +724,7 @@ static void net_slirp_cleanup(VLANClientState *vc)
     qemu_free(s);
 }
 
-static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
+static int net_slirp_init(VLANState *vlan, const char *model,
                           const char *name, int restricted,
                           const char *vnetwork, const char *vhost,
                           const char *vhostname, const char *tftp_export,
@@ -847,11 +833,11 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
 
     for (config = slirp_configs; config; config = config->next) {
         if (config->flags & SLIRP_CFG_HOSTFWD) {
-            if (slirp_hostfwd(s, mon, config->str,
+            if (slirp_hostfwd(s, config->str,
                               config->flags & SLIRP_CFG_LEGACY) < 0)
                 return -1;
         } else {
-            if (slirp_guestfwd(s, mon, config->str,
+            if (slirp_guestfwd(s, config->str,
                                config->flags & SLIRP_CFG_LEGACY) < 0)
                 return -1;
         }
@@ -861,7 +847,7 @@ static int net_slirp_init(Monitor *mon, VLANState *vlan, const char *model,
         smb_export = legacy_smb_export;
     }
     if (smb_export) {
-        if (slirp_smb(s, mon, smb_export, smbsrv) < 0)
+        if (slirp_smb(s, smb_export, smbsrv) < 0)
             return -1;
     }
 #endif
@@ -955,7 +941,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "invalid format\n");
 }
 
-static int slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
+static int slirp_hostfwd(SlirpState *s, const char *redir_str,
                          int legacy_format)
 {
     struct in_addr host_addr = { .s_addr = INADDR_ANY };
@@ -1009,14 +995,14 @@ static int slirp_hostfwd(SlirpState *s, Monitor *mon, const char *redir_str,
 
     if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, guest_addr,
                           guest_port) < 0) {
-        config_error(mon, "could not set up host forwarding rule '%s'\n",
-                     redir_str);
+        qemu_error("could not set up host forwarding rule '%s'\n",
+                   redir_str);
         return -1;
     }
     return 0;
 
  fail_syntax:
-    config_error(mon, "invalid host forwarding rule '%s'\n", redir_str);
+    qemu_error("invalid host forwarding rule '%s'\n", redir_str);
     return -1;
 }
 
@@ -1036,7 +1022,7 @@ void net_slirp_hostfwd_add(Monitor *mon, const QDict *qdict)
         redir_str = arg1;
     }
     if (s) {
-        slirp_hostfwd(s, mon, redir_str, 0);
+        slirp_hostfwd(s, redir_str, 0);
     }
 
 }
@@ -1054,7 +1040,7 @@ int net_slirp_redir(const char *redir_str)
         return 0;
     }
 
-    return slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), NULL, redir_str, 1);
+    return slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), redir_str, 1);
 }
 
 #ifndef _WIN32
@@ -1071,7 +1057,7 @@ static void slirp_smb_cleanup(SlirpState *s)
     }
 }
 
-static int slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
+static int slirp_smb(SlirpState* s, const char *exported_dir,
                      struct in_addr vserver_addr)
 {
     static int instance;
@@ -1082,8 +1068,7 @@ static int slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
     snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
              (long)getpid(), instance++);
     if (mkdir(s->smb_dir, 0700) < 0) {
-        config_error(mon, "could not create samba server dir '%s'\n",
-                     s->smb_dir);
+        qemu_error("could not create samba server dir '%s'\n", s->smb_dir);
         return -1;
     }
     snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
@@ -1091,8 +1076,8 @@ static int slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
     f = fopen(smb_conf, "w");
     if (!f) {
         slirp_smb_cleanup(s);
-        config_error(mon, "could not create samba server "
-                     "configuration file '%s'\n", smb_conf);
+        qemu_error("could not create samba server configuration file '%s'\n",
+                   smb_conf);
         return -1;
     }
     fprintf(f,
@@ -1123,7 +1108,7 @@ static int slirp_smb(SlirpState* s, Monitor *mon, const char *exported_dir,
 
     if (slirp_add_exec(s->slirp, 0, smb_cmdline, &vserver_addr, 139) < 0) {
         slirp_smb_cleanup(s);
-        config_error(mon, "conflicting/invalid smbserver address\n");
+        qemu_error("conflicting/invalid smbserver address\n");
         return -1;
     }
     return 0;
@@ -1140,7 +1125,7 @@ int net_slirp_smb(const char *exported_dir)
     }
     legacy_smb_export = exported_dir;
     if (!QTAILQ_EMPTY(&slirp_stacks)) {
-        return slirp_smb(QTAILQ_FIRST(&slirp_stacks), NULL, exported_dir,
+        return slirp_smb(QTAILQ_FIRST(&slirp_stacks), exported_dir,
                          vserver_addr);
     }
     return 0;
@@ -1167,7 +1152,7 @@ static void guestfwd_read(void *opaque, const uint8_t *buf, int size)
     slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size);
 }
 
-static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
+static int slirp_guestfwd(SlirpState *s, const char *config_str,
                           int legacy_format)
 {
     struct in_addr server = { .s_addr = 0 };
@@ -1208,15 +1193,14 @@ static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
     snprintf(buf, sizeof(buf), "guestfwd.tcp:%d", port);
     fwd->hd = qemu_chr_open(buf, p, NULL);
     if (!fwd->hd) {
-        config_error(mon, "could not open guest forwarding device '%s'\n",
-                     buf);
+        qemu_error("could not open guest forwarding device '%s'\n", buf);
         qemu_free(fwd);
         return -1;
     }
 
     if (slirp_add_exec(s->slirp, 3, fwd->hd, &server, port) < 0) {
-        config_error(mon, "conflicting/invalid host:port in guest forwarding "
-                     "rule '%s'\n", config_str);
+        qemu_error("conflicting/invalid host:port in guest forwarding "
+                   "rule '%s'\n", config_str);
         qemu_free(fwd);
         return -1;
     }
@@ -1229,7 +1213,7 @@ static int slirp_guestfwd(SlirpState *s, Monitor *mon, const char *config_str,
     return 0;
 
  fail_syntax:
-    config_error(mon, "invalid guest forwarding rule '%s'\n", config_str);
+    qemu_error("invalid guest forwarding rule '%s'\n", config_str);
     return -1;
 }
 
@@ -1380,7 +1364,7 @@ static void tap_send(void *opaque)
  */
 #define TAP_DEFAULT_SNDBUF 1024*1024
 
-static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str)
 {
     int sndbuf = TAP_DEFAULT_SNDBUF;
 
@@ -1393,17 +1377,16 @@ static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
     }
 
     if (ioctl(s->fd, TUNSETSNDBUF, &sndbuf) == -1 && sndbuf_str) {
-        config_error(mon, "TUNSETSNDBUF ioctl failed: %s\n",
-                     strerror(errno));
+        qemu_error("TUNSETSNDBUF ioctl failed: %s\n", strerror(errno));
         return -1;
     }
     return 0;
 }
 #else
-static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str, Monitor *mon)
+static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str)
 {
     if (sndbuf_str) {
-        config_error(mon, "No '-net tap,sndbuf=<nbytes>' support available\n");
+        qemu_error("No '-net tap,sndbuf=<nbytes>' support available\n");
         return -1;
     }
     return 0;
@@ -2287,7 +2270,7 @@ static void net_dump_cleanup(VLANClientState *vc)
     qemu_free(s);
 }
 
-static int net_dump_init(Monitor *mon, VLANState *vlan, const char *device,
+static int net_dump_init(VLANState *vlan, const char *device,
                          const char *name, const char *filename, int len)
 {
     struct pcap_file_hdr hdr;
@@ -2297,7 +2280,7 @@ static int net_dump_init(Monitor *mon, VLANState *vlan, const char *device,
 
     s->fd = open(filename, O_CREAT | O_WRONLY | O_BINARY, 0644);
     if (s->fd < 0) {
-        config_error(mon, "-net dump: can't open %s\n", filename);
+        qemu_error("-net dump: can't open %s\n", filename);
         return -1;
     }
 
@@ -2312,7 +2295,7 @@ static int net_dump_init(Monitor *mon, VLANState *vlan, const char *device,
     hdr.linktype = 1;
 
     if (write(s->fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
-        config_error(mon, "-net dump write error: %s\n", strerror(errno));
+        qemu_error("-net dump write error: %s\n", strerror(errno));
         close(s->fd);
         qemu_free(s);
         return -1;
@@ -2407,7 +2390,7 @@ static int net_handle_fd_param(Monitor *mon, const char *param)
 
         fd = monitor_get_fd(mon, param);
         if (fd == -1) {
-            config_error(mon, "No file descriptor named %s found", param);
+            qemu_error("No file descriptor named %s found", param);
             return -1;
         }
 
@@ -2442,12 +2425,12 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         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);
+            qemu_error("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");
+            qemu_error("Too Many NICs\n");
             ret = -1;
             goto out;
         }
@@ -2463,7 +2446,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
 
         if (get_param_value(buf, sizeof(buf), "macaddr", p)) {
             if (parse_macaddr(macaddr, buf) < 0) {
-                config_error(mon, "invalid syntax for ethernet address\n");
+                qemu_error("invalid syntax for ethernet address\n");
                 ret = -1;
                 goto out;
             }
@@ -2482,12 +2465,12 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             char *endptr;
             long vectors = strtol(buf, &endptr, 0);
             if (*endptr) {
-                config_error(mon, "invalid syntax for # of vectors\n");
+                qemu_error("invalid syntax for # of vectors\n");
                 ret = -1;
                 goto out;
             }
             if (vectors < 0 || vectors > 0x7ffffff) {
-                config_error(mon, "invalid # of vectors\n");
+                qemu_error("invalid # of vectors\n");
                 ret = -1;
                 goto out;
             }
@@ -2503,7 +2486,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
     } else
     if (!strcmp(device, "none")) {
         if (*p != '\0') {
-            config_error(mon, "'none' takes no parameters\n");
+            qemu_error("'none' takes no parameters\n");
             ret = -1;
             goto out;
         }
@@ -2532,7 +2515,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         const char *q;
 
         if (check_params(buf, sizeof(buf), slirp_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
+            qemu_error("invalid parameter '%s' in '%s'\n", buf, p);
             ret = -1;
             goto out;
         }
@@ -2599,7 +2582,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         }
         qemu_free(config);
         vlan->nb_host_devs++;
-        ret = net_slirp_init(mon, vlan, device, name, restricted, vnet, vhost,
+        ret = net_slirp_init(vlan, device, name, restricted, vnet, vhost,
                              vhostname, tftp_export, bootfile, vdhcp_start,
                              vnamesrv, smb_export, vsmbsrv);
         while (slirp_configs) {
@@ -2627,7 +2610,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             slirp_configs = config;
             ret = 0;
         } else {
-            ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), mon, p, 1);
+            ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), p, 1);
         }
     } else
 #endif
@@ -2639,12 +2622,12 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         char ifname[64];
 
         if (check_params(buf, sizeof(buf), tap_params, p) < 0) {
-            config_error(mon, "invalid parameter '%s' in '%s'\n", buf, p);
+            qemu_error("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");
+            qemu_error("tap: no interface name\n");
             ret = -1;
             goto out;
         }
@@ -2665,7 +2648,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             };
             ret = -1;
             if (check_params(chkbuf, sizeof(chkbuf), fd_params, p) < 0) {
-                config_error(mon, "invalid parameter '%s' in '%s'\n", chkbuf, p);
+                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
                 goto out;
             }
             fd = net_handle_fd_param(mon, buf);
@@ -2682,7 +2665,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
                 "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);
+                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
                 ret = -1;
                 goto out;
             }
@@ -2702,7 +2685,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             if (get_param_value(buf, sizeof(buf), "sndbuf", p)) {
                 sndbuf_str = buf;
             }
-            ret = tap_set_sndbuf(s, sndbuf_str, mon);
+            ret = tap_set_sndbuf(s, sndbuf_str);
         } else {
             ret = -1;
         }
@@ -2717,7 +2700,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
             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);
+                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
                 goto out;
             }
             fd = net_handle_fd_param(mon, buf);
@@ -2734,7 +2717,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
                 "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);
+                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
                 ret = -1;
                 goto out;
             }
@@ -2744,7 +2727,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
                 "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);
+                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
                 ret = -1;
                 goto out;
             }
@@ -2754,13 +2737,13 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
                 "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);
+                qemu_error("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);
+            qemu_error("Unknown socket options: %s\n", p);
             ret = -1;
             goto out;
         }
@@ -2775,7 +2758,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
 	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);
+            qemu_error("invalid parameter '%s' in '%s'\n", buf, p);
             ret = -1;
             goto out;
         }
@@ -2808,14 +2791,14 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         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);
+        ret = net_dump_init(vlan, device, name, buf, len);
     } else {
-        config_error(mon, "Unknown network device: %s\n", device);
+        qemu_error("Unknown network device: %s\n", device);
         ret = -1;
         goto out;
     }
     if (ret < 0) {
-        config_error(mon, "Could not initialize device '%s'\n", device);
+        qemu_error("Could not initialize device '%s'\n", device);
     }
 out:
     qemu_free(name);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Remove bogus error message from qemu_opts_set()
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (8 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Drop config_error(), use qemu_error() instead Mark McLoughlin
@ 2009-10-06 11:16       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message in qemu_option_set() Mark McLoughlin
                         ` (16 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Remove double error message in qemu_option_set()
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (9 preceding siblings ...)
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Remove bogus error message from qemu_opts_set() Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message for -device option parsing Mark McLoughlin
                         ` (15 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 2e396ae..f5c1a12 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -209,8 +209,6 @@ int qemu_set_option(const char *str)
     }
 
     if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
-        qemu_error("failed to set \"%s\" for %s \"%s\"\n",
-                arg, lists[i]->name, id);
         return -1;
     }
     return 0;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Remove double error message for -device option parsing
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (10 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message in qemu_option_set() Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Make qemu_opts_parse() handle empty strings Mark McLoughlin
                         ` (14 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index b7d370b..811f26d 100644
--- a/vl.c
+++ b/vl.c
@@ -5354,9 +5354,7 @@ 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;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Make qemu_opts_parse() handle empty strings
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (11 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message for -device option parsing Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
                         ` (13 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 293f94c..735259f 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -712,8 +712,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)) {
@@ -750,7 +749,6 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
         if (*p != ',') {
             break;
         }
-        p++;
     }
     return 0;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Add qemu_opts_validate() for post parsing validation
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (12 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Make qemu_opts_parse() handle empty strings Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Never overwrite a QemuOpt Mark McLoughlin
                         ` (12 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |   33 +++++++++++++++++++++++++++++++++
 qemu-option.h |    1 +
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 735259f..de41c06 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -778,6 +778,39 @@ 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);
+
+    QTAILQ_FOREACH(opt, &opts->head, next) {
+        int i;
+
+        for (i = 0; desc[i].name != NULL; i++) {
+            if (strcmp(desc[i].name, opt->name) == 0) {
+                break;
+            }
+        }
+        if (desc[i].name == NULL) {
+            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 525b9b6..666b666 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Never overwrite a QemuOpt
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (13 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_net_opts Mark McLoughlin
                         ` (11 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 de41c06..49efd39 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -483,7 +483,7 @@ struct QemuOpt {
 struct QemuOpts {
     const char *id;
     QemuOptsList *list;
-    QTAILQ_HEAD(, QemuOpt) head;
+    QTAILQ_HEAD(QemuOptHead, QemuOpt) head;
     QTAILQ_ENTRY(QemuOpts) next;
 };
 
@@ -491,7 +491,7 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
 
-    QTAILQ_FOREACH(opt, &opts->head, next) {
+    QTAILQ_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;
-        QTAILQ_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;
+    QTAILQ_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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Add qemu_net_opts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (14 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Never overwrite a QemuOpt Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net none and -net nic to QemuOpts Mark McLoughlin
                         ` (10 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 f5c1a12..bafaea2 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -152,6 +152,18 @@ QemuOptsList qemu_device_opts = {
     },
 };
 
+QemuOptsList qemu_net_opts = {
+    .name = "net",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_net_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end of list */ }
+    },
+};
+
 QemuOptsList qemu_rtc_opts = {
     .name = "rtc",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
@@ -176,6 +188,7 @@ static QemuOptsList *lists[] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
     &qemu_device_opts,
+    &qemu_net_opts,
     &qemu_rtc_opts,
     NULL,
 };
diff --git a/qemu-config.h b/qemu-config.h
index 4ae7b74..cdad5ac 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;
 extern QemuOptsList qemu_rtc_opts;
 
 int qemu_set_option(const char *str);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net none and -net nic to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (15 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_net_opts Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net user " Mark McLoughlin
                         ` (9 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  239 ++++++++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 161 insertions(+), 78 deletions(-)

diff --git a/net.c b/net.c
index 2a95935..30f477a 100644
--- a/net.c
+++ b/net.c
@@ -110,6 +110,7 @@
 #include "audio/audio.h"
 #include "qemu_socket.h"
 #include "qemu-log.h"
+#include "qemu-config.h"
 
 #include "slirp/libslirp.h"
 #include "qemu-queue.h"
@@ -2400,6 +2401,151 @@ 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);
+
+    if (qemu_opts_id(opts)) {
+        nd->id = qemu_strdup(qemu_opts_id(opts));
+    }
+    if (qemu_opt_get(opts, "name")) {
+        nd->name = qemu_strdup(qemu_opt_get(opts, "name"));
+    }
+    if (qemu_opt_get(opts, "model")) {
+        nd->model = qemu_strdup(qemu_opt_get(opts, "model"));
+    }
+    if (qemu_opt_get(opts, "addr")) {
+        nd->devaddr = qemu_strdup(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 +2553,20 @@ 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,84 +2576,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) {
-            qemu_error("invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        if (idx == -1 || nb_nics >= MAX_NICS) {
-            qemu_error("Too Many NICs\n");
-            ret = -1;
-            goto out;
-        }
-        nd = &nd_table[idx];
-        memset(nd, 0, sizeof(*nd));
-        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) {
-                qemu_error("invalid syntax for ethernet address\n");
-                ret = -1;
-                goto out;
-            }
-        }
-        if (get_param_value(buf, sizeof(buf), "model", p)) {
-            nd->model = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "addr", p)) {
-            nd->devaddr = qemu_strdup(buf);
-        }
-        if (get_param_value(buf, sizeof(buf), "id", p)) {
-            nd->id = qemu_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) {
-                qemu_error("invalid syntax for # of vectors\n");
-                ret = -1;
-                goto out;
-            }
-            if (vectors < 0 || vectors > 0x7ffffff) {
-                qemu_error("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') {
-            qemu_error("'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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net user to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (16 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net none and -net nic to QemuOpts Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net tap " Mark McLoughlin
                         ` (8 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  267 +++++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 161 insertions(+), 106 deletions(-)

diff --git a/net.c b/net.c
index 30f477a..3ff4e4f 100644
--- a/net.c
+++ b/net.c
@@ -2458,6 +2458,102 @@ 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;
+    struct slirp_config_str *config;
+    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(vlan, "user", name, restricted, vnet, vhost,
+                         vhostname, tftp_export, bootfile, vdhcp_start,
+                         vnamesrv, smb_export, vsmbsrv);
+
+    while (slirp_configs) {
+        config = slirp_configs;
+        slirp_configs = config->next;
+        qemu_free(config);
+    }
+
+    if (ret != -1) {
+        vlan->nb_host_devs++;
+    }
+
+    qemu_free(vnet);
+
+    return ret;
+}
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2513,6 +2609,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 */ }
 };
@@ -2554,7 +2712,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);
@@ -2578,111 +2737,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) {
-            qemu_error("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(vlan, device, name, restricted, vnet, vhost,
-                             vhostname, tftp_export, bootfile, vdhcp_start,
-                             vnamesrv, smb_export, vsmbsrv);
-        while (slirp_configs) {
-            config = slirp_configs;
-            slirp_configs = config->next;
-            qemu_free(config);
-        }
-        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 (QTAILQ_EMPTY(&slirp_stacks)) {
             struct slirp_config_str *config;
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net tap to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (17 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net user " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net socket " Mark McLoughlin
                         ` (7 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |  230 +++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 140 insertions(+), 90 deletions(-)

diff --git a/net.c b/net.c
index 3ff4e4f..6a9a9eb 100644
--- a/net.c
+++ b/net.c
@@ -1365,31 +1365,24 @@ static void tap_send(void *opaque)
  */
 #define TAP_DEFAULT_SNDBUF 1024*1024
 
-static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str)
+static int tap_set_sndbuf(TAPState *s, QemuOpts *opts)
 {
-    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")) {
         qemu_error("TUNSETSNDBUF ioctl failed: %s\n", strerror(errno));
         return -1;
     }
     return 0;
 }
 #else
-static int tap_set_sndbuf(TAPState *s, const char *sndbuf_str)
+static int tap_set_sndbuf(TAPState *s, QemuOpts *opts)
 {
-    if (sndbuf_str) {
-        qemu_error("No '-net tap,sndbuf=<nbytes>' support available\n");
-        return -1;
-    }
     return 0;
 }
 #endif /* TUNSETSNDBUF */
@@ -2554,6 +2547,94 @@ 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;
+    }
+
+    if (tap_set_sndbuf(s, opts) < 0) {
+        return -1;
+    }
+
+    vlan->nb_host_devs++;
+
+    return 0;
+}
+#endif
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2671,6 +2752,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 */ }
 };
@@ -2713,7 +2839,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);
@@ -2752,83 +2879,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         }
     } 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) {
-            qemu_error("invalid parameter '%s' in '%s'\n", buf, p);
-            ret = -1;
-            goto out;
-        }
-        if (get_param_value(ifname, sizeof(ifname), "ifname", p) <= 0) {
-            qemu_error("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) {
-                qemu_error("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) {
-                qemu_error("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;
-            }
-            ret = tap_set_sndbuf(s, sndbuf_str);
-        } 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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net socket to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (18 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net tap " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net vde " Mark McLoughlin
                         ` (6 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 6a9a9eb..a8f3c6a 100644
--- a/net.c
+++ b/net.c
@@ -2635,6 +2635,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",                            \
@@ -2797,6 +2880,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 */ }
 };
@@ -2840,7 +2947,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);
@@ -2879,64 +2987,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         }
     } 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) {
-                qemu_error("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) {
-                qemu_error("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) {
-                qemu_error("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) {
-                qemu_error("invalid parameter '%s' in '%s'\n", chkbuf, p);
-                ret = -1;
-                goto out;
-            }
-            ret = net_socket_mcast_init(vlan, device, name, buf);
-        } else {
-            qemu_error("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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net vde to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (19 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net socket " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net dump " Mark McLoughlin
                         ` (5 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |   94 ++++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 36 deletions(-)

diff --git a/net.c b/net.c
index a8f3c6a..8e216f6 100644
--- a/net.c
+++ b/net.c
@@ -1732,8 +1732,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,
@@ -2718,6 +2718,34 @@ 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",                            \
@@ -2904,6 +2932,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 */ }
 };
@@ -2948,7 +3002,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);
@@ -2987,39 +3042,6 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         }
     } 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) {
-            qemu_error("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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port -net dump to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (20 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net vde " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Clean up legacy code in net_client_init() Mark McLoughlin
                         ` (4 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 8e216f6..fd4af3c 100644
--- a/net.c
+++ b/net.c
@@ -2746,6 +2746,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(vlan, "dump", name, file, len);
+}
+
 #define NET_COMMON_PARAMS_DESC                     \
     {                                              \
         .name = "type",                            \
@@ -2958,6 +2981,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 */ }
 };
@@ -3003,7 +3042,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);
@@ -3042,17 +3082,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         }
     } 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(vlan, device, name, buf, len);
-    } else {
+    {
         qemu_error("Unknown network device: %s\n", device);
         ret = -1;
         goto out;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Clean up legacy code in net_client_init()
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (21 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net dump " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port host_net_add monitor command to QemuOpts Mark McLoughlin
                         ` (3 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 |   58 ++++++++++++++--------------------------------------------
 1 files changed, 14 insertions(+), 44 deletions(-)

diff --git a/net.c b/net.c
index fd4af3c..534305c 100644
--- a/net.c
+++ b/net.c
@@ -3032,42 +3032,12 @@ 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")) {
+        int ret;
+
         if (QTAILQ_EMPTY(&slirp_stacks)) {
             struct slirp_config_str *config;
 
@@ -3080,19 +3050,19 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         } else {
             ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), p, 1);
         }
-    } else
-#endif
-    {
-        qemu_error("Unknown network device: %s\n", device);
-        ret = -1;
-        goto out;
+
+        return ret;
     }
-    if (ret < 0) {
-        qemu_error("Could not initialize device '%s'\n", device);
+#endif
+
+    opts = qemu_opts_parse(&qemu_net_opts, p, NULL);
+    if (!opts) {
+      return -1;
     }
-out:
-    qemu_free(name);
-    return ret;
+
+    qemu_opt_set(opts, "type", device);
+
+    return net_client_init_from_opts(mon, opts);
 }
 
 void net_client_uninit(NICInfo *nd)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port host_net_add monitor command to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (22 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Clean up legacy code in net_client_init() Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port usb net " Mark McLoughlin
                         ` (2 subsequent siblings)
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 534305c..1ebf147 100644
--- a/net.c
+++ b/net.c
@@ -3101,13 +3101,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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port usb net to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (23 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port host_net_add monitor command to QemuOpts Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port PCI NIC hotplug " Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Final net cleanup after conversion " Mark McLoughlin
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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  |   19 +++++++++++++++----
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index 1ebf147..c8c8c78 100644
--- a/net.c
+++ b/net.c
@@ -3001,7 +3001,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 a2d5b3c..27a1c50 100644
--- a/net.h
+++ b/net.h
@@ -4,6 +4,7 @@
 #include "qemu-queue.h"
 #include "qemu-common.h"
 #include "qdict.h"
+#include "qemu-option.h"
 
 /* VLANs support */
 
@@ -136,6 +137,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);
 int net_slirp_smb(const char *exported_dir);
diff --git a/vl.c b/vl.c
index 811f26d..70d1aed 100644
--- a/vl.c
+++ b/vl.c
@@ -2593,12 +2593,23 @@ 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 = qemu_strdup("usb");
-        dev = usb_net_init(&nd_table[nic]);
+        }
+
+        qemu_opt_set(opts, "type", "nic");
+        qemu_opt_set(opts, "model", "usb");
+
+        idx = net_client_init_from_opts(NULL, opts);
+        if (idx == -1) {
+            return -1;
+        }
+
+        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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Port PCI NIC hotplug to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (24 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port usb net " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Final net cleanup after conversion " Mark McLoughlin
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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 f781386..b5d532d 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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [Qemu-devel] [PATCH] Final net cleanup after conversion to QemuOpts
  2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
                         ` (25 preceding siblings ...)
  2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port PCI NIC hotplug " Mark McLoughlin
@ 2009-10-06 11:17       ` Mark McLoughlin
  26 siblings, 0 replies; 58+ messages in thread
From: Mark McLoughlin @ 2009-10-06 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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            |  116 +++++++++++++++++++++++++++---------------------------
 net.h            |    5 +-
 vl.c             |   28 ++-----------
 4 files changed, 65 insertions(+), 86 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index b5d532d..dcb5f0f 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 c8c8c78..2e4dd58 100644
--- a/net.c
+++ b/net.c
@@ -3001,7 +3001,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;
@@ -3030,41 +3030,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")) {
-        int ret;
-
-        if (QTAILQ_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;
-            ret = 0;
-        } else {
-            ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), p, 1);
-        }
-
-        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)
 {
     nd->vlan->nb_guest_devs--;
@@ -3118,7 +3083,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);
     }
 }
@@ -3140,26 +3105,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;
@@ -3240,7 +3185,7 @@ void net_cleanup(void)
     }
 }
 
-void net_client_check(void)
+static void net_check_clients(void)
 {
     VLANState *vlan;
 
@@ -3255,3 +3200,58 @@ 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 (QTAILQ_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,"))) {
+        int ret;
+
+        optarg += strlen("channel,");
+
+        if (QTAILQ_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;
+            ret = 0;
+        } else {
+            ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), optarg, 1);
+        }
+
+        return ret;
+    }
+
+    if (!qemu_opts_parse(&qemu_net_opts, optarg, "type")) {
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/net.h b/net.h
index 27a1c50..2b0ed9b 100644
--- a/net.h
+++ b/net.h
@@ -136,16 +136,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);
 int 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);
 int 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 70d1aed..a5a56af 100644
--- a/vl.c
+++ b/vl.c
@@ -2604,7 +2604,7 @@ static int usb_device_add(const char *devname, int is_hotplug)
         qemu_opt_set(opts, "type", "nic");
         qemu_opt_set(opts, "model", "usb");
 
-        idx = net_client_init_from_opts(NULL, opts);
+        idx = net_client_init(NULL, opts);
         if (idx == -1) {
             return -1;
         }
@@ -4536,8 +4536,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)
@@ -4741,8 +4739,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;
@@ -4845,7 +4841,6 @@ int main(int argc, char **argv, char **envp)
         node_cpumask[i] = 0;
     }
 
-    nb_net_clients = 0;
     nb_numa_nodes = 0;
     nb_nics = 0;
 
@@ -5091,12 +5086,9 @@ 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:
@@ -5656,25 +5648,13 @@ 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

^ permalink raw reply related	[flat|nested] 58+ messages in thread

* Re: [Qemu-devel] [PATCH] Make NICInfo string fields non-const
  2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make NICInfo string fields non-const Mark McLoughlin
@ 2009-10-06 19:19         ` Anthony Liguori
  0 siblings, 0 replies; 58+ messages in thread
From: Anthony Liguori @ 2009-10-06 19:19 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: qemu-devel

Mark McLoughlin wrote:
> We now only assign strdup()ed strings to these fields, never static
> strings.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
>   

This breaks the build because of a literal assignment in mips_jazz.c.

Regards,

Anthony Liguori
>      void *private;
>   

^ permalink raw reply	[flat|nested] 58+ messages in thread

end of thread, other threads:[~2009-10-06 19:20 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-23 10:23 [Qemu-devel] [PATCH 00/19 v2] Port -net to QemuOpts Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 01/24] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 02/24] Don't assign a static string to NICInfo::model Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 03/24] Make NICInfo string fields non-const Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 04/24] Correctly free nd structure Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 05/24] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 06/24] Fix coding style issue Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 07/24] Remove bogus error message from qemu_opts_set() Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 08/24] Remove double error message in qemu_option_set() Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 09/24] Remove double error message for -device option parsing Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 10/24] Make qemu_opts_parse() handle empty strings Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 11/24] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 12/24] Never overwrite a QemuOpt Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 13/24] Add qemu_net_opts Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 14/24] Port -net none and -net nic to QemuOpts Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 15/24] Port -net user " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 16/24] Port -net tap " Mark McLoughlin
2009-09-30 19:41   ` Anthony Liguori
2009-10-01  6:43     ` Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 17/24] Port -net socket " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 18/24] Port -net vde " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 19/24] Port -net dump " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 20/24] Clean up legacy code in net_client_init() Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 21/24] Port host_net_add monitor command to QemuOpts Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 22/24] Port usb net " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 23/24] Port PCI NIC hotplug " Mark McLoughlin
2009-09-23 10:24 ` [Qemu-devel] [PATCH 24/24] Final net cleanup after conversion " Mark McLoughlin
2009-09-23 15:58 ` [Qemu-devel] [PATCH 00/19 v2] Port -net " Mark McLoughlin
2009-09-30 10:33   ` Mark McLoughlin
2009-10-06 11:16     ` [Qemu-devel] [PATCH 00/19 v3] " Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Register rtc options for -set Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for NICInfo string fields Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't assign a static string to NICInfo::model Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make NICInfo string fields non-const Mark McLoughlin
2009-10-06 19:19         ` Anthony Liguori
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Move memset() from net_client_uninit() to net_client_init() Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Use qemu_strdup() for VLANClientState string fields Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Make net_client_init() consume slirp_configs even on error Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Don't exit() in config_error() Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Drop config_error(), use qemu_error() instead Mark McLoughlin
2009-10-06 11:16       ` [Qemu-devel] [PATCH] Remove bogus error message from qemu_opts_set() Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message in qemu_option_set() Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Remove double error message for -device option parsing Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Make qemu_opts_parse() handle empty strings Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_opts_validate() for post parsing validation Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Never overwrite a QemuOpt Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Add qemu_net_opts Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net none and -net nic to QemuOpts Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net user " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net tap " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net socket " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net vde " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port -net dump " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Clean up legacy code in net_client_init() Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port host_net_add monitor command to QemuOpts Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port usb net " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Port PCI NIC hotplug " Mark McLoughlin
2009-10-06 11:17       ` [Qemu-devel] [PATCH] Final net cleanup after conversion " Mark McLoughlin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.