On 06/24/2011 09:18 PM, Daniel P. Berrange wrote:
> On Fri, Jun 24, 2011 at 02:33:29PM +0800, Lai Jiangshan wrote:
>> Add virtkey lib for usage-improvment and keycode translating.
>> Add 4 internal API for the aim
>>
>> const char *virKeycodeSetName(virKeycodeSet codeset);
>> virKeycodeSet virParseKeycodeSet(const char *name);
>
> These should just be done using the standard VIR_ENUM_DECL/IMPL macros.
>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index 3f634e6..2f2efe7 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -1815,6 +1815,12 @@ typedef enum {
>> VIR_KEYCODE_SET_ATSET1 = 2,
>> VIR_KEYCODE_SET_ATSET2 = 3,
>> VIR_KEYCODE_SET_ATSET3 = 4,
>> + VIR_KEYCODE_SET_OSX = 5,
>> + VIR_KEYCODE_SET_XT_KBD = 6,
>> + VIR_KEYCODE_SET_USB = 7,
>> + VIR_KEYCODE_SET_WIN32 = 8,
>> + VIR_KEYCODE_SET_XWIN_XT = 9,
>> + VIR_KEYCODE_SET_XFREE86_KBD_XT = 10,
>> } virKeycodeSet;
>
> IMHO, we don't really need to include the XT_KBD, XWIN_XT or
> XFREE86_KBD_XT codesets, since these are all special purpose
> sets which are just derived from the based XT set. Lets just
> stick to the core interesting sets. So add OSX, USB and WIN32
> only.
I found qemu monitor just accept XT_KBD, not XT, maybe I'm wrong.
>
>> diff --git a/src/util/virtkey.c b/src/util/virtkey.c
>> new file mode 100644
>> index 0000000..48fbfcc
>> --- /dev/null
>> +++ b/src/util/virtkey.c
>> @@ -0,0 +1,633 @@
>> +
>> +/*
>> + * Copyright (c) 2011 Lai Jiangshan
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + */
>> +
>> +#include <config.h>
>> +#include <string.h>
>> +#include <stddef.h>
>> +#include <libvirt/libvirt.h>
>> +#include "virtkey.h"
>> +
>> +#define ARRAY_SIZE(array) (sizeof(array)/sizeof(array[0]))
>> +#define getfield(object, field_type, field_offset) \
>> + (*(typeof(field_type) *)((char *)(object) + field_offset))
>> +
>> +struct keycode {
>> + const char *linux_name;
>> + const char *os_x_name;
>> + const char *win32_name;
>> + unsigned short linux_keycode;
>> + unsigned short xt;
>> + unsigned short atset1;
>> + unsigned short atset2;
>> + unsigned short atset3;
>> + unsigned short os_x;
>> + unsigned short xt_kbd;
>> + unsigned short usb;
>> + unsigned short win32;
>> + unsigned short xwin_xt;
>> + unsigned short xfree86_kbd_xt;
>> +};
>> +
>> +/*
>> + * generated from http://git.gnome.org/browse/gtk-vnc/plain/src/keymaps.csv
>> + * script:
>> + *
>> + * #!/bin/python
>> + * import sys
>> + * import re
>> + *
>> + * for line in sys.stdin.xreadlines():
>> + * a = re.match("([^,]*)," * 13 + "([^,]*)$", line[0:-1]).groups()
>> + * b = ""
>> + * for i in (0,2,10,1,7,4,5,6,3,8,9,11,12,13):
>> + * if i in (0, 2, 10):
>> + * b = b + (a[i] and ('"' + a[i] + '"') or 'NULL') + ','
>> + * else:
>> + * b = b + (a[i] or '0') + ','
>> + * print " { " + b + "},"
>> + */
>
> One of the goals of having the keymap data in the CSV file was that
> it makes it trivially updatable across apps using it, without making
> code changes. In fact my goal is to actually put 'keymaps.csv' and
> 'keymaps.pl' into a separate shared package at some point. So rather
> than hardcoding this giant array in libvirt, just include the GTK-VNC
> keymaps.csv and keymaps.pl file as-is, and run them to generate the
> mapping tables for combinations we need.
>
> NB, keymaps.pl will need to be updated to be able to output a
> table for doing "string->keycode" mapping since it doesn't
> do that yet. For the plain keycode->keycode mappings though
> just use its currently functionality.
I didn't find separate git repository for keymaps.csv.
Should I copy keymaps.csv to libvirt?
keymaps.pl need to be run O(N*N) times and it will generate O(N*N) tables
for different translating, I think that 1 table is the best, even the
table are bigger.
>
>> +const char *virKeycodeSetName(virKeycodeSet codeset)
>> +{
>> + int i = (int)codeset;
>> +
>> + if (i < 0 || i >= ARRAY_SIZE(codesetInfo))
>> + return "UNKNOWN";
>> +
>> + return codesetInfo[i].name;
>> +}
>> +
>> +virKeycodeSet virParseKeycodeSet(const char *name)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(codesetInfo); i++) {
>> + if (!strcmp(codesetInfo[i].name, name))
>> + return (virKeycodeSet)i;
>> + }
>> +
>> + return (virKeycodeSet)-1;
>> +}
>
> These just get replaced by VIR_ENUM_IMPL
Will do, thanks,
>
>> +static int virParseKeyNameOffset(unsigned int name_offset,
>> + unsigned int code_offset,
>> + const char *keyname)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
>> + const char *name = getfield(keycodes + i, const char *, name_offset);
>> +
>> + if (name && !strcmp(name, keyname))
>> + return getfield(keycodes + i, unsigned short, code_offset);
>> + }
>> +
>> + return -1;
>> +}
>
> This will want to use a number table that keymaps.pl will
> need to generate for name -> code mapping.
>
>> +static int virTranslateKeyCodeOffset(unsigned int from_offset,
>> + unsigned int to_offset,
>> + int key_value)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
>> + if (getfield(keycodes + i, unsigned short, from_offset) == key_value)
>> + return getfield(keycodes + i, unsigned short, to_offset);
>> + }
>> +
>> + return -1;
>> +}
>
> This is not nice because it is O(n) lookups. If you just use
> the keymaps.pl script to generate all the conversion tables
> we need, we get O(1) lookups & simpler code.
I think O(n) lookups is OK for <=16 keycodes.
Thank you very much. I need to investigate/think more
Lai
>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index fcd254d..a1e2f83 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -58,6 +58,7 @@
>> #include "threads.h"
>> #include "command.h"
>> #include "count-one-bits.h"
>> +#include "virtkey.h"
>>
>> static char *progname;
>
> This ought to be in the next patch, since this doesn't need it yet
>
> Regards,
> Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list