Patchwork [v3,07/10] spice: add keyboard

login
register
mail settings
Submitter Gerd Hoffmann
Date Aug. 25, 2010, 2:19 p.m.
Message ID <1282745970-11506-8-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/62684/
State New
Headers show

Comments

Gerd Hoffmann - Aug. 25, 2010, 2:19 p.m.
Open keyboard channel.  Now you can type into the spice client and the
keyboard events are sent to your guest.  You'll need some other display
like vnc to actually see the guest responding to them though.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.objs    |    2 +-
 ui/qemu-spice.h  |    1 +
 ui/spice-core.c  |    2 +
 ui/spice-input.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 1 deletions(-)
 create mode 100644 ui/spice-input.c
Anthony Liguori - Aug. 25, 2010, 7:53 p.m.
On 08/25/2010 09:19 AM, Gerd Hoffmann wrote:
> Open keyboard channel.  Now you can type into the spice client and the
> keyboard events are sent to your guest.  You'll need some other display
> like vnc to actually see the guest responding to them though.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>   Makefile.objs    |    2 +-
>   ui/qemu-spice.h  |    1 +
>   ui/spice-core.c  |    2 +
>   ui/spice-input.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 86 insertions(+), 1 deletions(-)
>   create mode 100644 ui/spice-input.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 1a53027..20e6ad7 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -88,7 +88,7 @@ common-obj-y += pflib.o
>   common-obj-$(CONFIG_BRLAPI) += baum.o
>   common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>
> -common-obj-$(CONFIG_SPICE) += ui/spice-core.o
> +common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o
>
>   audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>   audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
> index d58d74c..75e2502 100644
> --- a/ui/qemu-spice.h
> +++ b/ui/qemu-spice.h
> @@ -29,6 +29,7 @@ extern SpiceServer *spice_server;
>   extern int using_spice;
>
>   void qemu_spice_init(void);
> +void qemu_spice_input_init(void);
>
>   #else  /* CONFIG_SPICE */
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 3041309..75f0fe9 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -161,6 +161,8 @@ void qemu_spice_init(void)
>
>       spice_server_init(spice_server,&core_interface);
>       using_spice = 1;
> +
> +    qemu_spice_input_init();
>    

Why not pass spice_server to qemu_spice_input_init() and avoid the global?

Regards,

Anthony Liguori

>   }
>
>   static void spice_register_config(void)
> diff --git a/ui/spice-input.c b/ui/spice-input.c
> new file mode 100644
> index 0000000..d4f44f4
> --- /dev/null
> +++ b/ui/spice-input.c
> @@ -0,0 +1,82 @@
> +/*
> + * Copyright (C) 2010 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +#include<stdlib.h>
> +#include<stdio.h>
> +#include<string.h>
> +
> +#include<spice.h>
> +#include<spice/enums.h>
> +
> +#include "qemu-common.h"
> +#include "qemu-spice.h"
> +#include "console.h"
> +
> +/* keyboard bits */
> +
> +typedef struct QemuSpiceKbd {
> +    SpiceKbdInstance sin;
> +    int ledstate;
> +} QemuSpiceKbd;
> +
> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag);
> +static uint8_t kbd_get_leds(SpiceKbdInstance *sin);
> +static void kbd_leds(void *opaque, int l);
> +
> +static const SpiceKbdInterface kbd_interface = {
> +    .base.type          = SPICE_INTERFACE_KEYBOARD,
> +    .base.description   = "qemu keyboard",
> +    .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
> +    .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
> +    .push_scan_freg     = kbd_push_key,
> +    .get_leds           = kbd_get_leds,
> +};
> +
> +static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
> +{
> +    kbd_put_keycode(frag);
> +}
> +
> +static uint8_t kbd_get_leds(SpiceKbdInstance *sin)
> +{
> +    QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
> +    return kbd->ledstate;
> +}
> +
> +static void kbd_leds(void *opaque, int ledstate)
> +{
> +    QemuSpiceKbd *kbd = opaque;
> +
> +    kbd->ledstate = 0;
> +    if (ledstate&  QEMU_SCROLL_LOCK_LED)
> +        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_SCROLL_LOCK;
> +    if (ledstate&  QEMU_NUM_LOCK_LED)
> +        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_NUM_LOCK;
> +    if (ledstate&  QEMU_CAPS_LOCK_LED)
> +        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_CAPS_LOCK;
> +    spice_server_kbd_leds(&kbd->sin, ledstate);
> +}
> +
> +void qemu_spice_input_init(void)
> +{
> +    QemuSpiceKbd *kbd;
> +
> +    kbd = qemu_mallocz(sizeof(*kbd));
> +    kbd->sin.base.sif =&kbd_interface.base;
> +    spice_server_add_interface(spice_server,&kbd->sin.base);
> +    qemu_add_led_event_handler(kbd_leds, kbd);
> +}
>
Gerd Hoffmann - Aug. 26, 2010, 6:55 a.m.
Hi,

