From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:44654) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R32oW-0003Lk-Me for qemu-devel@nongnu.org; Mon, 12 Sep 2011 05:28:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R32oV-0007VC-6O for qemu-devel@nongnu.org; Mon, 12 Sep 2011 05:28:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28474) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R32oU-0007Um-Ti for qemu-devel@nongnu.org; Mon, 12 Sep 2011 05:28:27 -0400 Date: Mon, 12 Sep 2011 10:28:15 +0100 From: "Daniel P. Berrange" Message-ID: <20110912092815.GB2523@redhat.com> References: <1314984898-19141-1-git-send-email-aliguori@us.ibm.com> <1314984898-19141-13-git-send-email-aliguori@us.ibm.com> <20110902175005.49784ffc@doriath> <20110912091721.GA2523@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20110912091721.GA2523@redhat.com> Subject: Re: [Qemu-devel] [PATCH 12/15] qapi: add change-vnc-listen (v2) Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org, Michael Roth On Mon, Sep 12, 2011 at 10:17:21AM +0100, Daniel P. Berrange wrote: > On Fri, Sep 02, 2011 at 05:50:05PM -0300, Luiz Capitulino wrote: > > On Fri, 2 Sep 2011 12:34:55 -0500 > > Anthony Liguori wrote: > > > > > New QMP only command to change the VNC server's listening address. > > > > > > Signed-off-by: Anthony Liguori > > > --- > > > v1 -> v2 > > > - Enhanced docs (Luiz) > > > --- > > > qapi-schema.json | 15 +++++++++++++++ > > > qmp-commands.hx | 8 ++++++++ > > > qmp.c | 7 +++++++ > > > 3 files changed, 30 insertions(+), 0 deletions(-) > > > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > index 350cf1c..0c6c9b8 100644 > > > --- a/qapi-schema.json > > > +++ b/qapi-schema.json > > > @@ -109,3 +109,18 @@ > > > # string. Existing clients are unaffected by executing this command. > > > ## > > > { 'command': 'change-vnc-password', 'data': {'password': 'str'} } > > > + > > > +## > > > +# @change-vnc-listen: > > > +# > > > +# Change the host that the VNC server listens on. > > > +# > > > +# @target: the new server specification to listen on > > > +# > > > +# Since: 1.0 > > > +# > > > +# Notes: At this moment in time, the behavior of existing client connections > > > +# when this command is executed is undefined. The authentication > > > +# settings may change after executing this command. > > > > It seems to completely disable authentication. At least when using > > password auth. I'd be very clear about that. > > That is really bad, since even if we have another command to set the > authentication mode, this creates a designed-in race condition. To be > securely race-free, we need to be able to set the desired auth mode > first, and then change the listen address without it affecting auth. > > change-vnc-auth tls > change-vnc-listen 123.2.3.5:5901 On closer inspection, I see that 'change-vnc-listen' just accepts the full string with encoded options, that is used for the '-vnc' command line. I thought that for QMP we going to make sure we didn't use any encoded strings, and gave each option a dedicated parameter ? eg instead of: { 'command': 'change-vnc-password', 'data': {'target': 'str'} } Wouldn't we want something like: { 'command': 'change-vnc-password', 'data': { 'listen': bool, /* Whether to listen, or do a reverse connection */ 'address': 'str', 'port': 'int', 'password': 'string', 'sasl': bool, 'tls': bool, 'x509': bool, 'lossy': bool, 'no-lock-key-sync': bool, ... } } At which point we could also make '-vnc' use qemu-config for its option parsing ? Or is your idea that we just do the more straightforward QMP command for change-vnc-listen now, with the view that everything will be changed for the future QEMU Object model rewrite ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|