Re: [Qemu-devel] [PATCH] Monitor: Convert do_sendkey() to QObject, QError

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 22 Jul 2010 15:03:27 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, Jul 22, 2010 at 10:50:00AM -0300, Luiz Capitulino wrote:
> > On Thu, 22 Jul 2010 14:45:35 +0100
> > "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > 
> > > On Thu, Jul 22, 2010 at 10:28:39AM -0300, Luiz Capitulino wrote:
> > > > On Wed, 21 Jul 2010 20:06:56 +0100
> > > > "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > > > 
> > > > > On Wed, Jul 21, 2010 at 03:44:14PM -0300, Luiz Capitulino wrote:
> > > > > > Another related issue is that, this probably should an async handler. But
> > > > > > as we don't have the proper infrastructure yet, I'm ok with having this in
> > > > > > its current form.
> > > > > > 
> > > > > > > +- "hold_time": duration in milliseconds to hold the keys down (json-int, optional, default=100)
> > > > > 
> > > > > Having 'hold-time' which applies to the full list of keys is limiting
> > > > > the flexibility of apps. eg, it means you can only do
> > > > > 
> > > > >    down ctrl
> > > > >    down alt
> > > > >    down f1
> > > > >    wait 100ms
> > > > >    up ctrl
> > > > >    up alt
> > > > >    up f1
> > > > > 
> > > > > Again I can see why the impl works this way currently, because it is
> > > > > clearly a nicer option for humans. For a machine protocol though it
> > > > > seems sub-optimal. What if app needed more flexibility over ordering
> > > > > of press+release events eg to release in a different order
> > > > > 
> > > > >    down ctrl
> > > > >    down alt
> > > > >    down f1
> > > > >    wait 100ms
> > > > >    up f1
> > > > >    up ctrl
> > > > >    up alt
> > > > > 
> > > > > Should we just follow VNC and explicitly have a up/down flag in
> > > > > the protocol & let press & release events be sent separately.
> > > > > 
> > > > >   { "execute": "sendkey", "arguments":  { "keycode": 0x31, "down": true } }
> > > > > 
> > > > > We could allow multiple keycodes in one message
> > > > > 
> > > > >   { "execute": "sendkey", "arguments":  { "keycodes": [ 0x31, 0x32 ], "down": true } }
> > > > > 
> > > > > but its not really adding critical functionality that can't be got by
> > > > > sending a sequence of sendkey commands in a row.
> > > > 
> > > > Hm, looks good to me, but then the hold time would be the time period
> > > > between the down/up commands. This won't be reliable in case the client
> > > > wants to exactly wait 100ms, as we can have network latency, for example.
> > > > 
> > > > Isn't this a problem? I believe VNC doesn't have this feature, right?
> > > 
> > > Correct, VNC just sends each individual press / release event as a separate
> > > message, so you can have network delay effects there too.
> > > 
> > > If we needed to support precise delays safe from network delay for some
> > > simulation needs, then you'd probably need a more complex structure
> > > where you can provide a whole sequence of operations. And why stop at
> > > keys, including mouse movement & buttons to.
> > > 
> > >    { "execute": "sendinput", "arguments":  { 
> > >        "sequence" : [
> > >           { "event": "keypress", "keycode": 0x31 },
> > >           { "event": "keypress", "keycode": 0x75 },
> > >           { "event": "wait", "delay": 100 },
> > >           { "event": "mousepress", "button": 1 },
> > >           { "event": "mousemove", "xdelta": 1, "ydelta": 1 },
> > >           { "event": "keyrelease", "keycode": 0x31 },
> > >           { "event": "wait", "delay": 100 },
> > >           { "event": "keyrelease", "keycode": 0x75 },
> > >           { "event": "mousepos", "x": 102, "y": 102 },
> > >        ] 
> > >    } }
> > > 
> > > This is getting kind of advanced now. Whether we need this vs the simpler
> > > sendkey, mouse_move, etc command comes down to whether we need ability to
> > > set precise delays between events.  We could stick with the individual
> > > simple commands & add a advanced one alter  
> > 
> > Yeah, agreed and I think the current design (ie. hold_time) is simpler.
> 
> When I said stick with simpler sendkey, I mean the style I outlined
> without any hold time at all
> 
> > > > >   { "execute": "sendkey", "arguments":  { "keycode": 0x31, "down": true } }
> 
> If its good enough for VNC & SPICE, it should be good enough for most
> monitor users, even more so since QMP communication is usually over a
> low latency UNIX domain socket. I think hold time is a flawed concept
> as currently provide since it prevents up/down interleaving by sending
> a sequence of QMP commands. 

I don't want to suck the QMP interface because of that, but the problem
here is how to maintain the user monitor's current behavior.

The best option is to wait for the user monitor/QMP split. This way the
hold time could be a feature of the user monitor command only.

Or, we could add a new command. But this would just be a workaround..