diff mbox series

input-linux: toggle for lock keys

Message ID petKSachdmuxWMgY2EXHzES3PpUCAUwQacKEaIU9PHykNXfsah1sP3OaVuDLz0VBYJM7qtJsWSrv3U9bkb_2LT_itBxDbAYKIHlEJJXtEiE=@protonmail.com
State New
Headers show
Series input-linux: toggle for lock keys | expand

Commit Message

Cameron Esfahani via Aug. 23, 2018, 1:23 a.m. UTC 
This patch introduces three new options on the input-linux commandline:

(a) ignore_caps_lock=[on|off]
(b) ignore_num_lock=[on|off]
(c) ignore_scroll_lock=[on|off]

If enabled, the key will be disabled and not forwarded to the guest.
There are two main reasons for this:

(a) Without keyboard LEDs, it can be difficult to tell whether it is
enabled or not
(b) Preparation for another patch which will allow changing the keys
used to toggle the input device's grab. If you set the key to
KEY_SCROLLLOCK for example, it can be frustrating disabling scroll lock
in the guest, as it will require pressing the key more than twice.

I'm new to this, so if I've made a mistake, let me know ;-)

Signed-off-by: Ryan El Kochta <lightn1ngstr1ke@protonmail.com>
---
ui/input-linux.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)

--
2.18.0

Comments

Gerd Hoffmann Aug. 27, 2018, 10:16 a.m. UTC | #1 
On Thu, Aug 23, 2018 at 01:23:20AM +0000, Ryan El Kochta wrote:
> This patch introduces three new options on the input-linux commandline:
> 
> (a) ignore_caps_lock=[on|off]
> (b) ignore_num_lock=[on|off]
> (c) ignore_scroll_lock=[on|off]
> 
> If enabled, the key will be disabled and not forwarded to the guest.
> There are two main reasons for this:
> 
> (a) Without keyboard LEDs, it can be difficult to tell whether it is
> enabled or not
> (b) Preparation for another patch which will allow changing the keys
> used to toggle the input device's grab. If you set the key to
> KEY_SCROLLLOCK for example, it can be frustrating disabling scroll lock
> in the guest, as it will require pressing the key more than twice.

Hmm.  You have the same issue on the host, right?

For the guest we can easily filter out the key events.
For the host we can't.  This is the reason I picked a
key combination which has no side effects to switch
between host and guest.  And for the same reason I don't
think it is useful to allow the user to specify arbitrary
key combinations.

Which combination do you want use instead of leftshift + rightshift?

cheers,
  Gerd
Cameron Esfahani via Aug. 27, 2018, 1:49 p.m. UTC | #2 
I personally use the Scroll Lock key. On Linux, Scroll Lock (along with all the other lock keys) is easy to disable with an xmodmap command. On Windows (or, I'd assume, other guests), it requires third party software, which is why it's likely easier to just filter them out from QEMU.
-------- Original Message --------
On Aug 27, 2018, 6:16 AM, kraxel@redhat.com wrote:

> On Thu, Aug 23, 2018 at 01:23:20AM +0000, Ryan El Kochta wrote:
>> This patch introduces three new options on the input-linux commandline:
>>
>> (a) ignore_caps_lock=[on|off]
>> (b) ignore_num_lock=[on|off]
>> (c) ignore_scroll_lock=[on|off]
>>
>> If enabled, the key will be disabled and not forwarded to the guest.
>> There are two main reasons for this:
>>
>> (a) Without keyboard LEDs, it can be difficult to tell whether it is
>> enabled or not
>> (b) Preparation for another patch which will allow changing the keys
>> used to toggle the input device's grab. If you set the key to
>> KEY_SCROLLLOCK for example, it can be frustrating disabling scroll lock
>> in the guest, as it will require pressing the key more than twice.
>
> Hmm. You have the same issue on the host, right?
>
> For the guest we can easily filter out the key events.
> For the host we can't. This is the reason I picked a
> key combination which has no side effects to switch
> between host and guest. And for the same reason I don't
> think it is useful to allow the user to specify arbitrary
> key combinations.
>
> Which combination do you want use instead of leftshift + rightshift?
>
> cheers,
> Gerd
Gerd Hoffmann Aug. 30, 2018, 10:51 a.m. UTC | #3 
On Mon, Aug 27, 2018 at 01:49:22PM +0000, Ryan El Kochta wrote:

> I personally use the Scroll Lock key. On Linux, Scroll Lock (along
> with all the other lock keys) is easy to disable with an xmodmap
> command. On Windows (or, I'd assume, other guests), it requires third
> party software, which is why it's likely easier to just filter them
> out from QEMU.

Ok, disable with xmodmap is a reasonable approach for that key.
I think it would be a good idea to document that.

I'd suggest to add a single option, to configure alternative toggle
key(s), with just a few hard-coded variants which actually make sense.
Something like grab-toggle={two-shift,scroll-lock}.  Not sure whenever
we need the other lock keys.  scroll lock is probably the
least-frequently used key and therefore the best choice for the
purpose.

In case scroll-lock is selected we can automatically ignore it, without
ignore_scroll_lock=off option.

cheers,
  Gerd
Cameron Esfahani via Aug. 30, 2018, 4:35 p.m. UTC | #4 
On August 30, 2018 6:51 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mon, Aug 27, 2018 at 01:49:22PM +0000, Ryan El Kochta wrote:
>
> > I personally use the Scroll Lock key. On Linux, Scroll Lock (along
> > with all the other lock keys) is easy to disable with an xmodmap
> > command. On Windows (or, I'd assume, other guests), it requires third
> > party software, which is why it's likely easier to just filter them
> > out from QEMU.
>
> Ok, disable with xmodmap is a reasonable approach for that key.
> I think it would be a good idea to document that.
>
> I'd suggest to add a single option, to configure alternative toggle
> key(s), with just a few hard-coded variants which actually make sense.
> Something like grab-toggle={two-shift,scroll-lock}. Not sure whenever
> we need the other lock keys. scroll lock is probably the
> least-frequently used key and therefore the best choice for the
> purpose.
>
> In case scroll-lock is selected we can automatically ignore it, without
> ignore_scroll_lock=off option.
>
> cheers,
> Gerd

That's a wonderful idea. Would you be comfortable with
grab_toggle=custom:x:x as well?

Thanks,

Ryan El Kochta
Gerd Hoffmann Aug. 31, 2018, 6:07 a.m. UTC | #5 
Hi,

> That's a wonderful idea. Would you be comfortable with
> grab_toggle=custom:x:x as well?

What this is supposed to do?

cheers,
  Gerd
Cameron Esfahani via Aug. 31, 2018, 1:31 p.m. UTC | #6 
On Aug 31, 2018, 2:07 AM, Gerd Hoffmann wrote:

Hi,

> That's a wonderful idea. Would you be comfortable with
> grab_toggle=custom:x:x as well?

What this is supposed to do?

cheers,
Gerd

It would be the same as the original patch (you can input custom key IDs if you so desire) in addition to a few hardcoded sane defaults.

Thanks,

Ryan El Kochta
Gerd Hoffmann Sept. 3, 2018, 6:29 a.m. UTC | #7 
On Fri, Aug 31, 2018 at 01:31:06PM +0000, Ryan El Kochta wrote:
> On Aug 31, 2018, 2:07 AM, Gerd Hoffmann wrote:
> 
> Hi,
> 
> > That's a wonderful idea. Would you be comfortable with
> > grab_toggle=custom:x:x as well?
> 
> What this is supposed to do?
> 
> cheers,
> Gerd
> 
> It would be the same as the original patch (you can input custom key IDs if you so desire) in addition to a few hardcoded sane defaults.

What would be the use case?  Most keys have the problem that they have
some effect on the host, so using them to switch between host + guest
will have annonying side effects ...

cheers,
  Gerd
Cameron Esfahani via Sept. 3, 2018, 1:46 p.m. UTC | #8 
On September 3, 2018 2:29 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Fri, Aug 31, 2018 at 01:31:06PM +0000, Ryan El Kochta wrote:
>
> > On Aug 31, 2018, 2:07 AM, Gerd Hoffmann wrote:
> > Hi,
> >
> > > That's a wonderful idea. Would you be comfortable with
> > > grab_toggle=custom:x:x as well?
> >
> > What this is supposed to do?
> > cheers,
> > Gerd
> > It would be the same as the original patch (you can input custom key IDs if you so desire) in addition to a few hardcoded sane defaults.
>
> What would be the use case? Most keys have the problem that they have
> some effect on the host, so using them to switch between host + guest
> will have annonying side effects ...
>
> cheers,
> Gerd

You do have a point. I can really only think of about 4 or 5 key combinations that do not cause problems. I'll submit a patch later today.

Thanks,

Ryan El Kochta
diff mbox series

Patch

diff --git a/ui/input-linux.c b/ui/input-linux.c
index 9720333b2c..059d0c02a7 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -63,6 +63,10 @@  struct InputLinux {
     struct input_event event;
     int         read_offset;

+    bool        ignore_caps_lock;
+    bool        ignore_num_lock;
+    bool        ignore_scroll_lock;
+
     QTAILQ_ENTRY(InputLinux) next;
};

@@ -98,6 +102,21 @@  static void input_linux_toggle_grab(InputLinux *il)
     }
}

+static bool input_linux_ignore_event(InputLinux *il,
+                                     struct input_event *event)
+{
+    if (il->ignore_caps_lock && event->code == KEY_CAPSLOCK) {
+        return true;
+    }
+    if (il->ignore_num_lock && event->code == KEY_NUMLOCK) {
+        return true;
+    }
+    if (il->ignore_scroll_lock && event->code == KEY_SCROLLLOCK) {
+        return true;
+    }
+    return false;
+}
+
static void input_linux_handle_keyboard(InputLinux *il,
                                         struct input_event *event)
{
@@ -127,8 +146,11 @@  static void input_linux_handle_keyboard(InputLinux *il,
             il->keycount--;
         }

-        /* send event to guest when grab is active */
-        if (il->grab_active) {
+        /*
+         * send event to guest when grab is active,
+         * ignoring caps/num/scroll lock when ignore bit set
+         */
+        if (il->grab_active && !input_linux_ignore_event(il, event)) {
             int qcode = qemu_input_linux_to_qcode(event->code);
             qemu_input_event_send_key_qcode(NULL, qcode, event->value);
         }
@@ -410,6 +432,51 @@  static void input_linux_set_repeat(Object *obj, bool value,
     il->repeat = value;
}

+static bool input_linux_get_ignore_caps_lock(Object *obj, Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    return il->ignore_caps_lock;
+}
+
+static void input_linux_set_ignore_caps_lock(Object *obj, bool value,
+                                             Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    il->ignore_caps_lock = value;
+}
+
+static bool input_linux_get_ignore_num_lock(Object *obj, Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    return il->ignore_num_lock;
+}
+
+static void input_linux_set_ignore_num_lock(Object *obj, bool value,
+                                            Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    il->ignore_num_lock = value;
+}
+
+static bool input_linux_get_ignore_scroll_lock(Object *obj, Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    return il->ignore_scroll_lock;
+}
+
+static void input_linux_set_ignore_scroll_lock(Object *obj, bool value,
+                                               Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+
+    il->ignore_scroll_lock = value;
+}
+
static void input_linux_instance_init(Object *obj)
{
     object_property_add_str(obj, "evdev",
@@ -421,6 +488,15 @@  static void input_linux_instance_init(Object *obj)
     object_property_add_bool(obj, "repeat",
                              input_linux_get_repeat,
                              input_linux_set_repeat, NULL);
+    object_property_add_bool(obj, "ignore_caps_lock",
+                             input_linux_get_ignore_caps_lock,
+                             input_linux_set_ignore_caps_lock, NULL);
+    object_property_add_bool(obj, "ignore_num_lock",
+                             input_linux_get_ignore_num_lock,
+                             input_linux_set_ignore_num_lock, NULL);
+    object_property_add_bool(obj, "ignore_scroll_lock",
+                             input_linux_get_ignore_scroll_lock,
+                             input_linux_set_ignore_scroll_lock, NULL);
}

static void input_linux_class_init(ObjectClass *oc, void *data)