From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMKvf-0002OS-SK for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:25:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMKva-00057X-D0 for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:25:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48593) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMKva-00057O-4o for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:25:22 -0500 From: Markus Armbruster References: <1421067237-6955-1-git-send-email-kraxel@redhat.com> <1421067237-6955-5-git-send-email-kraxel@redhat.com> Date: Fri, 13 Feb 2015 19:25:16 +0100 In-Reply-To: <1421067237-6955-5-git-send-email-kraxel@redhat.com> (Gerd Hoffmann's message of "Mon, 12 Jan 2015 13:53:51 +0100") Message-ID: <87k2zly8hf.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 04/10] vnc: switch to QemuOpts, allow multiple servers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Paolo Bonzini , qemu-devel@nongnu.org, Anthony Liguori , Luiz Capitulino Gerd Hoffmann 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 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, [...]