chardev: Use timer instead of bottom-half to postpone open event

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).

Signed-off-by Jan Kiszka <[email protected]>

---

This obsoletes the versatilepb hack.

 qemu-char.c |   13 +++++++------
 qemu-char.h |    2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

qemu-char.c +7 -6

index f9ee2f0..fb4e3dc 100644
@@ -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);
     }
 }
 

qemu-char.h +0 -0

index 486644b..297dd98 100644

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).

Signed-off-by Jan Kiszka <[email protected]>

---

This obsoletes the versatilepb hack.

  qemu-char.c |   13 +++++++------
  qemu-char.h |    2 +-
  2 files changed, 8 insertions(+), 7 deletions(-)

qemu-char.c +7 -6

index f9ee2f0..fb4e3dc 100644
@@ -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);
      }
  }
 

qemu-char.h +1 -1

index 486644b..297dd98 100644
@@ -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;

Hi,

Funny that you ping about this just now, as I wanted to work on this today, but sofar I did not get around to it. Maybe tonight I will :)

Long story short: Yes I've some remarks, but I pan to fix those myself and then merge it in my tree.

I'll get back to you when this is done.

Regards,

Hans

Quoted 61 lines · Jan Kiszka

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).

Signed-off-by Jan Kiszka <[email protected]>

---

This obsoletes the versatilepb hack.

  qemu-char.c |   13 +++++++------
  qemu-char.h |    2 +-
  2 files changed, 8 insertions(+), 7 deletions(-)

qemu-char.c +7 -6

index f9ee2f0..fb4e3dc 100644
@@ -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);
      }
  }
 

qemu-char.h +1 -1

index 486644b..297dd98 100644
@@ -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;

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 <[email protected]>

---

Changes in v2:
- Move init_alarm_timer call to fix e.g. -monitor stdio

 qemu-char.c |   13 +++++++------
 qemu-char.h |    2 +-
 vl.c        |   10 +++++-----
 3 files changed, 13 insertions(+), 12 deletions(-)

qemu-char.c +7 -6

index b082bae..d8eed21 100644
@@ -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);
     }
 }
 

qemu-char.h +1 -1

index 486644b..297dd98 100644
@@ -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;

vl.c +5 -5

index 5b357a3..586773c 100644
@@ -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();

Quoted 10 lines · Jan Kiszka

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.

Quoted 5 lines

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- Move init_alarm_timer call to fix e.g. -monitor stdio

Ping?

Jan

Quoted 70 lines

 qemu-char.c |   13 +++++++------
 qemu-char.h |    2 +-
 vl.c        |   10 +++++-----
 3 files changed, 13 insertions(+), 12 deletions(-)

qemu-char.c +7 -6

index b082bae..d8eed21 100644
@@ -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);
     }
 }
 

qemu-char.h +1 -1

index 486644b..297dd98 100644
@@ -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;

vl.c +5 -0

index 5b357a3..586773c 100644
@@ -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();
 

vl.c int main(int argc, char **argv, char **envp) · line 3516

Quoted 9 lines

vl.c +0 -5

-    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();