diff mbox

[RFC,2/2] vnc: add change keyboard layout interface

Message ID 1417257582-1272-3-git-send-email-arei.gonglei@huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) Nov. 29, 2014, 10:39 a.m. UTC
From: Gonglei <arei.gonglei@huawei.com>

Example QMP command of Change VNC keyboard layout:

-> { "execute": "change",
             "arguments": { "device": "vnc", "target": "keymap",
                            "arg": "de" } }
<- { "return": {} }

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 qapi-schema.json |  8 +++++---
 qmp.c            | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

Comments

Eric Blake Dec. 1, 2014, 4:40 p.m. UTC | #1
On 11/29/2014 03:39 AM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Example QMP command of Change VNC keyboard layout:
> 
> -> { "execute": "change",
>              "arguments": { "device": "vnc", "target": "keymap",
>                             "arg": "de" } }
> <- { "return": {} }

As I said in the cover letter, we should NOT be adding stuff to the
broken 'change' command, but should instead add a new command.

> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  qapi-schema.json |  8 +++++---
>  qmp.c            | 17 +++++++++++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9ffdcf8..8c02a9f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1552,13 +1552,15 @@
>  #
>  # @target: If @device is a block device, then this is the new filename.
>  #          If @device is 'vnc', then if the value 'password' selects the vnc
> -#          change password command.   Otherwise, this specifies a new server URI
> +#          change password command, if the value 'keymap'selects the vnc change

s/'keymap'selects/'keymap' selects/

> +#          keyboard layout command. Otherwise, this specifies a new server URI
>  #          address to listen to for VNC connections.
>  #
>  # @arg:    If @device is a block device, then this is an optional format to open
>  #          the device with.
> -#          If @device is 'vnc' and @target is 'password', this is the new VNC
> -#          password to set.  If this argument is an empty string, then no future
> +#          If @device is 'vnc' and if @target is 'password', this is the new VNC
> +#          password to set; if @target is 'keymap', this is the new VNC keyboard
> +#          layout to set. If this argument is an empty string, then no future
>  #          logins will be allowed.

Not discoverable.  As proposed, libvirt has no way of knowing if qemu is
new enough to support this horrible hack.  A new command has multiple
benefits: it would be discoverable ('query-commands') and type-safe
(none of this horrid overloading of special text values).
Gonglei (Arei) Dec. 2, 2014, 1:48 a.m. UTC | #2
On 2014/12/2 0:40, Eric Blake wrote:

> On 11/29/2014 03:39 AM, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Example QMP command of Change VNC keyboard layout:
>>
>> -> { "execute": "change",
>>              "arguments": { "device": "vnc", "target": "keymap",
>>                             "arg": "de" } }
>> <- { "return": {} }
> 
> As I said in the cover letter, we should NOT be adding stuff to the
> broken 'change' command, but should instead add a new command.
> 

OK.

>>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  qapi-schema.json |  8 +++++---
>>  qmp.c            | 17 +++++++++++++++++
>>  2 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 9ffdcf8..8c02a9f 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1552,13 +1552,15 @@
>>  #
>>  # @target: If @device is a block device, then this is the new filename.
>>  #          If @device is 'vnc', then if the value 'password' selects the vnc
>> -#          change password command.   Otherwise, this specifies a new server URI
>> +#          change password command, if the value 'keymap'selects the vnc change
> 
> s/'keymap'selects/'keymap' selects/
> 
>> +#          keyboard layout command. Otherwise, this specifies a new server URI
>>  #          address to listen to for VNC connections.
>>  #
>>  # @arg:    If @device is a block device, then this is an optional format to open
>>  #          the device with.
>> -#          If @device is 'vnc' and @target is 'password', this is the new VNC
>> -#          password to set.  If this argument is an empty string, then no future
>> +#          If @device is 'vnc' and if @target is 'password', this is the new VNC
>> +#          password to set; if @target is 'keymap', this is the new VNC keyboard
>> +#          layout to set. If this argument is an empty string, then no future
>>  #          logins will be allowed.
> 
> Not discoverable.  As proposed, libvirt has no way of knowing if qemu is
> new enough to support this horrible hack.  A new command has multiple
> benefits: it would be discoverable ('query-commands') and type-safe
> (none of this horrid overloading of special text values).
> 

Great! Thank you so much for your comments, Eric.
I will add a new QMP command for this.

Regards,
-Gonglei
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 9ffdcf8..8c02a9f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1552,13 +1552,15 @@ 
 #
 # @target: If @device is a block device, then this is the new filename.
 #          If @device is 'vnc', then if the value 'password' selects the vnc
-#          change password command.   Otherwise, this specifies a new server URI
+#          change password command, if the value 'keymap'selects the vnc change
+#          keyboard layout command. Otherwise, this specifies a new server URI
 #          address to listen to for VNC connections.
 #
 # @arg:    If @device is a block device, then this is an optional format to open
 #          the device with.
-#          If @device is 'vnc' and @target is 'password', this is the new VNC
-#          password to set.  If this argument is an empty string, then no future
+#          If @device is 'vnc' and if @target is 'password', this is the new VNC
+#          password to set; if @target is 'keymap', this is the new VNC keyboard
+#          layout to set. If this argument is an empty string, then no future
 #          logins will be allowed.
 #
 # Returns: Nothing on success.
diff --git a/qmp.c b/qmp.c
index 3fda973..044cd34 100644
--- a/qmp.c
+++ b/qmp.c
@@ -366,6 +366,13 @@  void qmp_change_vnc_password(const char *password, Error **errp)
     }
 }
 
+static void qmp_change_vnc_kbd_layout(const char *kbd_layout, Error **errp)
+{
+    if (vnc_display_kbd_layout(NULL, kbd_layout) < 0) {
+        error_setg(errp, "keyboard layout '%s' set failed", kbd_layout);
+    }
+}
+
 static void qmp_change_vnc_listen(const char *target, Error **errp)
 {
     QemuOptsList *olist = qemu_find_opts("vnc");
@@ -393,6 +400,12 @@  static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
         } else {
             qmp_change_vnc_password(arg, errp);
         }
+    } else if (strcmp(target, "keymap") == 0) {
+        if (!has_arg) {
+            error_set(errp, QERR_MISSING_PARAMETER, "keymap");
+        } else {
+            qmp_change_vnc_kbd_layout(arg, errp);
+        }
     } else {
         qmp_change_vnc_listen(target, errp);
     }
@@ -402,6 +415,10 @@  void qmp_change_vnc_password(const char *password, Error **errp)
 {
     error_set(errp, QERR_FEATURE_DISABLED, "vnc");
 }
+static void qmp_change_vnc_kbd_layout(const char *kbd_layout, Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "vnc");
+}
 static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
                            Error **errp)
 {