Re: [Qemu-devel] [PATCH] Rework vm_state_change notifiers

[Anthony Liguori <aliguori@us.ibm.com>]

Jan Kiszka wrote:
> [ Now tested and found working. ]
>
> Enhance the vm_state_change notifier API to pass also a change reason.
> This allows for replacing the vm_stop_cb with it and drop related code.
>
> Converting gdb_vm_stopped to gdb_vm_state_change also includes that this
> callback will now officially only handle EXCP_DEBUG and EXCP_INTERRUPT
> stop reasons. That allows to define and use new (non-zero) stop reasons
> without disturbing the gdbstub.
>   

Applied but this was missing a Signed-off-by.  Can you send one so that 
there's at least a record of it in the mailing list?

The missing Signed-off-by caused my scripts to eat your commit message 
too.  Sorry about that.

Regards,

Anthony Liguori

> ---
>
>  audio/audio.c |    3 ++-
>  gdbstub.c     |   11 +++++------
>  sysemu.h      |    6 +-----
>  vl.c          |   29 ++++-------------------------
>  4 files changed, 12 insertions(+), 37 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 762c2e3..e2635c0 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1622,7 +1622,8 @@ static int audio_driver_init (AudioState *s, struct audio_driver *drv)
>      }
>  }
>  
> -static void audio_vm_change_state_handler (void *opaque, int running)
> +static void audio_vm_change_state_handler (void *opaque, int running,
> +                                           int reason)
>  {
>      AudioState *s = opaque;
>      HWVoiceOut *hwo = NULL;
> diff --git a/gdbstub.c b/gdbstub.c
> index 99a4772..bb9b405 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1867,7 +1867,7 @@ void gdb_set_stop_cpu(CPUState *env)
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -static void gdb_vm_stopped(void *opaque, int reason)
> +static void gdb_vm_state_change(void *opaque, int running, int reason)
>  {
>      GDBState *s = gdbserver_state;
>      CPUState *env = s->c_cpu;
> @@ -1875,7 +1875,8 @@ static void gdb_vm_stopped(void *opaque, int reason)
>      const char *type;
>      int ret;
>  
> -    if (s->state == RS_SYSCALL)
> +    if (running || (reason != EXCP_DEBUG && reason != EXCP_INTERRUPT) ||
> +        s->state == RS_SYSCALL)
>          return;
>  
>      /* disable single step if it was enable */
> @@ -1904,10 +1905,8 @@ static void gdb_vm_stopped(void *opaque, int reason)
>          }
>  	tb_flush(env);
>          ret = GDB_SIGNAL_TRAP;
> -    } else if (reason == EXCP_INTERRUPT) {
> -        ret = GDB_SIGNAL_INT;
>      } else {
> -        ret = 0;
> +        ret = GDB_SIGNAL_INT;
>      }
>      snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, env->cpu_index+1);
>      put_packet(s, buf);
> @@ -2300,7 +2299,7 @@ int gdbserver_start(const char *port)
>      gdbserver_state = s;
>      qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
>                            gdb_chr_event, NULL);
> -    qemu_add_vm_stop_handler(gdb_vm_stopped, NULL);
> +    qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
>      return 0;
>  }
>  #endif
> diff --git a/sysemu.h b/sysemu.h
> index 56eb9b3..abd8e81 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -12,16 +12,12 @@ extern uint8_t qemu_uuid[];
>  #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
>  
>  typedef struct vm_change_state_entry VMChangeStateEntry;
> -typedef void VMChangeStateHandler(void *opaque, int running);
> -typedef void VMStopHandler(void *opaque, int reason);
> +typedef void VMChangeStateHandler(void *opaque, int running, int reason);
>  
>  VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>                                                       void *opaque);
>  void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>  
> -int qemu_add_vm_stop_handler(VMStopHandler *cb, void *opaque);
> -void qemu_del_vm_stop_handler(VMStopHandler *cb, void *opaque);
> -
>  void vm_start(void);
>  void vm_stop(int reason);
>  
> diff --git a/vl.c b/vl.c
> index 34ddc07..77f7234 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3420,37 +3420,21 @@ void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
>      qemu_free (e);
>  }
>  
> -static void vm_state_notify(int running)
> +static void vm_state_notify(int running, int reason)
>  {
>      VMChangeStateEntry *e;
>  
>      for (e = vm_change_state_head.lh_first; e; e = e->entries.le_next) {
> -        e->cb(e->opaque, running);
> +        e->cb(e->opaque, running, reason);
>      }
>  }
>  
> -/* XXX: support several handlers */
> -static VMStopHandler *vm_stop_cb;
> -static void *vm_stop_opaque;
> -
> -int qemu_add_vm_stop_handler(VMStopHandler *cb, void *opaque)
> -{
> -    vm_stop_cb = cb;
> -    vm_stop_opaque = opaque;
> -    return 0;
> -}
> -
> -void qemu_del_vm_stop_handler(VMStopHandler *cb, void *opaque)
> -{
> -    vm_stop_cb = NULL;
> -}
> -
>  void vm_start(void)
>  {
>      if (!vm_running) {
>          cpu_enable_ticks();
>          vm_running = 1;
> -        vm_state_notify(1);
> +        vm_state_notify(1, 0);
>          qemu_rearm_alarm_timer(alarm_timer);
>      }
>  }
> @@ -3460,12 +3444,7 @@ void vm_stop(int reason)
>      if (vm_running) {
>          cpu_disable_ticks();
>          vm_running = 0;
> -        if (reason != 0) {
> -            if (vm_stop_cb) {
> -                vm_stop_cb(vm_stop_opaque, reason);
> -            }
> -        }
> -        vm_state_notify(0);
> +        vm_state_notify(0, reason);
>      }
>  }
>  
>
>
>
>