On 2014/12/16 21:20, Gerd Hoffmann wrote:
> 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 <kra...@redhat.com>
> ---
> include/ui/console.h | 4 +-
> qmp.c | 15 ++-
> ui/vnc.c | 271
> ++++++++++++++++++++++++++++++++-------------------
> vl.c | 42 +++-----
> 4 files changed, 200 insertions(+), 132 deletions(-)
>
Hi, Gerd
I'm afraid you forgot to update this patch following comments of version 2. :)
Regards,
-Gonglei
> 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..3fda973 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_init_func(opts, NULL);
> }
>
> 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..cf8bed8 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,44 @@ 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);
> + }
> + fprintf(stderr, "%s: id \"%s\"\n", __func__, 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 afb6212..a7d4a57 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;
> @@ -2103,16 +2100,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);
> }
> @@ -3585,7 +3578,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);
> @@ -4069,7 +4064,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;
> @@ -4374,21 +4369,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
