[PATCH] input-linux: Add option to not grab a device upon guest startup

[PATCH] input-linux: Add option to not grab a device upon guest startup

[Justinien Bouron]

Depending on your use-case, it might be inconvenient to have qemu grab
the input device immediately upon starting the guest, especially if the
guest takes a while to start in which case it may take a few seconds
before being able to release the device via the toggle combination.

Added a new bool option to input-linux: grab-on-startup. If true, the
device is grabbed as soon as the guest is started, otherwise it is not
grabbed until the toggle combination is entered. To avoid breaking
existing setups, the default value of grab-on-startup is true, ie. same
behaviour as before this change.

Signed-off-by: Justinien Bouron <justinien.bouron@gmail.com>
---
 qapi/qom.json    | 13 ++++++++++++-
 ui/input-linux.c | 20 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index 032c6fa037..50e66d55cc 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -508,13 +508,24 @@
 # @grab-toggle: the key or key combination that toggles device grab
 #     (default: ctrl-ctrl)
 #
+# @grab-on-startup: if true, grab the device immediately upon starting the
+#     guest. Otherwise, don't grab the device until the combination is entered.
+#     This does not influence other devices even if grab_all is true, ie. in the
+#     unlikely scenario where device1 has grab_all=true + grab-on-startup=true
+#     and device2 has grab-on-startup=false, only device1 is grabbed on startup,
+#     then, once the grab combination is entered, grabbing is toggled off for
+#     both devices (because device1 enforces the grab_all property) until the
+#     combination is entered again at which point both devices will be grabbed.
+#     (default: true).
+
 # Since: 2.6
 ##
 { 'struct': 'InputLinuxProperties',
   'data': { 'evdev': 'str',
             '*grab_all': 'bool',
             '*repeat': 'bool',
-            '*grab-toggle': 'GrabToggleKeys' } }
+            '*grab-toggle': 'GrabToggleKeys',
+            '*grab-on-startup': 'bool'} }
 
 ##
 # @EventLoopBaseProperties:
diff --git a/ui/input-linux.c b/ui/input-linux.c
index e572a2e905..cf9376ddb0 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -44,6 +44,7 @@ struct InputLinux {
     bool        grab_request;
     bool        grab_active;
     bool        grab_all;
+    bool        grab_on_startup;
     bool        keydown[KEY_CNT];
     int         keycount;
     int         wheel;
@@ -400,7 +401,7 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
     if (il->keycount) {
         /* delay grab until all keys are released */
         il->grab_request = true;
-    } else {
+    } else if (il->grab_on_startup) {
         input_linux_toggle_grab(il);
     }
     QTAILQ_INSERT_TAIL(&inputs, il, next);
@@ -491,6 +492,19 @@ static void input_linux_set_grab_toggle(Object *obj, int value,
     il->grab_toggle = value;
 }
 
+static bool input_linux_get_grab_on_startup(Object *obj, Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+    return il->grab_on_startup;
+}
+
+static void input_linux_set_grab_on_startup(Object *obj, bool value,
+                                            Error **errp)
+{
+    InputLinux *il = INPUT_LINUX(obj);
+    il->grab_on_startup = value;
+}
+
 static void input_linux_instance_init(Object *obj)
 {
 }
@@ -498,6 +512,7 @@ static void input_linux_instance_init(Object *obj)
 static void input_linux_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+    ObjectProperty *grab_on_startup_prop;
 
     ucc->complete = input_linux_complete;
 
@@ -514,6 +529,9 @@ static void input_linux_class_init(ObjectClass *oc, void *data)
                                    &GrabToggleKeys_lookup,
                                    input_linux_get_grab_toggle,
                                    input_linux_set_grab_toggle);
+    grab_on_startup_prop = object_class_property_add_bool(oc, "grab-on-startup",
+        input_linux_get_grab_on_startup, input_linux_set_grab_on_startup);
+    object_property_set_default_bool(grab_on_startup_prop, true);
 }
 
 static const TypeInfo input_linux_info = {
-- 
2.43.0

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Daniel P. Berrangé]

On Wed, Mar 06, 2024 at 10:28:22PM -0800, Justinien Bouron wrote:
> Depending on your use-case, it might be inconvenient to have qemu grab
> the input device immediately upon starting the guest, especially if the
> guest takes a while to start in which case it may take a few seconds
> before being able to release the device via the toggle combination.

This last two lines doesn't make sense to me. Isn't the grab
toggling entirely in control of the QEMU process, regardless
of what state the guest is at ?

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Justinien Bouron]

> This last two lines doesn't make sense to me. Isn't the grab
> toggling entirely in control of the QEMU process, regardless
> of what state the guest is at ?

Actually, you're right, they do not make sense. This issue of having the guest
taking a while to start and the toggle keys not working, only seem to appear
when running the VM under libvirt. I was not able to reproduce this issue when
running qemu directly from the command line. So either this is a libvirt issue
or something related to my setup (VFIO with GPU passthrough, so a lot can go
wrong).

Should I send a new version of the patch with an updated commit message that
does not mention this issue?

Regards,
Justinien

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Daniel P. Berrangé]

On Thu, Mar 07, 2024 at 07:38:27PM -0800, Justinien Bouron wrote:
> > This last two lines doesn't make sense to me. Isn't the grab
> > toggling entirely in control of the QEMU process, regardless
> > of what state the guest is at ?
> 
> Actually, you're right, they do not make sense. This issue of having the guest
> taking a while to start and the toggle keys not working, only seem to appear
> when running the VM under libvirt. I was not able to reproduce this issue when
> running qemu directly from the command line. So either this is a libvirt issue
> or something related to my setup (VFIO with GPU passthrough, so a lot can go
> wrong).
> 
> Should I send a new version of the patch with an updated commit message that
> does not mention this issue?

If that probem does not exist, what is the compelling reason to
add this patch ?

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Justinien Bouron]

> > > This last two lines doesn't make sense to me. Isn't the grab
> > > toggling entirely in control of the QEMU process, regardless
> > > of what state the guest is at ?
> >
> > Actually, you're right, they do not make sense. This issue of having the guest
> > taking a while to start and the toggle keys not working, only seem to appear
> > when running the VM under libvirt. I was not able to reproduce this issue when
> > running qemu directly from the command line. So either this is a libvirt issue
> > or something related to my setup (VFIO with GPU passthrough, so a lot can go
> > wrong).
> >
> > Should I send a new version of the patch with an updated commit message that
> > does not mention this issue?
> 
> If that probem does not exist, what is the compelling reason to
> add this patch ?

I still think this patch is useful. Having the guest "steal" the kb/mouse from
the host immediately upon starting is frankly annoying and most of the time
unnecessary as at this point the guest is barely booting anyway. Most of the
time I don't need/want to redirect the kb/mouse until the guest is fully booted
and instead want to do something on the host while the guest is starting in the
background. So I end up having to press the toggle keys every time after
starting the guest just so I can keep control of my inputs. This might seem like
a minor annoyance but it is still that, an annoyance.

This can get particularly annoying if the guest is started from a script. In
this situation the guest may not necessarily be started immediately upon running
the script depending on what the script is doing prior to the `virsh start`
command. So you can't tell exactly when your kb/mouse are going to be grabbed
from you. If you do something else on your host in the meantime, while the
script is running in the background, you end up having your inputs grabbed from
you without notice.

There are a few posts[1][2] on the internet asking how to prevent evdev grabbing
upon boot, so it seems that I am not the only one that would like to see such an
option.

[1] https://www.reddit.com/r/VFIO/comments/14xuksq/evedv_passthough_dont_grab_on_start/
[2] https://www.reddit.com/r/VFIO/comments/frbk0q/disabling_auto_keyboard_grab_evdev/

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Justinien Bouron]

Just a ping to make sure this patch hasn't been lost in the noise.
Any chance to get this merged? Should I send a v2 with a revised commit message?

Regards,
Justinien

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Marc-André Lureau]

Hi Justinien

On Fri, Mar 15, 2024 at 6:37 AM Justinien Bouron
<justinien.bouron@gmail.com> wrote:
>
> Just a ping to make sure this patch hasn't been lost in the noise.
> Any chance to get this merged? Should I send a v2 with a revised commit message?
>

It's too late for 9.0. Please send a v2 with updated commit message.

thanks

Re: [PATCH] input-linux: Add option to not grab a device upon guest startup

[Markus Armbruster]

Justinien Bouron <justinien.bouron@gmail.com> writes:

> Depending on your use-case, it might be inconvenient to have qemu grab
> the input device immediately upon starting the guest, especially if the
> guest takes a while to start in which case it may take a few seconds
> before being able to release the device via the toggle combination.
>
> Added a new bool option to input-linux: grab-on-startup. If true, the
> device is grabbed as soon as the guest is started, otherwise it is not
> grabbed until the toggle combination is entered. To avoid breaking
> existing setups, the default value of grab-on-startup is true, ie. same
> behaviour as before this change.
>
> Signed-off-by: Justinien Bouron <justinien.bouron@gmail.com>
> ---
>  qapi/qom.json    | 13 ++++++++++++-
>  ui/input-linux.c | 20 +++++++++++++++++++-
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 032c6fa037..50e66d55cc 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -508,13 +508,24 @@
>  # @grab-toggle: the key or key combination that toggles device grab
>  #     (default: ctrl-ctrl)
>  #
> +# @grab-on-startup: if true, grab the device immediately upon starting the
> +#     guest. Otherwise, don't grab the device until the combination is entered.
> +#     This does not influence other devices even if grab_all is true, ie. in the

i.e.

> +#     unlikely scenario where device1 has grab_all=true + grab-on-startup=true
> +#     and device2 has grab-on-startup=false, only device1 is grabbed on startup,
> +#     then, once the grab combination is entered, grabbing is toggled off for
> +#     both devices (because device1 enforces the grab_all property) until the
> +#     combination is entered again at which point both devices will be grabbed.
> +#     (default: true).

From docs/devel/qapi-code-gen.rst section Documentation markup:

    For legibility, wrap text paragraphs so every line is at most 70
    characters long.

    Separate sentences with two spaces.

> +

No blank lines in the middle of comment blocks, please.

>  # Since: 2.6
>  ##

Together:

   # @grab-on-startup: if true, grab the device immediately upon starting
   #     the guest.  Otherwise, don't grab the device until the
   #     combination is entered.  This does not influence other devices
   #     even if grab_all is true, i.e. in the unlikely scenario where
   #     device1 has grab_all=true + grab-on-startup=true and device2 has
   #     grab-on-startup=false, only device1 is grabbed on startup, then,
   #     once the grab combination is entered, grabbing is toggled off
   #     for both devices (because device1 enforces the grab_all
   #     property) until the combination is entered again at which point
   #     both devices will be grabbed.  (default: true).
   #
   # Since: 2.6
   ##

>  { 'struct': 'InputLinuxProperties',
>    'data': { 'evdev': 'str',
>              '*grab_all': 'bool',
>              '*repeat': 'bool',
> -            '*grab-toggle': 'GrabToggleKeys' } }
> +            '*grab-toggle': 'GrabToggleKeys',
> +            '*grab-on-startup': 'bool'} }
>  
>  ##
>  # @EventLoopBaseProperties:

[...]