| Message ID | 1314211389-28915-11-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 08/24/11 20:43, Anthony Liguori wrote:
> This is a new QMP only command that only changes the VNC password.
What is wrong with "set_password vnc $secret" ?
cheers,
Gerd
On 08/25/2011 04:07 AM, Gerd Hoffmann wrote: > On 08/24/11 20:43, Anthony Liguori wrote: >> This is a new QMP only command that only changes the VNC password. > > What is wrong with "set_password vnc $secret" ? Overloading a single function to do multiple things makes the function hard to use. Consider the C interface: void qmp_set_password(QmpSession *sess, const char *protocol, const char *password, bool has_connected, const char *connected) Since both protocol and connected are strings, you can get hard to debug errors by using the string "VNC" instead of "vnc" or "Keep" instead of "keep". There's no obvious way to figure out what strings are valid for protocol programatically so if we added rdesktop or something like that, the interface would become awkward to use. Likewise, it's non-intuitive that connected only has one valid value for VNC which means that for changing the VNC password, it's just extra typing. Compare that to: void qmp_change_vnc_password(QmpSession *sess, const char *password) It's impossible to use wrong. If we add a qmp_change_rdesktop_password, it's easily detectable. Regards, Anthony Liguori > > cheers, > Gerd > >
On Wed, 24 Aug 2011 13:43:05 -0500 Anthony Liguori <aliguori@us.ibm.com> wrote: > This is a new QMP only command that only changes the VNC password. Isn't this useful in HMP too? > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > qapi-schema.json | 13 +++++++++++++ > qmp-commands.hx | 8 ++++++++ > qmp.c | 11 ++++++++++- > 3 files changed, 31 insertions(+), 1 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index f159d81..3b2229f 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -96,3 +96,16 @@ > { 'command': 'set-blockdev-password', > 'data': {'device': 'str', 'password': 'str'} } > > +## > +# @change-vnc-password: > +# > +# Change the VNC server password. > +# > +# @target: the new password to use with VNC authentication The argument name is "password". > +# > +# Since: 1.0 > +# > +# Notes: An empty password in this command will set the password to the empty > +# string. Existing clients are unaffected by executing this command. > +## > +{ 'command': 'change-vnc-password', 'data': {'password': 'str'} } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 909c778..d60f72f 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -868,6 +868,14 @@ Example: > EQMP > > { > + .name = "change-vnc-password", > + .args_type = "password:s", > + .params = "password", > + .help = "set vnc password", > + .mhandler.cmd_new = qmp_marshal_input_change_vnc_password, > + }, > + > + { > .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 8aa9c66..f817a88 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -12,9 +12,11 @@ > */ > > #include "qemu-common.h" > -#include "sysemu.h" > #include "qmp-commands.h" > > +#include "sysemu.h" > +#include "console.h" > + > NameInfo *qmp_query_name(Error **errp) > { > NameInfo *info = g_malloc0(sizeof(*info)); > @@ -26,3 +28,10 @@ NameInfo *qmp_query_name(Error **errp) > > return info; > } > + > +void qmp_change_vnc_password(const char *password, Error **err) > +{ > + if (vnc_display_password(NULL, password) < 0) { > + error_set(err, QERR_SET_PASSWD_FAILED); > + } > +}
On 08/25/2011 08:33 AM, Luiz Capitulino wrote: > On Wed, 24 Aug 2011 13:43:05 -0500 > Anthony Liguori<aliguori@us.ibm.com> wrote: > >> This is a new QMP only command that only changes the VNC password. > > Isn't this useful in HMP too? I think the right design goal for HMP is to minimize the number of commands since a human can only remember so much. > >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> qapi-schema.json | 13 +++++++++++++ >> qmp-commands.hx | 8 ++++++++ >> qmp.c | 11 ++++++++++- >> 3 files changed, 31 insertions(+), 1 deletions(-) >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index f159d81..3b2229f 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -96,3 +96,16 @@ >> { 'command': 'set-blockdev-password', >> 'data': {'device': 'str', 'password': 'str'} } >> >> +## >> +# @change-vnc-password: >> +# >> +# Change the VNC server password. >> +# >> +# @target: the new password to use with VNC authentication > > The argument name is "password". Ack. Regards, Anthony Liguori > >> +# >> +# Since: 1.0 >> +# >> +# Notes: An empty password in this command will set the password to the empty >> +# string. Existing clients are unaffected by executing this command. >> +## >> +{ 'command': 'change-vnc-password', 'data': {'password': 'str'} } >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index 909c778..d60f72f 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -868,6 +868,14 @@ Example: >> EQMP >> >> { >> + .name = "change-vnc-password", >> + .args_type = "password:s", >> + .params = "password", >> + .help = "set vnc password", >> + .mhandler.cmd_new = qmp_marshal_input_change_vnc_password, >> + }, >> + >> + { >> .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 8aa9c66..f817a88 100644 >> --- a/qmp.c >> +++ b/qmp.c >> @@ -12,9 +12,11 @@ >> */ >> >> #include "qemu-common.h" >> -#include "sysemu.h" >> #include "qmp-commands.h" >> >> +#include "sysemu.h" >> +#include "console.h" >> + >> NameInfo *qmp_query_name(Error **errp) >> { >> NameInfo *info = g_malloc0(sizeof(*info)); >> @@ -26,3 +28,10 @@ NameInfo *qmp_query_name(Error **errp) >> >> return info; >> } >> + >> +void qmp_change_vnc_password(const char *password, Error **err) >> +{ >> + if (vnc_display_password(NULL, password)< 0) { >> + error_set(err, QERR_SET_PASSWD_FAILED); >> + } >> +} > >
diff --git a/qapi-schema.json b/qapi-schema.json index f159d81..3b2229f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -96,3 +96,16 @@ { 'command': 'set-blockdev-password', 'data': {'device': 'str', 'password': 'str'} } +## +# @change-vnc-password: +# +# Change the VNC server password. +# +# @target: the new password to use with VNC authentication +# +# Since: 1.0 +# +# Notes: An empty password in this command will set the password to the empty +# string. Existing clients are unaffected by executing this command. +## +{ 'command': 'change-vnc-password', 'data': {'password': 'str'} } diff --git a/qmp-commands.hx b/qmp-commands.hx index 909c778..d60f72f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -868,6 +868,14 @@ Example: EQMP { + .name = "change-vnc-password", + .args_type = "password:s", + .params = "password", + .help = "set vnc password", + .mhandler.cmd_new = qmp_marshal_input_change_vnc_password, + }, + + { .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 8aa9c66..f817a88 100644 --- a/qmp.c +++ b/qmp.c @@ -12,9 +12,11 @@ */ #include "qemu-common.h" -#include "sysemu.h" #include "qmp-commands.h" +#include "sysemu.h" +#include "console.h" + NameInfo *qmp_query_name(Error **errp) { NameInfo *info = g_malloc0(sizeof(*info)); @@ -26,3 +28,10 @@ NameInfo *qmp_query_name(Error **errp) return info; } + +void qmp_change_vnc_password(const char *password, Error **err) +{ + if (vnc_display_password(NULL, password) < 0) { + error_set(err, QERR_SET_PASSWD_FAILED); + } +}
This is a new QMP only command that only changes the VNC password. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- qapi-schema.json | 13 +++++++++++++ qmp-commands.hx | 8 ++++++++ qmp.c | 11 ++++++++++- 3 files changed, 31 insertions(+), 1 deletions(-)