Re: [Qemu-devel] [PATCH v2] chardev: Use timer instead of bottom-half to postpone open event

[Jan Kiszka]

On 2012-10-12 09:52, Jan Kiszka wrote:
> As the block layer may decide to flush bottom-halfs while the machine is
> still initializing (e.g. to read geometry data from the disk), our
> postponed open event may be processed before the last frontend
> registered with a muxed chardev.
> 
> Until the semantics of BHs have been clarified, use an expired timer to
> achieve the same effect (suggested by Paolo Bonzini). This requires to
> perform the alarm timer initialization earlier as otherwise timer
> subsystem can be used before being ready.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
> 
> Changes in v2:
>  - Move init_alarm_timer call to fix e.g. -monitor stdio

Ping?

Jan

> 
>  qemu-char.c |   13 +++++++------
>  qemu-char.h |    2 +-
>  vl.c        |   10 +++++-----
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..d8eed21 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -123,19 +123,20 @@ void qemu_chr_be_event(CharDriverState *s, int event)
>      s->chr_event(s->handler_opaque, event);
>  }
>  
> -static void qemu_chr_generic_open_bh(void *opaque)
> +static void qemu_chr_fire_open_event(void *opaque)
>  {
>      CharDriverState *s = opaque;
>      qemu_chr_be_event(s, CHR_EVENT_OPENED);
> -    qemu_bh_delete(s->bh);
> -    s->bh = NULL;
> +    qemu_free_timer(s->open_timer);
> +    s->open_timer = NULL;
>  }
>  
>  void qemu_chr_generic_open(CharDriverState *s)
>  {
> -    if (s->bh == NULL) {
> -     s->bh = qemu_bh_new(qemu_chr_generic_open_bh, s);
> -     qemu_bh_schedule(s->bh);
> +    if (s->open_timer == NULL) {
> +        s->open_timer = qemu_new_timer_ms(vm_clock,
> +                                          qemu_chr_fire_open_event, s);
> +        qemu_mod_timer(s->open_timer, qemu_get_clock_ms(vm_clock) - 1);
>      }
>  }
>  
> diff --git a/qemu-char.h b/qemu-char.h
> index 486644b..297dd98 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -69,7 +69,7 @@ struct CharDriverState {
>      void (*chr_guest_open)(struct CharDriverState *chr);
>      void (*chr_guest_close)(struct CharDriverState *chr);
>      void *opaque;
> -    QEMUBH *bh;
> +    QEMUTimer *open_timer;
>      char *label;
>      char *filename;
>      int opened;
> diff --git a/vl.c b/vl.c
> index 5b357a3..586773c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3439,6 +3439,11 @@ int main(int argc, char **argv, char **envp)
>              add_device_config(DEV_VIRTCON, "vc:80Cx24C");
>      }
>  
> +    if (init_timer_alarm() < 0) {
> +        fprintf(stderr, "could not initialize alarm timer\n");
> +        exit(1);
> +    }
> +
>      socket_init();
>  
>      if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, 
> NULL, 1) != 0)
> @@ -3506,11 +3511,6 @@ int main(int argc, char **argv, char **envp)
>  
>      os_set_line_buffering();
>  
> -    if (init_timer_alarm() < 0) {
> -        fprintf(stderr, "could not initialize alarm timer\n");
> -        exit(1);
> -    }
> -
>  #ifdef CONFIG_SPICE
>      /* spice needs the timers to be initialized by this point */
>      qemu_spice_init();
> 


-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux