[10/14] qapi: add change-vnc-password

Submitted by Anthony Liguori on Aug. 24, 2011, 6:43 p.m.

Details

Message ID 1314211389-28915-11-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Aug. 24, 2011, 6:43 p.m.
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(-)

Comments

Gerd Hoffmann Aug. 25, 2011, 9:07 a.m.
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
Anthony Liguori Aug. 25, 2011, 1:12 p.m.
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
>
>
Luiz Capitulino Aug. 25, 2011, 1:33 p.m.
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);
> +    }
> +}
Anthony Liguori Sept. 2, 2011, 4:08 p.m.
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);
>> +    }
>> +}
>
>

Patch hide | download patch | download mbox

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);
+    }
+}