diff mbox

[04/10] vnc: switch to QemuOpts, allow multiple servers

Message ID 1421067237-6955-5-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Jan. 12, 2015, 12:53 p.m. UTC 
This patch switches vnc over to QemuOpts, and it (more or less
as side effect) allows multiple vnc server instances.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h |   4 +-
 qmp.c                |  15 ++-
 ui/vnc.c             | 270 ++++++++++++++++++++++++++++++++-------------------
 vl.c                 |  42 +++-----
 4 files changed, 199 insertions(+), 132 deletions(-)

Comments

Markus Armbruster Feb. 13, 2015, 6:25 p.m. UTC | #1 
Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch switches vnc over to QemuOpts, and it (more or less
> as side effect) allows multiple vnc server instances.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I'm afraid this broke monitor command change vnc.

Reproducer

    Terminal 1:

        $ qemu -nodefaults -S -display vnc=:0 -monitor stdio
        QEMU 2.2.50 monitor - type 'help' for more information

    Terminal 2:

        $ vncviewer :0 

    Terminal 1:

        (qemu) change vnc :1

    Terminal 3:

        $ vncviewer :1

Before this patch, vncviewer works both times.  The second one kills the
first one.

After this patch, the first one still works, but the second one fails.
netstat shows nobody's listening on the port.

Speculation on possible cause inline.

Furthermore, the conversion to QemuOpts makes VNC visible in
-writeconfig, but if you -readconfig it back, it doesn't quite work.

With -display vnc=:0, -writeconfig produces

    [vnc]
      vnc = ":0"

If I append that to the config file I -readconfig, I a working VNC
display on :0 (good), but I also get an SDL display (not good).

> ---
>  include/ui/console.h |   4 +-
>  qmp.c                |  15 ++-
>  ui/vnc.c             | 270 ++++++++++++++++++++++++++++++++-------------------
>  vl.c                 |  42 +++-----
>  4 files changed, 199 insertions(+), 132 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 5ff2e27..887ed91 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -328,12 +328,14 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
>  
>  /* vnc.c */
>  void vnc_display_init(const char *id);
> -void vnc_display_open(const char *id, const char *display, Error **errp);
> +void vnc_display_open(const char *id, Error **errp);
>  void vnc_display_add_client(const char *id, int csock, bool skipauth);
>  char *vnc_display_local_addr(const char *id);
>  #ifdef CONFIG_VNC
>  int vnc_display_password(const char *id, const char *password);
>  int vnc_display_pw_expire(const char *id, time_t expires);
> +QemuOpts *vnc_parse_func(const char *str);
> +int vnc_init_func(QemuOpts *opts, void *opaque);
>  #else
>  static inline int vnc_display_password(const char *id, const char *password)
>  {
> diff --git a/qmp.c b/qmp.c
> index 0b4f131..963305c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -368,7 +368,20 @@ void qmp_change_vnc_password(const char *password, Error **errp)
>  
>  static void qmp_change_vnc_listen(const char *target, Error **errp)
>  {
> -    vnc_display_open(NULL, target, errp);
> +    QemuOptsList *olist = qemu_find_opts("vnc");
> +    QemuOpts *opts;
> +
> +    if (strstr(target, "id=")) {
> +        error_setg(errp, "id not supported");
> +        return;
> +    }

Aside: this is unclean.  Could we somehow test qemu_opts_id() instead?

> +

This deletes the QemuOpts with id=default:

> +    opts = qemu_opts_find(olist, "default");
> +    if (opts) {
> +        qemu_opts_del(opts);
> +    }

But this creates one without id:

> +    opts = vnc_parse_func(target);

The first argument orders vnc_display_open() to look for QemuOpts with
id=default, which doesn't exist anymore:

> +    vnc_display_open("default", errp);

>  }
>  
>  static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
[...]
Markus Armbruster Feb. 13, 2015, 6:30 p.m. UTC | #2 
One more question...

Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch switches vnc over to QemuOpts, and it (more or less
> as side effect) allows multiple vnc server instances.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/console.h |   4 +-
>  qmp.c                |  15 ++-
>  ui/vnc.c             | 270 ++++++++++++++++++++++++++++++++-------------------
>  vl.c                 |  42 +++-----
>  4 files changed, 199 insertions(+), 132 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 5ff2e27..887ed91 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -328,12 +328,14 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
>  
>  /* vnc.c */
>  void vnc_display_init(const char *id);
> -void vnc_display_open(const char *id, const char *display, Error **errp);
> +void vnc_display_open(const char *id, Error **errp);
>  void vnc_display_add_client(const char *id, int csock, bool skipauth);
>  char *vnc_display_local_addr(const char *id);
>  #ifdef CONFIG_VNC
>  int vnc_display_password(const char *id, const char *password);
>  int vnc_display_pw_expire(const char *id, time_t expires);
> +QemuOpts *vnc_parse_func(const char *str);
> +int vnc_init_func(QemuOpts *opts, void *opaque);
>  #else
>  static inline int vnc_display_password(const char *id, const char *password)
>  {
> diff --git a/qmp.c b/qmp.c
> index 0b4f131..963305c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -368,7 +368,20 @@ void qmp_change_vnc_password(const char *password, Error **errp)
>  
>  static void qmp_change_vnc_listen(const char *target, Error **errp)
>  {
> -    vnc_display_open(NULL, target, errp);
> +    QemuOptsList *olist = qemu_find_opts("vnc");
> +    QemuOpts *opts;
> +
> +    if (strstr(target, "id=")) {
> +        error_setg(errp, "id not supported");
> +        return;
> +    }
> +
> +    opts = qemu_opts_find(olist, "default");
> +    if (opts) {
> +        qemu_opts_del(opts);
> +    }
> +    opts = vnc_parse_func(target);
> +    vnc_display_open("default", errp);

vnc_parse_func() can fail.  Shouldn't we keep the old display
configuration then?

>  }
>  
[...]
Gerd Hoffmann Feb. 17, 2015, 8:50 a.m. UTC | #3 
On Fr, 2015-02-13 at 19:25 +0100, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > This patch switches vnc over to QemuOpts, and it (more or less
> > as side effect) allows multiple vnc server instances.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> I'm afraid this broke monitor command change vnc.
> 
> Reproducer
> 
>     Terminal 1:
> 
>         $ qemu -nodefaults -S -display vnc=:0 -monitor stdio
>         QEMU 2.2.50 monitor - type 'help' for more information
> 
>     Terminal 2:
> 
>         $ vncviewer :0 
> 
>     Terminal 1:
> 
>         (qemu) change vnc :1
> 
>     Terminal 3:
> 
>         $ vncviewer :1
> 
> Before this patch, vncviewer works both times.  The second one kills the
> first one.
> 
> After this patch, the first one still works, but the second one fails.
> netstat shows nobody's listening on the port.

Fix for this is on the list already (Gonglei) and included in the
pending vnc pull request.

> Furthermore, the conversion to QemuOpts makes VNC visible in
> -writeconfig, but if you -readconfig it back, it doesn't quite work.
> 
> With -display vnc=:0, -writeconfig produces
> 
>     [vnc]
>       vnc = ":0"
> 
> If I append that to the config file I -readconfig, I a working VNC
> display on :0 (good), but I also get an SDL display (not good).

Will send a fix to the list in a moment.

cheers,
  Gerd
Gerd Hoffmann Feb. 17, 2015, 8:58 a.m. UTC | #4 
Hi,

> >  static void qmp_change_vnc_listen(const char *target, Error **errp)
> >  {
> > -    vnc_display_open(NULL, target, errp);
> > +    QemuOptsList *olist = qemu_find_opts("vnc");
> > +    QemuOpts *opts;
> > +
> > +    if (strstr(target, "id=")) {
> > +        error_setg(errp, "id not supported");
> > +        return;
> > +    }
> 
> Aside: this is unclean.  Could we somehow test qemu_opts_id() instead?

For that we would have to parse it first, which has some ugly corner
cases on id clashes ...

All I wanna do here is keep it alive for the existing use cases, without
support for multiple displays, with minimum effort.

Should we need support for vnc config change in a multiple vnc server
setup a new qmp monitor command should be designed for that.

cheers,
  Gerd
Markus Armbruster Feb. 17, 2015, 12:15 p.m. UTC | #5 
Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> >  static void qmp_change_vnc_listen(const char *target, Error **errp)
>> >  {
>> > -    vnc_display_open(NULL, target, errp);
>> > +    QemuOptsList *olist = qemu_find_opts("vnc");
>> > +    QemuOpts *opts;
>> > +
>> > +    if (strstr(target, "id=")) {
>> > +        error_setg(errp, "id not supported");
>> > +        return;
>> > +    }
>> 
>> Aside: this is unclean.  Could we somehow test qemu_opts_id() instead?
>
> For that we would have to parse it first, which has some ugly corner
> cases on id clashes ...
>
> All I wanna do here is keep it alive for the existing use cases, without
> support for multiple displays, with minimum effort.
>
> Should we need support for vnc config change in a multiple vnc server
> setup a new qmp monitor command should be designed for that.

Fair enough.  Suggests that the QemuOpts API is lacking, though.
diff mbox

Patch

diff --git a/include/ui/console.h b/include/ui/console.h
index 5ff2e27..887ed91 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -328,12 +328,14 @@  void cocoa_display_init(DisplayState *ds, int full_screen);
 
 /* vnc.c */
 void vnc_display_init(const char *id);
-void vnc_display_open(const char *id, const char *display, Error **errp);
+void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 char *vnc_display_local_addr(const char *id);
 #ifdef CONFIG_VNC
 int vnc_display_password(const char *id, const char *password);
 int vnc_display_pw_expire(const char *id, time_t expires);
+QemuOpts *vnc_parse_func(const char *str);
+int vnc_init_func(QemuOpts *opts, void *opaque);
 #else
 static inline int vnc_display_password(const char *id, const char *password)
 {
diff --git a/qmp.c b/qmp.c
index 0b4f131..963305c 100644
--- a/qmp.c
+++ b/qmp.c
@@ -368,7 +368,20 @@  void qmp_change_vnc_password(const char *password, Error **errp)
 
 static void qmp_change_vnc_listen(const char *target, Error **errp)
 {
-    vnc_display_open(NULL, target, errp);
+    QemuOptsList *olist = qemu_find_opts("vnc");
+    QemuOpts *opts;
+
+    if (strstr(target, "id=")) {
+        error_setg(errp, "id not supported");
+        return;
+    }
+
+    opts = qemu_opts_find(olist, "default");
+    if (opts) {
+        qemu_opts_del(opts);
+    }
+    opts = vnc_parse_func(target);
+    vnc_display_open("default", errp);
 }
 
 static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
diff --git a/ui/vnc.c b/ui/vnc.c
index 1b86365..ce1dd59 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -31,6 +31,7 @@ 
 #include "qemu/sockets.h"
 #include "qemu/timer.h"
 #include "qemu/acl.h"
+#include "qemu/config-file.h"
 #include "qapi/qmp/types.h"
 #include "qmp-commands.h"
 #include "qemu/osdep.h"
@@ -2969,7 +2970,12 @@  static const DisplayChangeListenerOps dcl_ops = {
 
 void vnc_display_init(const char *id)
 {
-    VncDisplay *vs = g_malloc0(sizeof(*vs));
+    VncDisplay *vs;
+
+    if (vnc_display_find(id) != NULL) {
+        return;
+    }
+    vs = g_malloc0(sizeof(*vs));
 
     vs->id = strdup(id);
     QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
@@ -3065,14 +3071,65 @@  char *vnc_display_local_addr(const char *id)
     return vnc_socket_local_addr("%s:%s", vs->lsock);
 }
 
-void vnc_display_open(const char *id, const char *display, Error **errp)
+static QemuOptsList qemu_vnc_opts = {
+    .name = "vnc",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_vnc_opts.head),
+    .implied_opt_name = "vnc",
+    .desc = {
+        {
+            .name = "vnc",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "websocket",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "x509",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "share",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "password",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "reverse",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "lock-key-sync",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "sasl",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "tls",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "x509verify",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "acl",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "lossy",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "non-adaptive",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
+void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vs = vnc_display_find(id);
-    const char *options;
+    QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
+    const char *display, *websocket, *share;
     int password = 0;
     int reverse = 0;
 #ifdef CONFIG_VNC_TLS
     int tls = 0, x509 = 0;
+    const char *path;
 #endif
 #ifdef CONFIG_VNC_SASL
     int sasl = 0;
@@ -3088,115 +3145,86 @@  void vnc_display_open(const char *id, const char *display, Error **errp)
         return;
     }
     vnc_display_close(vs);
-    if (strcmp(display, "none") == 0)
-        return;
 
+    if (!opts) {
+        return;
+    }
+    display = qemu_opt_get(opts, "vnc");
+    if (!display || strcmp(display, "none") == 0) {
+        return;
+    }
     vs->display = g_strdup(display);
-    vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
-
-    options = display;
-    while ((options = strchr(options, ','))) {
-        options++;
-        if (strncmp(options, "password", 8) == 0) {
-            if (fips_get_state()) {
-                error_setg(errp,
-                           "VNC password auth disabled due to FIPS mode, "
-                           "consider using the VeNCrypt or SASL authentication "
-                           "methods as an alternative");
-                goto fail;
-            }
-            password = 1; /* Require password auth */
-        } else if (strncmp(options, "reverse", 7) == 0) {
-            reverse = 1;
-        } else if (strncmp(options, "no-lock-key-sync", 16) == 0) {
-            lock_key_sync = 0;
+
+    password = qemu_opt_get_bool(opts, "password", false);
+    if (password && fips_get_state()) {
+        error_setg(errp,
+                   "VNC password auth disabled due to FIPS mode, "
+                   "consider using the VeNCrypt or SASL authentication "
+                   "methods as an alternative");
+        goto fail;
+    }
+
+    reverse = qemu_opt_get_bool(opts, "reverse", false);
+    lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
 #ifdef CONFIG_VNC_SASL
-        } else if (strncmp(options, "sasl", 4) == 0) {
-            sasl = 1; /* Require SASL auth */
+    sasl = qemu_opt_get_bool(opts, "sasl", false);
 #endif
-#ifdef CONFIG_VNC_WS
-        } else if (strncmp(options, "websocket", 9) == 0) {
-            char *start, *end;
-            vs->websocket = 1;
-
-            /* Check for 'websocket=<port>' */
-            start = strchr(options, '=');
-            end = strchr(options, ',');
-            if (start && (!end || (start < end))) {
-                int len = end ? end-(start+1) : strlen(start+1);
-                if (len < 6) {
-                    /* extract the host specification from display */
-                    char  *host = NULL, *port = NULL, *host_end = NULL;
-                    port = g_strndup(start + 1, len);
-
-                    /* ipv6 hosts have colons */
-                    end = strchr(display, ',');
-                    host_end = g_strrstr_len(display, end - display, ":");
-
-                    if (host_end) {
-                        host = g_strndup(display, host_end - display + 1);
-                    } else {
-                        host = g_strndup(":", 1);
-                    }
-                    vs->ws_display = g_strconcat(host, port, NULL);
-                    g_free(host);
-                    g_free(port);
-                }
-            }
-#endif /* CONFIG_VNC_WS */
 #ifdef CONFIG_VNC_TLS
-        } else if (strncmp(options, "tls", 3) == 0) {
-            tls = 1; /* Require TLS */
-        } else if (strncmp(options, "x509", 4) == 0) {
-            char *start, *end;
-            x509 = 1; /* Require x509 certificates */
-            if (strncmp(options, "x509verify", 10) == 0)
-                vs->tls.x509verify = 1; /* ...and verify client certs */
-
-            /* Now check for 'x509=/some/path' postfix
-             * and use that to setup x509 certificate/key paths */
-            start = strchr(options, '=');
-            end = strchr(options, ',');
-            if (start && (!end || (start < end))) {
-                int len = end ? end-(start+1) : strlen(start+1);
-                char *path = g_strndup(start + 1, len);
-
-                VNC_DEBUG("Trying certificate path '%s'\n", path);
-                if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
-                    error_setg(errp, "Failed to find x509 certificates/keys in %s", path);
-                    g_free(path);
-                    goto fail;
-                }
-                g_free(path);
-            } else {
-                error_setg(errp, "No certificate path provided");
-                goto fail;
-            }
+    tls  = qemu_opt_get_bool(opts, "tls", false);
+    path = qemu_opt_get(opts, "x509");
+    if (path) {
+        x509 = 1;
+        vs->tls.x509verify = qemu_opt_get_bool(opts, "x509verify", false);
+        if (vnc_tls_set_x509_creds_dir(vs, path) < 0) {
+            error_setg(errp, "Failed to find x509 certificates/keys in %s",
+                       path);
+            goto fail;
+        }
+    }
 #endif
 #if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
-        } else if (strncmp(options, "acl", 3) == 0) {
-            acl = 1;
-#endif
-        } else if (strncmp(options, "lossy", 5) == 0) {
-#ifdef CONFIG_VNC_JPEG
-            vs->lossy = true;
+    acl = qemu_opt_get_bool(opts, "acl", false);
 #endif
-        } else if (strncmp(options, "non-adaptive", 12) == 0) {
-            vs->non_adaptive = true;
-        } else if (strncmp(options, "share=", 6) == 0) {
-            if (strncmp(options+6, "ignore", 6) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_IGNORE;
-            } else if (strncmp(options+6, "allow-exclusive", 15) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
-            } else if (strncmp(options+6, "force-shared", 12) == 0) {
-                vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
-            } else {
-                error_setg(errp, "unknown vnc share= option");
-                goto fail;
-            }
+
+    share = qemu_opt_get(opts, "share");
+    if (share) {
+        if (strcmp(share, "ignore") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_IGNORE;
+        } else if (strcmp(share, "allow-exclusive") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+        } else if (strcmp(share, "force-shared") == 0) {
+            vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
+        } else {
+            error_setg(errp, "unknown vnc share= option");
+            goto fail;
+        }
+    } else {
+        vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+    }
+
+ #ifdef CONFIG_VNC_WS
+    websocket = qemu_opt_get(opts, "websocket");
+    if (websocket) {
+        /* extract the host specification from display */
+        char  *host = NULL, *host_end = NULL;
+        vs->websocket = 1;
+
+        /* ipv6 hosts have colons */
+        host_end = strrchr(display, ':');
+        if (host_end) {
+            host = g_strndup(display, host_end - display + 1);
+        } else {
+            host = g_strdup(":");
         }
+        vs->ws_display = g_strconcat(host, websocket, NULL);
+        g_free(host);
     }
+#endif /* CONFIG_VNC_WS */
 
+#ifdef CONFIG_VNC_JPEG
+    vs->lossy = qemu_opt_get_bool(opts, "lossy", false);
+#endif
+    vs->non_adaptive = qemu_opt_get_bool(opts, "non-adaptive", false);
     /* adaptive updates are only used with tight encoding and
      * if lossy updates are enabled so we can disable all the
      * calculations otherwise */
@@ -3407,3 +3435,43 @@  void vnc_display_add_client(const char *id, int csock, bool skipauth)
     }
     vnc_connect(vs, csock, skipauth, false);
 }
+
+QemuOpts *vnc_parse_func(const char *str)
+{
+    return qemu_opts_parse(qemu_find_opts("vnc"), str, 1);
+}
+
+int vnc_init_func(QemuOpts *opts, void *opaque)
+{
+    Error *local_err = NULL;
+    QemuOptsList *olist = qemu_find_opts("vnc");
+    char *id = (char *)qemu_opts_id(opts);
+
+    if (!id) {
+        /* auto-assign id if not present */
+        int i = 2;
+        id = g_strdup("default");
+        while (qemu_opts_find(olist, id)) {
+            g_free(id);
+            id = g_strdup_printf("vnc%d", i++);
+        }
+        qemu_opts_set_id(opts, id);
+    }
+
+    vnc_display_init(id);
+    vnc_display_open(id, &local_err);
+    if (local_err != NULL) {
+        error_report("Failed to start VNC server on `%s': %s",
+                     qemu_opt_get(opts, "display"),
+                     error_get_pretty(local_err));
+        error_free(local_err);
+        exit(1);
+    }
+    return 0;
+}
+
+static void vnc_register_config(void)
+{
+    qemu_add_opts(&qemu_vnc_opts);
+}
+machine_init(vnc_register_config);
diff --git a/vl.c b/vl.c
index f29f04f..00ee206 100644
--- a/vl.c
+++ b/vl.c
@@ -158,9 +158,6 @@  int smp_cpus = 1;
 int max_cpus = 0;
 int smp_cores = 1;
 int smp_threads = 1;
-#ifdef CONFIG_VNC
-const char *vnc_display;
-#endif
 int acpi_enabled = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
@@ -1994,16 +1991,12 @@  static DisplayType select_display(const char *p)
 #endif
     } else if (strstart(p, "vnc", &opts)) {
 #ifdef CONFIG_VNC
-        display_remote++;
-
-        if (*opts) {
-            const char *nextopt;
-
-            if (strstart(opts, "=", &nextopt)) {
-                vnc_display = nextopt;
+        if (*opts == '=') {
+            display_remote++;
+            if (vnc_parse_func(opts+1) == NULL) {
+                exit(1);
             }
-        }
-        if (!vnc_display) {
+        } else {
             fprintf(stderr, "VNC requires a display argument vnc=<display>\n");
             exit(1);
         }
@@ -3473,7 +3466,9 @@  int main(int argc, char **argv, char **envp)
 	    case QEMU_OPTION_vnc:
 #ifdef CONFIG_VNC
                 display_remote++;
-                vnc_display = optarg;
+                if (vnc_parse_func(optarg) == NULL) {
+                    exit(1);
+                }
 #else
                 fprintf(stderr, "VNC support is disabled\n");
                 exit(1);
@@ -3960,7 +3955,7 @@  int main(int argc, char **argv, char **envp)
 #elif defined(CONFIG_SDL) || defined(CONFIG_COCOA)
         display_type = DT_SDL;
 #elif defined(CONFIG_VNC)
-        vnc_display = "localhost:0,to=99";
+        vnc_parse_func("localhost:0,to=99,id=default");
         show_vnc_port = 1;
 #else
         display_type = DT_NONE;
@@ -4274,21 +4269,10 @@  int main(int argc, char **argv, char **envp)
 
 #ifdef CONFIG_VNC
     /* init remote displays */
-    if (vnc_display) {
-        Error *local_err = NULL;
-        const char *id = "default";
-        vnc_display_init(id);
-        vnc_display_open(id, vnc_display, &local_err);
-        if (local_err != NULL) {
-            error_report("Failed to start VNC server on `%s': %s",
-                         vnc_display, error_get_pretty(local_err));
-            error_free(local_err);
-            exit(1);
-        }
-
-        if (show_vnc_port) {
-            printf("VNC server running on `%s'\n", vnc_display_local_addr(id));
-        }
+    qemu_opts_foreach(qemu_find_opts("vnc"), vnc_init_func, NULL, 0);
+    if (show_vnc_port) {
+        printf("VNC server running on `%s'\n",
+               vnc_display_local_addr("default"));
     }
 #endif
 #ifdef CONFIG_SPICE