* [Qemu-devel] [PATCH 1/3] vnc: drop display+ws_display from VncDisplay
2015-03-10 11:18 [Qemu-devel] [PATCH 0/3] vnc: some code cleanups Gerd Hoffmann
@ 2015-03-10 11:18 ` Gerd Hoffmann
2015-03-11 1:58 ` Gonglei
2015-03-10 11:18 ` [Qemu-devel] [PATCH 2/3] vnc: remove dead code Gerd Hoffmann
2015-03-10 11:18 ` [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts Gerd Hoffmann
2 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2015-03-10 11:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori
Nobody cares about those strings, they are only used to check whenever
the vnc server / websocket support is enabled or not. Add bools for
this and drop the strings.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/vnc-auth-sasl.c | 2 +-
ui/vnc.c | 54 +++++++++++++++++++++++-------------------------------
ui/vnc.h | 6 +++---
3 files changed, 27 insertions(+), 35 deletions(-)
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index f3ad75d..2ddd259 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -555,7 +555,7 @@ void start_auth_sasl(VncState *vs)
memset (&secprops, 0, sizeof secprops);
/* Inform SASL that we've got an external SSF layer from TLS */
- if (strncmp(vs->vd->display, "unix:", 5) == 0
+ if (vs->vd->is_unix
#ifdef CONFIG_VNC_TLS
/* Disable SSF, if using TLS+x509+SASL only. TLS without x509
is not sufficiently strong */
diff --git a/ui/vnc.c b/ui/vnc.c
index ff0b5bd..65ba1c0 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -407,7 +407,7 @@ VncInfo *qmp_query_vnc(Error **errp)
VncInfo *info = g_malloc0(sizeof(*info));
VncDisplay *vd = vnc_display_find(NULL);
- if (vd == NULL || vd->display == NULL) {
+ if (vd == NULL || !vd->enabled) {
info->enabled = false;
} else {
struct sockaddr_storage sa;
@@ -3190,16 +3190,15 @@ static void vnc_display_close(VncDisplay *vs)
{
if (!vs)
return;
- g_free(vs->display);
- vs->display = NULL;
+ vs->enabled = false;
+ vs->is_unix = false;
if (vs->lsock != -1) {
qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);
close(vs->lsock);
vs->lsock = -1;
}
#ifdef CONFIG_VNC_WS
- g_free(vs->ws_display);
- vs->ws_display = NULL;
+ vs->ws_enabled = false;
if (vs->lwebsock != -1) {
qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
close(vs->lwebsock);
@@ -3329,7 +3328,7 @@ void vnc_display_open(const char *id, Error **errp)
bool reverse = false;
const char *vnc;
const char *has_to;
- char *display, *to = NULL;
+ char *display, *ws_display = NULL, *to = NULL;
bool has_ipv4 = false;
bool has_ipv6 = false;
#ifdef CONFIG_VNC_WS
@@ -3369,10 +3368,9 @@ void vnc_display_open(const char *id, Error **errp)
has_ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
has_ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
display = g_strdup_printf("%s%s%s%s", vnc,
- has_to ? to : "",
- has_ipv4 ? ",ipv4" : "",
- has_ipv6 ? ",ipv6" : "");
- vs->display = g_strdup(display);
+ has_to ? to : "",
+ has_ipv4 ? ",ipv4" : "",
+ has_ipv6 ? ",ipv6" : "");
password = qemu_opt_get_bool(opts, "password", false);
if (password && fips_get_state()) {
@@ -3427,7 +3425,7 @@ void vnc_display_open(const char *id, Error **errp)
if (websocket) {
/* extract the host specification from display */
char *host = NULL, *host_end = NULL;
- vs->websocket = 1;
+ vs->ws_enabled = true;
/* ipv6 hosts have colons */
host_end = strrchr(display, ':');
@@ -3436,7 +3434,7 @@ void vnc_display_open(const char *id, Error **errp)
} else {
host = g_strdup(":");
}
- vs->ws_display = g_strconcat(host, websocket, NULL);
+ ws_display = g_strconcat(host, websocket, NULL);
g_free(host);
}
#endif /* CONFIG_VNC_WS */
@@ -3618,34 +3616,29 @@ void vnc_display_open(const char *id, Error **errp)
vnc_connect(vs, csock, false, false);
} else {
/* listen for connects */
- char *dpy;
- dpy = g_malloc(256);
if (strncmp(display, "unix:", 5) == 0) {
- pstrcpy(dpy, 256, "unix:");
- vs->lsock = unix_listen(display+5, dpy+5, 256-5, errp);
+ vs->lsock = unix_listen(display+5, NULL, 0, errp);
+ vs->is_unix = true;
} else {
- vs->lsock = inet_listen(display, dpy, 256,
+ vs->lsock = inet_listen(display, NULL, 0,
SOCK_STREAM, 5900, errp);
if (vs->lsock < 0) {
- g_free(dpy);
goto fail;
}
#ifdef CONFIG_VNC_WS
- if (vs->websocket) {
- if (vs->ws_display) {
- vs->lwebsock = inet_listen(vs->ws_display, NULL, 256,
+ if (vs->ws_enabled) {
+ if (ws_display) {
+ vs->lwebsock = inet_listen(ws_display, NULL, 0,
SOCK_STREAM, 0, errp);
} else {
- vs->lwebsock = inet_listen(vs->display, NULL, 256,
+ vs->lwebsock = inet_listen(display, NULL, 0,
SOCK_STREAM, 5700, errp);
}
-
if (vs->lwebsock < 0) {
if (vs->lsock) {
close(vs->lsock);
vs->lsock = -1;
}
- g_free(dpy);
goto fail;
}
}
@@ -3653,12 +3646,12 @@ void vnc_display_open(const char *id, Error **errp)
}
g_free(to);
g_free(display);
- g_free(vs->display);
- vs->display = dpy;
+ vs->enabled = true;
qemu_set_fd_handler2(vs->lsock, NULL,
vnc_listen_regular_read, NULL, vs);
#ifdef CONFIG_VNC_WS
- if (vs->websocket) {
+ g_free(ws_display);
+ if (vs->ws_enabled) {
qemu_set_fd_handler2(vs->lwebsock, NULL,
vnc_listen_websocket_read, NULL, vs);
}
@@ -3669,11 +3662,10 @@ void vnc_display_open(const char *id, Error **errp)
fail:
g_free(to);
g_free(display);
- g_free(vs->display);
- vs->display = NULL;
+ vs->enabled = false;
#ifdef CONFIG_VNC_WS
- g_free(vs->ws_display);
- vs->ws_display = NULL;
+ g_free(ws_display);
+ vs->ws_enabled = false;
#endif /* CONFIG_VNC_WS */
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 5e2b1a5..66a0298 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -158,8 +158,7 @@ struct VncDisplay
int lsock;
#ifdef CONFIG_VNC_WS
int lwebsock;
- bool websocket;
- char *ws_display;
+ bool ws_enabled;
#endif
DisplaySurface *ds;
DisplayChangeListener dcl;
@@ -176,7 +175,8 @@ struct VncDisplay
const char *id;
QTAILQ_ENTRY(VncDisplay) next;
- char *display;
+ bool enabled;
+ bool is_unix;
char *password;
time_t expires;
int auth;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] vnc: drop display+ws_display from VncDisplay
2015-03-10 11:18 ` [Qemu-devel] [PATCH 1/3] vnc: drop display+ws_display from VncDisplay Gerd Hoffmann
@ 2015-03-11 1:58 ` Gonglei
0 siblings, 0 replies; 10+ messages in thread
From: Gonglei @ 2015-03-11 1:58 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 2015/3/10 19:18, Gerd Hoffmann wrote:
> Nobody cares about those strings, they are only used to check whenever
> the vnc server / websocket support is enabled or not. Add bools for
> this and drop the strings.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> ui/vnc-auth-sasl.c | 2 +-
> ui/vnc.c | 54 +++++++++++++++++++++++-------------------------------
> ui/vnc.h | 6 +++---
> 3 files changed, 27 insertions(+), 35 deletions(-)
>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index f3ad75d..2ddd259 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -555,7 +555,7 @@ void start_auth_sasl(VncState *vs)
>
> memset (&secprops, 0, sizeof secprops);
> /* Inform SASL that we've got an external SSF layer from TLS */
> - if (strncmp(vs->vd->display, "unix:", 5) == 0
> + if (vs->vd->is_unix
> #ifdef CONFIG_VNC_TLS
> /* Disable SSF, if using TLS+x509+SASL only. TLS without x509
> is not sufficiently strong */
> diff --git a/ui/vnc.c b/ui/vnc.c
> index ff0b5bd..65ba1c0 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -407,7 +407,7 @@ VncInfo *qmp_query_vnc(Error **errp)
> VncInfo *info = g_malloc0(sizeof(*info));
> VncDisplay *vd = vnc_display_find(NULL);
>
> - if (vd == NULL || vd->display == NULL) {
> + if (vd == NULL || !vd->enabled) {
> info->enabled = false;
> } else {
> struct sockaddr_storage sa;
> @@ -3190,16 +3190,15 @@ static void vnc_display_close(VncDisplay *vs)
> {
> if (!vs)
> return;
> - g_free(vs->display);
> - vs->display = NULL;
> + vs->enabled = false;
> + vs->is_unix = false;
> if (vs->lsock != -1) {
> qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);
> close(vs->lsock);
> vs->lsock = -1;
> }
> #ifdef CONFIG_VNC_WS
> - g_free(vs->ws_display);
> - vs->ws_display = NULL;
> + vs->ws_enabled = false;
> if (vs->lwebsock != -1) {
> qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
> close(vs->lwebsock);
> @@ -3329,7 +3328,7 @@ void vnc_display_open(const char *id, Error **errp)
> bool reverse = false;
> const char *vnc;
> const char *has_to;
> - char *display, *to = NULL;
> + char *display, *ws_display = NULL, *to = NULL;
> bool has_ipv4 = false;
> bool has_ipv6 = false;
> #ifdef CONFIG_VNC_WS
> @@ -3369,10 +3368,9 @@ void vnc_display_open(const char *id, Error **errp)
> has_ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
> has_ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
> display = g_strdup_printf("%s%s%s%s", vnc,
> - has_to ? to : "",
> - has_ipv4 ? ",ipv4" : "",
> - has_ipv6 ? ",ipv6" : "");
> - vs->display = g_strdup(display);
> + has_to ? to : "",
> + has_ipv4 ? ",ipv4" : "",
> + has_ipv6 ? ",ipv6" : "");
>
> password = qemu_opt_get_bool(opts, "password", false);
> if (password && fips_get_state()) {
> @@ -3427,7 +3425,7 @@ void vnc_display_open(const char *id, Error **errp)
> if (websocket) {
> /* extract the host specification from display */
> char *host = NULL, *host_end = NULL;
> - vs->websocket = 1;
> + vs->ws_enabled = true;
>
> /* ipv6 hosts have colons */
> host_end = strrchr(display, ':');
> @@ -3436,7 +3434,7 @@ void vnc_display_open(const char *id, Error **errp)
> } else {
> host = g_strdup(":");
> }
> - vs->ws_display = g_strconcat(host, websocket, NULL);
> + ws_display = g_strconcat(host, websocket, NULL);
> g_free(host);
> }
> #endif /* CONFIG_VNC_WS */
> @@ -3618,34 +3616,29 @@ void vnc_display_open(const char *id, Error **errp)
> vnc_connect(vs, csock, false, false);
> } else {
> /* listen for connects */
> - char *dpy;
> - dpy = g_malloc(256);
> if (strncmp(display, "unix:", 5) == 0) {
> - pstrcpy(dpy, 256, "unix:");
> - vs->lsock = unix_listen(display+5, dpy+5, 256-5, errp);
> + vs->lsock = unix_listen(display+5, NULL, 0, errp);
> + vs->is_unix = true;
> } else {
> - vs->lsock = inet_listen(display, dpy, 256,
> + vs->lsock = inet_listen(display, NULL, 0,
> SOCK_STREAM, 5900, errp);
> if (vs->lsock < 0) {
> - g_free(dpy);
> goto fail;
> }
> #ifdef CONFIG_VNC_WS
> - if (vs->websocket) {
> - if (vs->ws_display) {
> - vs->lwebsock = inet_listen(vs->ws_display, NULL, 256,
> + if (vs->ws_enabled) {
> + if (ws_display) {
> + vs->lwebsock = inet_listen(ws_display, NULL, 0,
> SOCK_STREAM, 0, errp);
> } else {
> - vs->lwebsock = inet_listen(vs->display, NULL, 256,
> + vs->lwebsock = inet_listen(display, NULL, 0,
> SOCK_STREAM, 5700, errp);
> }
> -
> if (vs->lwebsock < 0) {
> if (vs->lsock) {
> close(vs->lsock);
> vs->lsock = -1;
> }
> - g_free(dpy);
> goto fail;
> }
> }
> @@ -3653,12 +3646,12 @@ void vnc_display_open(const char *id, Error **errp)
> }
> g_free(to);
> g_free(display);
> - g_free(vs->display);
> - vs->display = dpy;
> + vs->enabled = true;
> qemu_set_fd_handler2(vs->lsock, NULL,
> vnc_listen_regular_read, NULL, vs);
> #ifdef CONFIG_VNC_WS
> - if (vs->websocket) {
> + g_free(ws_display);
> + if (vs->ws_enabled) {
> qemu_set_fd_handler2(vs->lwebsock, NULL,
> vnc_listen_websocket_read, NULL, vs);
> }
> @@ -3669,11 +3662,10 @@ void vnc_display_open(const char *id, Error **errp)
> fail:
> g_free(to);
> g_free(display);
> - g_free(vs->display);
> - vs->display = NULL;
> + vs->enabled = false;
> #ifdef CONFIG_VNC_WS
> - g_free(vs->ws_display);
> - vs->ws_display = NULL;
> + g_free(ws_display);
> + vs->ws_enabled = false;
> #endif /* CONFIG_VNC_WS */
> }
>
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 5e2b1a5..66a0298 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -158,8 +158,7 @@ struct VncDisplay
> int lsock;
> #ifdef CONFIG_VNC_WS
> int lwebsock;
> - bool websocket;
> - char *ws_display;
> + bool ws_enabled;
> #endif
> DisplaySurface *ds;
> DisplayChangeListener dcl;
> @@ -176,7 +175,8 @@ struct VncDisplay
>
> const char *id;
> QTAILQ_ENTRY(VncDisplay) next;
> - char *display;
> + bool enabled;
> + bool is_unix;
> char *password;
> time_t expires;
> int auth;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/3] vnc: remove dead code
2015-03-10 11:18 [Qemu-devel] [PATCH 0/3] vnc: some code cleanups Gerd Hoffmann
2015-03-10 11:18 ` [Qemu-devel] [PATCH 1/3] vnc: drop display+ws_display from VncDisplay Gerd Hoffmann
@ 2015-03-10 11:18 ` Gerd Hoffmann
2015-03-11 1:59 ` Gonglei
2015-03-10 11:18 ` [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts Gerd Hoffmann
2 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2015-03-10 11:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori
If vs->ws_enabled is set ws_display is non-NULL.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/vnc.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 65ba1c0..bac44ce 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3627,13 +3627,8 @@ void vnc_display_open(const char *id, Error **errp)
}
#ifdef CONFIG_VNC_WS
if (vs->ws_enabled) {
- if (ws_display) {
- vs->lwebsock = inet_listen(ws_display, NULL, 0,
- SOCK_STREAM, 0, errp);
- } else {
- vs->lwebsock = inet_listen(display, NULL, 0,
- SOCK_STREAM, 5700, errp);
- }
+ vs->lwebsock = inet_listen(ws_display, NULL, 0,
+ SOCK_STREAM, 0, errp);
if (vs->lwebsock < 0) {
if (vs->lsock) {
close(vs->lsock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] vnc: remove dead code
2015-03-10 11:18 ` [Qemu-devel] [PATCH 2/3] vnc: remove dead code Gerd Hoffmann
@ 2015-03-11 1:59 ` Gonglei
0 siblings, 0 replies; 10+ messages in thread
From: Gonglei @ 2015-03-11 1:59 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 2015/3/10 19:18, Gerd Hoffmann wrote:
> If vs->ws_enabled is set ws_display is non-NULL.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> ui/vnc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 65ba1c0..bac44ce 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3627,13 +3627,8 @@ void vnc_display_open(const char *id, Error **errp)
> }
> #ifdef CONFIG_VNC_WS
> if (vs->ws_enabled) {
> - if (ws_display) {
> - vs->lwebsock = inet_listen(ws_display, NULL, 0,
> - SOCK_STREAM, 0, errp);
> - } else {
> - vs->lwebsock = inet_listen(display, NULL, 0,
> - SOCK_STREAM, 5700, errp);
> - }
> + vs->lwebsock = inet_listen(ws_display, NULL, 0,
> + SOCK_STREAM, 0, errp);
> if (vs->lwebsock < 0) {
> if (vs->lsock) {
> close(vs->lsock);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts
2015-03-10 11:18 [Qemu-devel] [PATCH 0/3] vnc: some code cleanups Gerd Hoffmann
2015-03-10 11:18 ` [Qemu-devel] [PATCH 1/3] vnc: drop display+ws_display from VncDisplay Gerd Hoffmann
2015-03-10 11:18 ` [Qemu-devel] [PATCH 2/3] vnc: remove dead code Gerd Hoffmann
@ 2015-03-10 11:18 ` Gerd Hoffmann
2015-03-10 15:50 ` Paolo Bonzini
2015-03-11 2:08 ` Gonglei
2 siblings, 2 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2015-03-10 11:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori
Use inet_listen_opts instead of inet_listen. Allows us to drop some
pointless indirection: Format strings just to parse them again later on.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/vnc.c | 75 +++++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 41 insertions(+), 34 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index bac44ce..2510c15 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3322,13 +3322,14 @@ void vnc_display_open(const char *id, Error **errp)
{
VncDisplay *vs = vnc_display_find(id);
QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
+ QemuOpts *sopts, *wsopts;
const char *share, *device_id;
QemuConsole *con;
bool password = false;
bool reverse = false;
const char *vnc;
const char *has_to;
- char *display, *ws_display = NULL, *to = NULL;
+ char *h;
bool has_ipv4 = false;
bool has_ipv6 = false;
#ifdef CONFIG_VNC_WS
@@ -3361,16 +3362,36 @@ void vnc_display_open(const char *id, Error **errp)
return;
}
- has_to = qemu_opt_get(opts, "to");
- if (has_to) {
- to = g_strdup_printf(",to=%s", has_to);
+ sopts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
+ wsopts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
+
+ h = strrchr(vnc, ':');
+ if (h) {
+ char *host = g_strndup(vnc, h - vnc);
+ qemu_opt_set(sopts, "host", host);
+ qemu_opt_set(wsopts, "host", host);
+ qemu_opt_set(sopts, "port", h+1);
+ g_free(host);
+ } else {
+ error_setg(errp, "no vnc port specified");
+ goto fail;
}
+
+ has_to = qemu_opt_get(opts, "to");
has_ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
has_ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
- display = g_strdup_printf("%s%s%s%s", vnc,
- has_to ? to : "",
- has_ipv4 ? ",ipv4" : "",
- has_ipv6 ? ",ipv6" : "");
+ if (has_to) {
+ qemu_opt_set(sopts, "to", has_to);
+ qemu_opt_set(wsopts, "to", has_to);
+ }
+ if (has_ipv4) {
+ qemu_opt_set(sopts, "ipv4", "on");
+ qemu_opt_set(wsopts, "ipv4", "on");
+ }
+ if (has_ipv6) {
+ qemu_opt_set(sopts, "ipv6", "on");
+ qemu_opt_set(wsopts, "ipv6", "on");
+ }
password = qemu_opt_get_bool(opts, "password", false);
if (password && fips_get_state()) {
@@ -3423,19 +3444,9 @@ void vnc_display_open(const char *id, Error **errp)
#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->ws_enabled = true;
+ qemu_opt_set(wsopts, "port", websocket);
- /* ipv6 hosts have colons */
- host_end = strrchr(display, ':');
- if (host_end) {
- host = g_strndup(display, host_end - display + 1);
- } else {
- host = g_strdup(":");
- }
- ws_display = g_strconcat(host, websocket, NULL);
- g_free(host);
}
#endif /* CONFIG_VNC_WS */
@@ -3605,10 +3616,10 @@ void vnc_display_open(const char *id, Error **errp)
#ifdef CONFIG_VNC_WS
vs->lwebsock = -1;
#endif
- if (strncmp(display, "unix:", 5) == 0) {
- csock = unix_connect(display+5, errp);
+ if (strncmp(vnc, "unix:", 5) == 0) {
+ csock = unix_connect(vnc+5, errp);
} else {
- csock = inet_connect(display, errp);
+ csock = inet_connect(vnc, errp);
}
if (csock < 0) {
goto fail;
@@ -3616,19 +3627,17 @@ void vnc_display_open(const char *id, Error **errp)
vnc_connect(vs, csock, false, false);
} else {
/* listen for connects */
- if (strncmp(display, "unix:", 5) == 0) {
- vs->lsock = unix_listen(display+5, NULL, 0, errp);
+ if (strncmp(vnc, "unix:", 5) == 0) {
+ vs->lsock = unix_listen(vnc+5, NULL, 0, errp);
vs->is_unix = true;
} else {
- vs->lsock = inet_listen(display, NULL, 0,
- SOCK_STREAM, 5900, errp);
+ vs->lsock = inet_listen_opts(sopts, 5900, errp);
if (vs->lsock < 0) {
goto fail;
}
#ifdef CONFIG_VNC_WS
if (vs->ws_enabled) {
- vs->lwebsock = inet_listen(ws_display, NULL, 0,
- SOCK_STREAM, 0, errp);
+ vs->lwebsock = inet_listen_opts(wsopts, 0, errp);
if (vs->lwebsock < 0) {
if (vs->lsock) {
close(vs->lsock);
@@ -3639,27 +3648,25 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif /* CONFIG_VNC_WS */
}
- g_free(to);
- g_free(display);
vs->enabled = true;
qemu_set_fd_handler2(vs->lsock, NULL,
vnc_listen_regular_read, NULL, vs);
#ifdef CONFIG_VNC_WS
- g_free(ws_display);
if (vs->ws_enabled) {
qemu_set_fd_handler2(vs->lwebsock, NULL,
vnc_listen_websocket_read, NULL, vs);
}
#endif /* CONFIG_VNC_WS */
}
+ qemu_opts_del(sopts);
+ qemu_opts_del(wsopts);
return;
fail:
- g_free(to);
- g_free(display);
+ qemu_opts_del(sopts);
+ qemu_opts_del(wsopts);
vs->enabled = false;
#ifdef CONFIG_VNC_WS
- g_free(ws_display);
vs->ws_enabled = false;
#endif /* CONFIG_VNC_WS */
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts
2015-03-10 11:18 ` [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts Gerd Hoffmann
@ 2015-03-10 15:50 ` Paolo Bonzini
2015-03-11 13:18 ` Gerd Hoffmann
2015-03-11 2:08 ` Gonglei
1 sibling, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2015-03-10 15:50 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 10/03/2015 12:18, Gerd Hoffmann wrote:
> Use inet_listen_opts instead of inet_listen. Allows us to drop some
> pointless indirection: Format strings just to parse them again later on.
Maybe use socket_listen then?
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts
2015-03-10 15:50 ` Paolo Bonzini
@ 2015-03-11 13:18 ` Gerd Hoffmann
0 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2015-03-11 13:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Anthony Liguori
On Di, 2015-03-10 at 16:50 +0100, Paolo Bonzini wrote:
>
> On 10/03/2015 12:18, Gerd Hoffmann wrote:
> > Use inet_listen_opts instead of inet_listen. Allows us to drop some
> > pointless indirection: Format strings just to parse them again later on.
>
> Maybe use socket_listen then?
Hmm, mapping "vnc qemuopts" -> "socket qemuopts" looks a bit easier than
mapping "vnc qemuopts" -> SocketAddress, which will then be translated
into "socket qemuopts" anyway ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts
2015-03-10 11:18 ` [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts Gerd Hoffmann
2015-03-10 15:50 ` Paolo Bonzini
@ 2015-03-11 2:08 ` Gonglei
2015-03-11 13:20 ` Gerd Hoffmann
1 sibling, 1 reply; 10+ messages in thread
From: Gonglei @ 2015-03-11 2:08 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: Anthony Liguori
On 2015/3/10 19:18, Gerd Hoffmann wrote:
> Use inet_listen_opts instead of inet_listen. Allows us to drop some
> pointless indirection: Format strings just to parse them again later on.
>
Nice improvement. :)
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> ui/vnc.c | 75 +++++++++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 41 insertions(+), 34 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index bac44ce..2510c15 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3322,13 +3322,14 @@ void vnc_display_open(const char *id, Error **errp)
> {
> VncDisplay *vs = vnc_display_find(id);
> QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
> + QemuOpts *sopts, *wsopts;
> const char *share, *device_id;
> QemuConsole *con;
> bool password = false;
> bool reverse = false;
> const char *vnc;
> const char *has_to;
> - char *display, *ws_display = NULL, *to = NULL;
> + char *h;
> bool has_ipv4 = false;
> bool has_ipv6 = false;
> #ifdef CONFIG_VNC_WS
> @@ -3361,16 +3362,36 @@ void vnc_display_open(const char *id, Error **errp)
> return;
> }
>
> - has_to = qemu_opt_get(opts, "to");
> - if (has_to) {
> - to = g_strdup_printf(",to=%s", has_to);
> + sopts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
> + wsopts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
> +
> + h = strrchr(vnc, ':');
> + if (h) {
> + char *host = g_strndup(vnc, h - vnc);
> + qemu_opt_set(sopts, "host", host);
> + qemu_opt_set(wsopts, "host", host);
> + qemu_opt_set(sopts, "port", h+1);
Maybe you missed passing '&error_abort' or something like that:
ui/vnc.c:3371: error: too few arguments to function ‘qemu_opt_set’
ui/vnc.c:3372: error: too few arguments to function ‘qemu_opt_set’
ui/vnc.c:3373: error: too few arguments to function ‘qemu_opt_set’
ui/vnc.c:3384: error: too few arguments to function ‘qemu_opt_set’
as the same as below several places of calling qemu_opt_set().
Regards,
-Gonglei
> + g_free(host);
> + } else {
> + error_setg(errp, "no vnc port specified");
> + goto fail;
> }
> +
> + has_to = qemu_opt_get(opts, "to");
> has_ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
> has_ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
> - display = g_strdup_printf("%s%s%s%s", vnc,
> - has_to ? to : "",
> - has_ipv4 ? ",ipv4" : "",
> - has_ipv6 ? ",ipv6" : "");
> + if (has_to) {
> + qemu_opt_set(sopts, "to", has_to);
> + qemu_opt_set(wsopts, "to", has_to);
> + }
> + if (has_ipv4) {
> + qemu_opt_set(sopts, "ipv4", "on");
> + qemu_opt_set(wsopts, "ipv4", "on");
> + }
> + if (has_ipv6) {
> + qemu_opt_set(sopts, "ipv6", "on");
> + qemu_opt_set(wsopts, "ipv6", "on");
> + }
>
> password = qemu_opt_get_bool(opts, "password", false);
> if (password && fips_get_state()) {
> @@ -3423,19 +3444,9 @@ void vnc_display_open(const char *id, Error **errp)
> #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->ws_enabled = true;
> + qemu_opt_set(wsopts, "port", websocket);
>
> - /* ipv6 hosts have colons */
> - host_end = strrchr(display, ':');
> - if (host_end) {
> - host = g_strndup(display, host_end - display + 1);
> - } else {
> - host = g_strdup(":");
> - }
> - ws_display = g_strconcat(host, websocket, NULL);
> - g_free(host);
> }
> #endif /* CONFIG_VNC_WS */
>
> @@ -3605,10 +3616,10 @@ void vnc_display_open(const char *id, Error **errp)
> #ifdef CONFIG_VNC_WS
> vs->lwebsock = -1;
> #endif
> - if (strncmp(display, "unix:", 5) == 0) {
> - csock = unix_connect(display+5, errp);
> + if (strncmp(vnc, "unix:", 5) == 0) {
> + csock = unix_connect(vnc+5, errp);
> } else {
> - csock = inet_connect(display, errp);
> + csock = inet_connect(vnc, errp);
> }
> if (csock < 0) {
> goto fail;
> @@ -3616,19 +3627,17 @@ void vnc_display_open(const char *id, Error **errp)
> vnc_connect(vs, csock, false, false);
> } else {
> /* listen for connects */
> - if (strncmp(display, "unix:", 5) == 0) {
> - vs->lsock = unix_listen(display+5, NULL, 0, errp);
> + if (strncmp(vnc, "unix:", 5) == 0) {
> + vs->lsock = unix_listen(vnc+5, NULL, 0, errp);
> vs->is_unix = true;
> } else {
> - vs->lsock = inet_listen(display, NULL, 0,
> - SOCK_STREAM, 5900, errp);
> + vs->lsock = inet_listen_opts(sopts, 5900, errp);
> if (vs->lsock < 0) {
> goto fail;
> }
> #ifdef CONFIG_VNC_WS
> if (vs->ws_enabled) {
> - vs->lwebsock = inet_listen(ws_display, NULL, 0,
> - SOCK_STREAM, 0, errp);
> + vs->lwebsock = inet_listen_opts(wsopts, 0, errp);
> if (vs->lwebsock < 0) {
> if (vs->lsock) {
> close(vs->lsock);
> @@ -3639,27 +3648,25 @@ void vnc_display_open(const char *id, Error **errp)
> }
> #endif /* CONFIG_VNC_WS */
> }
> - g_free(to);
> - g_free(display);
> vs->enabled = true;
> qemu_set_fd_handler2(vs->lsock, NULL,
> vnc_listen_regular_read, NULL, vs);
> #ifdef CONFIG_VNC_WS
> - g_free(ws_display);
> if (vs->ws_enabled) {
> qemu_set_fd_handler2(vs->lwebsock, NULL,
> vnc_listen_websocket_read, NULL, vs);
> }
> #endif /* CONFIG_VNC_WS */
> }
> + qemu_opts_del(sopts);
> + qemu_opts_del(wsopts);
> return;
>
> fail:
> - g_free(to);
> - g_free(display);
> + qemu_opts_del(sopts);
> + qemu_opts_del(wsopts);
> vs->enabled = false;
> #ifdef CONFIG_VNC_WS
> - g_free(ws_display);
> vs->ws_enabled = false;
> #endif /* CONFIG_VNC_WS */
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] vnc: switch to inet_listen_opts
2015-03-11 2:08 ` Gonglei
@ 2015-03-11 13:20 ` Gerd Hoffmann
0 siblings, 0 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2015-03-11 13:20 UTC (permalink / raw)
To: Gonglei; +Cc: qemu-devel, Anthony Liguori
Hi,
> > + qemu_opt_set(sopts, "port", h+1);
>
> Maybe you missed passing '&error_abort' or something like that:
>
> ui/vnc.c:3371: error: too few arguments to function ‘qemu_opt_set’
> ui/vnc.c:3372: error: too few arguments to function ‘qemu_opt_set’
> ui/vnc.c:3373: error: too few arguments to function ‘qemu_opt_set’
> ui/vnc.c:3384: error: too few arguments to function ‘qemu_opt_set’
>
> as the same as below several places of calling qemu_opt_set().
Looks like a a error handling series from markus was merged meanwhile,
need to rebase & adapt patch.
cheers,
Gerd
^ permalink raw reply [flat|nested] 10+ messages in thread