Re: [Qemu-devel] [RFC PATCH v8 21/21] replay: recording of the user input

2015-02-12 Thread Pavel Dovgaluk 
 From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
 Bonzini
 On 22/01/2015 09:53, Pavel Dovgalyuk wrote:
  +if (replay_mode != REPLAY_MODE_PLAY) {
  +evt = qemu_input_event_new_key(key, down);
  +if (QTAILQ_EMPTY(kbd_queue)) {
  +qemu_input_event_send(src, evt);
  +qemu_input_event_sync();
  +if (replay_mode != REPLAY_MODE_RECORD) {
  +qapi_free_InputEvent(evt);
  +}
 
 This is wrong.  You have different lifetimes for different modes. Please
 make a copy of the event in the implementation of record mode.

What is the correct way for cloning the QAPI type?
I should invent the cloning visitor or just create a switch for correct cloning 
of the InputEvent union?

Pavel Dovgalyuk

Re: [Qemu-devel] [RFC PATCH v8 21/21] replay: recording of the user input

2015-02-12 Thread Paolo Bonzini 


On 12/02/2015 09:08, Pavel Dovgaluk wrote:
  
  This is wrong.  You have different lifetimes for different modes. Please
  make a copy of the event in the implementation of record mode.
 What is the correct way for cloning the QAPI type?
 I should invent the cloning visitor or just create a switch for correct 
 cloning of the InputEvent union?

You can use the existing visitors to clone objects, see
qapi_copy_SocketAddress in qemu-char.c.

Paolo

Re: [Qemu-devel] [RFC PATCH v8 21/21] replay: recording of the user input

2015-02-11 Thread Pavel Dovgaluk 
 From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
 Bonzini
 On 22/01/2015 09:53, Pavel Dovgalyuk wrote:
  +void replay_input_event(QemuConsole *src, InputEvent *evt)
  +{
  +if (replay_mode == REPLAY_MODE_PLAY) {
  +/* Nothing */
  +} else if (replay_mode == REPLAY_MODE_RECORD) {
  +replay_add_input_event(evt);
 
 Does replay_add_input_event ultimately call qemu_input_event_send_impl?

No, it just adds event to the queue.

  +} else {
  +qemu_input_event_send_impl(src, evt);
  +}
  +}
  +
 
 Perhaps make this and replay_input_sync_event return a bool and in the
 caller do:
 
 if (replay_input_event(src, evt)) {
 qemu_input_event_send_impl(src, evt):
 }

No, we can't. qemu_input_event_send_impl is called when the queue is saved to 
the log.

Pavel Dovgalyuk

Re: [Qemu-devel] [RFC PATCH v8 21/21] replay: recording of the user input

2015-01-30 Thread Paolo Bonzini 


On 22/01/2015 09:53, Pavel Dovgalyuk wrote:
 +void replay_input_event(QemuConsole *src, InputEvent *evt)
 +{
 +if (replay_mode == REPLAY_MODE_PLAY) {
 +/* Nothing */
 +} else if (replay_mode == REPLAY_MODE_RECORD) {
 +replay_add_input_event(evt);

Does replay_add_input_event ultimately call qemu_input_event_send_impl?

 +} else {
 +qemu_input_event_send_impl(src, evt);
 +}
 +}
 +

Perhaps make this and replay_input_sync_event return a bool and in the
caller do:

if (replay_input_event(src, evt)) {
qemu_input_event_send_impl(src, evt):
}

 +if (replay_mode != REPLAY_MODE_PLAY) {
 +evt = qemu_input_event_new_key(key, down);
 +if (QTAILQ_EMPTY(kbd_queue)) {
 +qemu_input_event_send(src, evt);
 +qemu_input_event_sync();
 +if (replay_mode != REPLAY_MODE_RECORD) {
 +qapi_free_InputEvent(evt);
 +}

This is wrong.  You have different lifetimes for different modes. Please
make a copy of the event in the implementation of record mode.

Also, you do not need the if for replay mode.  The functions would
just do nothing.

 +} else {
 +if (replay_mode != REPLAY_MODE_NONE) {
 +fprintf(stderr, Input queue is not supported 
 +in record/replay mode\n);
 +exit(1);

Why?  For record mode should just work since qemu_input_event_send is
called in qemu_input_queue_process.

Replay mode can just do nothing, by returning early from
qemu_input_queue_event/qemu_input_queue_sync.

Paolo

 +}
 +qemu_input_queue_event(kbd_queue, src, evt);
 +qemu_input_queue_sync(kbd_queue);
 +}
  }