>> +
>> + qemu_spice_input_init();
>
> Why not pass spice_server to qemu_spice_input_init() and avoid the global?
>

We need the global anyway for the guest devices (not yet in this 
series).  They are created using -device.  Thus the initialization 
functions are not called from spice code and we can't simply pass a 
reference to spice_server.

cheers,
   Gerd
Anthony Liguori - Aug. 26, 2010, 12:55 p.m.
On 08/26/2010 01:55 AM, Gerd Hoffmann wrote:
>   Hi,
>
>>> +
>>> + qemu_spice_input_init();
>>
>> Why not pass spice_server to qemu_spice_input_init() and avoid the 
>> global?
>>
>
> We need the global anyway for the guest devices (not yet in this 
> series).  They are created using -device.  Thus the initialization 
> functions are not called from spice code and we can't simply pass a 
> reference to spice_server.

Let's avoid it where we can and we'll do what we need to do for the 
guest devices.

In general, for the cases that we truly need a global, I'd prefer 
getting to it via a function instead of having a non-static variable.

Regards,

Anthony Liguori

>
> cheers,
>   Gerd

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 1a53027..20e6ad7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -88,7 +88,7 @@  common-obj-y += pflib.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
 
-common-obj-$(CONFIG_SPICE) += ui/spice-core.o
+common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o
 
 audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
 audio-obj-$(CONFIG_SDL) += sdlaudio.o
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index d58d74c..75e2502 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -29,6 +29,7 @@  extern SpiceServer *spice_server;
 extern int using_spice;
 
 void qemu_spice_init(void);
+void qemu_spice_input_init(void);
 
 #else  /* CONFIG_SPICE */
 
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3041309..75f0fe9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -161,6 +161,8 @@  void qemu_spice_init(void)
 
     spice_server_init(spice_server, &core_interface);
     using_spice = 1;
+
+    qemu_spice_input_init();
 }
 
 static void spice_register_config(void)
diff --git a/ui/spice-input.c b/ui/spice-input.c
new file mode 100644
index 0000000..d4f44f4
--- /dev/null
+++ b/ui/spice-input.c
@@ -0,0 +1,82 @@ 
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <spice.h>
+#include <spice/enums.h>
+
+#include "qemu-common.h"
+#include "qemu-spice.h"
+#include "console.h"
+
+/* keyboard bits */
+
+typedef struct QemuSpiceKbd {
+    SpiceKbdInstance sin;
+    int ledstate;
+} QemuSpiceKbd;
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag);
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin);
+static void kbd_leds(void *opaque, int l);
+
+static const SpiceKbdInterface kbd_interface = {
+    .base.type          = SPICE_INTERFACE_KEYBOARD,
+    .base.description   = "qemu keyboard",
+    .base.major_version = SPICE_INTERFACE_KEYBOARD_MAJOR,
+    .base.minor_version = SPICE_INTERFACE_KEYBOARD_MINOR,
+    .push_scan_freg     = kbd_push_key,
+    .get_leds           = kbd_get_leds,
+};
+
+static void kbd_push_key(SpiceKbdInstance *sin, uint8_t frag)
+{
+    kbd_put_keycode(frag);
+}
+
+static uint8_t kbd_get_leds(SpiceKbdInstance *sin)
+{
+    QemuSpiceKbd *kbd = container_of(sin, QemuSpiceKbd, sin);
+    return kbd->ledstate;
+}
+
+static void kbd_leds(void *opaque, int ledstate)
+{
+    QemuSpiceKbd *kbd = opaque;
+
+    kbd->ledstate = 0;
+    if (ledstate & QEMU_SCROLL_LOCK_LED)
+        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_SCROLL_LOCK;
+    if (ledstate & QEMU_NUM_LOCK_LED)
+        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_NUM_LOCK;
+    if (ledstate & QEMU_CAPS_LOCK_LED)
+        kbd->ledstate |= SPICE_KEYBOARD_MODIFIER_FLAGS_CAPS_LOCK;
+    spice_server_kbd_leds(&kbd->sin, ledstate);
+}
+
+void qemu_spice_input_init(void)
+{
+    QemuSpiceKbd *kbd;
+
+    kbd = qemu_mallocz(sizeof(*kbd));
+    kbd->sin.base.sif = &kbd_interface.base;
+    spice_server_add_interface(spice_server, &kbd->sin.base);
+    qemu_add_led_event_handler(kbd_leds, kbd);
+}