Re: [Qemu-devel] [PATCH 2/2] Added monitor commands: 'keyboard_set' and 'info keybaord'

[Luiz Capitulino <lcapitulino@redhat.com>]

On Sat, 20 Mar 2010 00:29:25 +0200
Shahar Havivi <shaharh@redhat.com> wrote:

> Fix to Luiz comments.

 Better to resend the whole series with a 'v2' in it, also I think you'll
have to rebase against master because this series conflicts with latest
Anthony's series.

 I have some QError comments below.

> 
> Signed-off-by: Shahar Havivi <shaharh@redhat.com>
> ---
>  console.h       |    4 ++
>  input.c         |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  monitor.c       |    8 ++++
>  qemu-monitor.hx |   17 +++++++++
>  qerror.c        |    8 ++++
>  qerror.h        |    6 +++
>  6 files changed, 147 insertions(+), 0 deletions(-)
> 
> diff --git a/console.h b/console.h
> index c81cd9d..27d36b4 100644
> --- a/console.h
> +++ b/console.h
> @@ -73,6 +73,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 f75d480..1ce0754 100644
> --- a/input.c
> +++ b/input.c
> @@ -337,3 +337,107 @@ void do_mouse_set(Monitor *mon, const QDict *qdict)
>      else
>          monitor_printf(mon, "Mouse at given index not found\n");
>  }
> +
> +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);
> +}
> +
> +/*
> + * 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_KEYBOARD_NOT_FOUND);
> +        return -1;
> +    }

 Couldn't you use QERR_DEVICE_NOT_FOUND?

> +
> +    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_KEYBOARD_INDEX, index);
> +        return -1;
> +    }
> +    return 0;

 We're using something like:

qerror_report(QERR_INVALID_PARAMETER, "index");

 For this kind of error.

> +}
> 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..d86be17 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
> +@item keyboard_set @var{index}
> +@findex keyboard_set
> +Set which keyboard device receives events at given @var{index}, index
> +can be obtained with
> +@example
> +info keyboard
> +@end example
> +ETEXI
>  #ifdef HAS_AUDIO
>      {
>          .name       = "wavcapture",
> diff --git a/qerror.c b/qerror.c
> index d0aba61..41e7373 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_KEYBOARD_NOT_FOUND,
> +        .desc      = "No keyboard device found",
> +    },
> +    {
> +        .error_fmt = QERR_INVALID_KEYBOARD_INDEX,
> +        .desc      = "Invalid index '%(index)' for keyboard device",
> +    },
>      {}
>  };
>  
> diff --git a/qerror.h b/qerror.h
> index d96abe1..4086640 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -141,4 +141,10 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_VNC_SERVER_FAILED \
>      "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
>  
> +#define QERR_KEYBOARD_NOT_FOUND \
> +    "{ 'class': 'KeyboardNotFound', 'data': {} }"
> +
> +#define QERR_INVALID_KEYBOARD_INDEX \
> +    "{ 'class': 'InvalidKeyboardIndex', 'data': { 'index': %d } }"
> +
>  #endif /* QERROR_H */