| Message ID | 1314984898-19141-13-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 2 Sep 2011 12:34:55 -0500 Anthony Liguori <aliguori@us.ibm.com> wrote: > New QMP only command to change the VNC server's listening address. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > 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. > +## > +{ 'command': 'change-vnc-listen', 'data': {'target': 'str'} } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 7df3938..5cab212 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -876,6 +876,14 @@ EQMP > }, > > { > + .name = "change-vnc-listen", > + .args_type = "target:s", > + .params = "target", > + .help = "set vnc listening address", > + .mhandler.cmd_new = qmp_marshal_input_change_vnc_listen, > + }, > + > + { > .name = "set_password", > .args_type = "protocol:s,password:s,connected:s?", > .params = "protocol password action-if-connected", > diff --git a/qmp.c b/qmp.c > index f817a88..73d6172 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -35,3 +35,10 @@ void qmp_change_vnc_password(const char *password, Error **err) > error_set(err, QERR_SET_PASSWD_FAILED); > } > } > + > +void qmp_change_vnc_listen(const char *target, Error **err) > +{ > + if (vnc_display_open(NULL, target) < 0) { > + error_set(err, QERR_VNC_SERVER_FAILED, target); > + } > +}
On Fri, Sep 02, 2011 at 05:50:05PM -0300, Luiz Capitulino wrote: > On Fri, 2 Sep 2011 12:34:55 -0500 > Anthony Liguori <aliguori@us.ibm.com> wrote: > > > New QMP only command to change the VNC server's listening address. > > > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > --- > > 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 If we really want vnc-listen to have possible side-effects on auth, then we need to be able to put the VNC server in an offline mode while making a sequence of configuration changes eg, something like change-vnc-status offline change-vnc-listen 123.2.3.5:5901 change-vnc-auth tls change-vnc-status online No incoming client connections would be allowed while it is offline Regards, Daniel
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 <aliguori@us.ibm.com> wrote: > > > > > New QMP only command to change the VNC server's listening address. > > > > > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > > > --- > > > 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
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. +## +{ 'command': 'change-vnc-listen', 'data': {'target': 'str'} } diff --git a/qmp-commands.hx b/qmp-commands.hx index 7df3938..5cab212 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -876,6 +876,14 @@ EQMP }, { + .name = "change-vnc-listen", + .args_type = "target:s", + .params = "target", + .help = "set vnc listening address", + .mhandler.cmd_new = qmp_marshal_input_change_vnc_listen, + }, + + { .name = "set_password", .args_type = "protocol:s,password:s,connected:s?", .params = "protocol password action-if-connected", diff --git a/qmp.c b/qmp.c index f817a88..73d6172 100644 --- a/qmp.c +++ b/qmp.c @@ -35,3 +35,10 @@ void qmp_change_vnc_password(const char *password, Error **err) error_set(err, QERR_SET_PASSWD_FAILED); } } + +void qmp_change_vnc_listen(const char *target, Error **err) +{ + if (vnc_display_open(NULL, target) < 0) { + error_set(err, QERR_VNC_SERVER_FAILED, target); + } +}
New QMP only command to change the VNC server's listening address. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- v1 -> v2 - Enhanced docs (Luiz) --- qapi-schema.json | 15 +++++++++++++++ qmp-commands.hx | 8 ++++++++ qmp.c | 7 +++++++ 3 files changed, 30 insertions(+), 0 deletions(-)