Re: [Qemu-devel] [PATCH 6/9] spice: add keyboard

[Anthony Liguori <anthony@codemonkey.ws>]

On 08/19/2010 07:40 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 +-
>   qemu-spice.h  |    1 +
>   spice-input.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   spice.c       |    2 ++
>   4 files changed, 61 insertions(+), 1 deletions(-)
>   create mode 100644 spice-input.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 021067b..6ddb373 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) += spice.o
> +common-obj-$(CONFIG_SPICE) += spice.o spice-input.o
>
>   audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>   audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/qemu-spice.h b/qemu-spice.h
> index 5597576..ceb3db2 100644
> --- a/qemu-spice.h
> +++ b/qemu-spice.h
> @@ -12,6 +12,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/spice-input.c b/spice-input.c
> new file mode 100644
> index 0000000..e1014d7
> --- /dev/null
> +++ b/spice-input.c
> @@ -0,0 +1,57 @@
> +#include<stdlib.h>
> +#include<stdio.h>
> +#include<string.h>
>    

copyright and extra headers.

> +
> +#include<spice.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);
> +}
>    

Instead of using this interface which includes multi-byte sequences, it 
would probably make sense to use the same compressed encoding that VNC 
uses and introduce a new function that takes this encoding type.  The 
advantage is that one call == one keycode so it's far less prone to 
goofiness.

> +
> +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 = ledstate;
> +    spice_server_kbd_leds(&kbd->sin, ledstate);
> +}
>    

It's probably more robust in the long term to explicitly convert the 
ledstate to from the QEMU format to the libspice format even if they 
both happen to be the same.

Regards,

Anthony Liguori

> +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);
> +}
> diff --git a/spice.c b/spice.c
> index 50fa5ca..c763d52 100644
> --- a/spice.c
> +++ b/spice.c
> @@ -148,4 +148,6 @@ void qemu_spice_init(void)
>
>       spice_server_init(spice_server,&core_interface);
>       using_spice = 1;
> +
> +    qemu_spice_input_init();
>   }
>