[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] Added monitor commands: 'keyboard_set' and
From: |
Shahar Havivi |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] Added monitor commands: 'keyboard_set' and 'info keybaord' |
Date: |
Fri, 26 Mar 2010 21:40:31 +0300 |
User-agent: |
Mutt/1.5.20 (2009-08-17) |
On Fri, Mar 26, 2010 at 10:57:58AM +0100, Markus Armbruster wrote:
> Date: Fri, 26 Mar 2010 10:57:58 +0100
> From: Markus Armbruster <address@hidden>
> To: Shahar Havivi <address@hidden>
> Cc: address@hidden
> Subject: Re: [Qemu-devel] [PATCH 2/2] Added monitor commands:
> 'keyboard_set' and 'info keybaord'
>
> Shahar Havivi <address@hidden> writes:
>
> > Two new monitor commands: adding ability to handle which keyboard qemu will
> > use and to see which keyboard are currently available.
> >
> > $ info keyboard
> > $ keyboard_set <index>
> >
> > Signed-off-by: Shahar Havivi <address@hidden>
> > ---
> > console.h | 4 ++
> > input.c | 104
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > monitor.c | 8 ++++
> > qemu-monitor.hx | 17 +++++++++
> > qerror.c | 8 ++++
> > 5 files changed, 141 insertions(+), 0 deletions(-)
> >
> > diff --git a/console.h b/console.h
> > index 16c9c3d..7efa88e 100644
> > --- a/console.h
> > +++ b/console.h
> > @@ -85,6 +85,10 @@ void do_info_mice_print(Monitor *mon, const QObject
> > *data);
> > void do_info_mice(Monitor *mon, QObject **ret_data);
> > void do_mouse_set(Monitor *mon, const QDict *qdict);
> >
> > +void do_info_keyboard_print(Monitor *mon, const QObject *data);
> > +void do_info_keyboard(Monitor *mon, QObject **ret_data);
> > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data);
> > +
> > /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx
> > constants) */
> > #define QEMU_KEY_ESC1(c) ((c) | 0xe100)
> > diff --git a/input.c b/input.c
> > index 563ecad..4daaeb0 100644
> > --- a/input.c
> > +++ b/input.c
> > @@ -368,3 +368,107 @@ void qemu_remove_mouse_mode_change_notifier(Notifier
> > *notify)
> > {
> > notifier_list_remove(&mouse_mode_notifiers, notify);
> > }
> > +
> > +static void info_keyboard_iter(QObject *data, void *opaque)
> > +{
> > + QDict *kbd;
> > + Monitor *mon = opaque;
> > +
> > + kbd = qobject_to_qdict(data);
> > + monitor_printf(mon, "%c Keyboard #%" PRId64 ": %s\n",
> > + (qdict_get_bool(kbd, "current") ? '*' : ' '),
> > + qdict_get_int(kbd, "index"), qdict_get_str(kbd, "name"));
> > +}
> > +
> > +void do_info_keyboard_print(Monitor *mon, const QObject *data)
> > +{
> > + QList *kbd_list;
> > +
> > + kbd_list = qobject_to_qlist(data);
> > + if (qlist_empty(kbd_list)) {
> > + monitor_printf(mon, "No keyboard devices connected\n");
> > + return;
> > + }
> > +
> > + qlist_iter(kbd_list, info_keyboard_iter, mon);
> > +}
> > +
> > +/*
> > + * do_info_keyboard(): Show VM keyboard information
> > + *
> > + * Each keyboard is represented by a QDict, the returned QObject is
> > + * a QList of all keyboards.
> > + *
> > + * The keyboard QDict contains the following:
> > + *
> > + * - "name": keyboard's name
> > + * - "index": keyboard's index
> > + * - "current": true if this keyboard is receiving events, false otherwise
> > + *
> > + * Example:
> > + *
> > + * [ { "name": "QEMU USB Keyboard", "index": 0, "current": false },
> > + * { "name": "QEMU PS/2 Keyboard", "index": 1, "current": true } ]
> > + */
> > +void do_info_keyboard(Monitor *mon, QObject **ret_data)
> > +{
> > + QEMUPutKbdEntry *cursor;
> > + QList *kbd_list;
> > + int index = 0;
> > +
> > + kbd_list = qlist_new();
> > +
> > + if (!qemu_put_kbd_event_head) {
> > + goto out;
> > + }
> > +
> > + cursor = qemu_put_kbd_event_head;
> > + while (cursor != NULL) {
> > + QObject *obj;
> > + obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i
> > }",
> > + cursor->qemu_put_kbd_name,
> > + index, cursor ==
> > qemu_put_kbd_event_current);
> > + qlist_append_obj(kbd_list, obj);
> > + index++;
> > + cursor = cursor->next;
> > + }
> > +out:
> > + *ret_data = QOBJECT(kbd_list);
> > +}
>
> Keyboard indexes change when keyboards other than the last one get
> removed. Hmm.
>
> > +
> > +/*
> > + * do_keyboard_set(): Set active keyboard
> > + *
> > + * Argument qdict contains
> > + * - "index": the keyboard index to set
> > + *
> > + * Example:
> > + *
> > + * { "index": "0" }
> > + */
> > +int do_keyboard_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > +{
> > + QEMUPutKbdEntry *cursor;
> > + int i = 0;
> > + int index = qdict_get_int(qdict, "index");
> > +
> > + if (!qemu_put_kbd_event_head) {
> > + qerror_report(QERR_DEVICE_NOT_FOUND, "keyboard");
> > + return -1;
> > + }
> > +
> > + cursor = qemu_put_kbd_event_head;
> > + while (cursor != NULL && index != i) {
> > + i++;
> > + cursor = cursor->next;
> > + }
> > +
> > + if (cursor != NULL) {
> > + qemu_put_kbd_event_current = cursor;
> > + }
> > + else {
> > + qerror_report(QERR_INVALID_PARAMETER, "index");
> > + return -1;
> > + }
> > + return 0;
> > +}
>
> Since the index is not a stable identifier of keyboards, and we support
> multiple monitors, you're prone to remove the wrong keyboard.
>
> Monitor#1 Monitor#2
> "info keyboard" to find the index
> unplug a keyboard, invalidating
> index
> keyboard_set <index>
>
> I think it's best to use a stable ID, like we do in other places. We
> commonly let the user specify it, e.g. as id=ID in -netdev, -device and
> elswhere.
What do we do when user not specify id for device?
>
> > diff --git a/monitor.c b/monitor.c
> > index 0448a70..cc95b3d 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2783,6 +2783,14 @@ static const mon_cmd_t info_cmds[] = {
> > .mhandler.info_new = do_info_mice,
> > },
> > {
> > + .name = "keyboard",
> > + .args_type = "",
> > + .params = "",
> > + .help = "show which guest keyboard is receiving events",
> > + .user_print = do_info_keyboard_print,
> > + .mhandler.info_new = do_info_keyboard,
> > + },
> > + {
> > .name = "vnc",
> > .args_type = "",
> > .params = "",
> > diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> > index 5308f36..e9beb12 100644
> > --- a/qemu-monitor.hx
> > +++ b/qemu-monitor.hx
> > @@ -659,6 +659,23 @@ info mice
> > @end example
> > ETEXI
> >
> > + {
> > + .name = "keyboard_set",
> > + .args_type = "index:i",
> > + .params = "index",
> > + .help = "set which keyboard device receives events",
> > + .mhandler.cmd_new = do_keyboard_set,
> > + },
> > +
> > +STEXI
> > address@hidden keyboard_set @var{index}
> > address@hidden keyboard_set
> > +Set which keyboard device receives events at given @var{index}, index
> > +can be obtained with
> > address@hidden
> > +info keyboard
> > address@hidden example
> > +ETEXI
> > #ifdef HAS_AUDIO
> > {
> > .name = "wavcapture",
> > diff --git a/qerror.c b/qerror.c
> > index d0aba61..42a5cf7 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -172,6 +172,14 @@ static const QErrorStringTable qerror_table[] = {
> > .error_fmt = QERR_VNC_SERVER_FAILED,
> > .desc = "Could not start VNC server on %(target)",
> > },
> > + {
> > + .error_fmt = QERR_DEVICE_NOT_FOUND,
> > + .desc = "No keyboard device found",
> > + },
> > + {
> > + .error_fmt = QERR_INVALID_PARAMETER,
> > + .desc = "Invalid index '%(name)' for keyboard device",
> > + },
> > {}
> > };
>
> I'm afraid you're breaking all other uses of these errors here.
you right, my bad - this errors where not meant to be in this patch.