Re: [Qemu-devel] [PATCH v3] add input-send-event command

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Sep 29, 2014 at 01:12:44PM -0600, Eric Blake wrote:
> On 09/29/2014 12:56 PM, Marcelo Tosatti wrote:
> > 
> > Which allows specification of absolute/relative,
> > up/down and console parameters.
> > 
> > Suggested by Gerd Hoffman.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > ---
> >  qapi-schema.json | 17 +++++++++++++++
> >  qmp-commands.hx  | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  ui/input.c       | 31 ++++++++++++++++++++++++++
> >  3 files changed, 111 insertions(+)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 4bfaf20..2e9e261 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3233,6 +3233,23 @@
> >                'abs'     : 'InputMoveEvent' } }
> >  
> >  ##
> > +# @input-send-event
> > +#
> > +# Send input event(s) to guest.
> > +#
> > +# @console: Which console to send event(s) to.
> > +#
> > +# @events: List of InputEvent union.
> > +#
> > +# Returns: Nothing on success.
> > +#
> > +# Since: 2.2
> > +#
> > +##
> > +{ 'command': 'input-send-event',
> > +  'data': { 'console':'int', 'events': [ 'InputEvent' ] } }
> 
> 'console' is mandatory; I guess that's okay.
> 
> Are we guaranteed that either all events are sent? Or is there a need to

Events can be dropped at hardware level if the event queue is full, for 
example. Would have to modify individual drivers to return error codes, 
i suppose. Gerd?

> worry about partial success (on a list of 3 events, the first gets sent,
> then some error is encountered on the second, and the third is not
> attempted)?  Are the only errors due to something that can be detected
> up front (such as trying to send a mouse event to a console that has
> only keyboard support)?
>
> > +The consoles are visible in the qom tree, under
> > +/backend/console[$index]. They have a device link and head property, so
> > +its possible to map which console belongs to which device and display.
> 
> s/its/it's/ (or 'it is')
>
> > +void qmp_input_send_event(int64_t console, InputEventList *events,
> > +                          Error **errp)
> > +{
> > +    InputEventList *e;
> > +    QemuConsole *con;
> > +
> > +    con = qemu_console_lookup_by_index(console);
> > +    if (!con) {
> > +        error_setg(errp, "console %" PRId64 " not found", console);
> > +        return;
> > +    }
> > +
> > +    if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) {
> > +        error_setg(errp, "VM not running");
> > +        return;
> > +    }
> > +
> > +    for (e = events; e != NULL; e = e->next) {
> > +        InputEvent *event = e->value;
> > +
> > +        if (!qemu_input_find_handler(1 << event->kind, con)) {
> > +            error_setg(errp, "Input handler not found for "
> > +                             "event type %s",
> > +                            InputEventKind_lookup[event->kind]);
> > +            return;
> 
> 
> Ouch. You can return mid-loop.  I'd be more comfortable with a two-pass
> algorithm (first pass ensures all list elements have a handler, second
> actually calls qemu_input_event_send) or with a return that gives an
> integer count of how many list items were processed.

Sure.