[Qemu-devel] char: Send out OPENED events only on char dev open; not on qemu initial reset

[Qemu-devel] char: Send out OPENED events only on char dev open; not on qemu initial reset

[Amit Shah]

Hello,

These patches rename the CHR_EVENT_RESET event to CHR_EVENT_OPENED,
which is what it really is.

The check for initial_reset_issued was not really necessary, so that has
been done away with.

Finally, this event is now only sent when a char dev is opened instead
of the current behaviour of also sending it when qemu does its early
init of char state. The consumers of the event shouldn't be interested
in this anyway.

I've tested this with the monitor.

[Qemu-devel] [PATCH 1/3] char: check for initial_reset_issued unnecessary

[Amit Shah]

At init, qemu_chr_reset is always called with initial_reset_issued set to 1.
So checking for it to be set is not necessary.

A later patch will make use of initial_reset_issued to send the reset event
only if the char device is opened instead of each time when qemu resets it

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8084a67..99ab471 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -127,7 +127,7 @@ static void qemu_chr_reset_bh(void *opaque)
 
 void qemu_chr_reset(CharDriverState *s)
 {
-    if (s->bh == NULL && initial_reset_issued) {
+    if (s->bh == NULL) {
 	s->bh = qemu_bh_new(qemu_chr_reset_bh, s);
 	qemu_bh_schedule(s->bh);
     }
@@ -137,11 +137,10 @@ void qemu_chr_initial_reset(void)
 {
     CharDriverState *chr;
 
-    initial_reset_issued = 1;
-
     QTAILQ_FOREACH(chr, &chardevs, next) {
         qemu_chr_reset(chr);
     }
+    initial_reset_issued = 1;
 }
 
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len)
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/3] char: rename CHR_EVENT_RESET to CHR_EVENT_OPENED

[Amit Shah]

The char event RESET is emitted when a char device is opened.
Give it a better name.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 gdbstub.c       |    2 +-
 hw/baum.c       |    2 +-
 hw/usb-serial.c |    2 +-
 monitor.c       |    2 +-
 qemu-char.c     |    2 +-
 qemu-char.h     |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 315f606..055093f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2447,7 +2447,7 @@ static void gdb_chr_receive(void *opaque, const uint8_t *buf, int size)
 static void gdb_chr_event(void *opaque, int event)
 {
     switch (event) {
-    case CHR_EVENT_RESET:
+    case CHR_EVENT_OPENED:
         vm_stop(EXCP_INTERRUPT);
         gdb_has_xml = 0;
         break;
diff --git a/hw/baum.c b/hw/baum.c
index 665deab..c66e737 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -475,7 +475,7 @@ static void baum_send_event(CharDriverState *chr, int event)
     switch (event) {
     case CHR_EVENT_BREAK:
         break;
-    case CHR_EVENT_RESET:
+    case CHR_EVENT_OPENED:
         /* Reset state */
         baum->in_buf_used = 0;
         break;
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index e2379c4..d02f6b2 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -516,7 +516,7 @@ static void usb_serial_event(void *opaque, int event)
             break;
         case CHR_EVENT_FOCUS:
             break;
-        case CHR_EVENT_RESET:
+        case CHR_EVENT_OPENED:
             usb_serial_reset(s);
             /* TODO: Reset USB port */
             break;
diff --git a/monitor.c b/monitor.c
index f105a2e..692b696 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3139,7 +3139,7 @@ static void monitor_event(void *opaque, int event)
         mon->mux_out = 1;
         break;
 
-    case CHR_EVENT_RESET:
+    case CHR_EVENT_OPENED:
         monitor_printf(mon, "QEMU %s monitor - type 'help' for more "
                        "information\n", QEMU_VERSION);
         if (!mon->mux_out) {
diff --git a/qemu-char.c b/qemu-char.c
index 99ab471..8fde8cf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -120,7 +120,7 @@ static void qemu_chr_event(CharDriverState *s, int event)
 static void qemu_chr_reset_bh(void *opaque)
 {
     CharDriverState *s = opaque;
-    qemu_chr_event(s, CHR_EVENT_RESET);
+    qemu_chr_event(s, CHR_EVENT_OPENED);
     qemu_bh_delete(s->bh);
     s->bh = NULL;
 }
diff --git a/qemu-char.h b/qemu-char.h
index c0654bc..05fe15d 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -10,7 +10,7 @@
 
 #define CHR_EVENT_BREAK   0 /* serial break char */
 #define CHR_EVENT_FOCUS   1 /* focus to this terminal (modal input needed) */
-#define CHR_EVENT_RESET   2 /* new connection established */
+#define CHR_EVENT_OPENED  2 /* new connection established */
 #define CHR_EVENT_MUX_IN  3 /* mux-focus was set to this terminal */
 #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
 #define CHR_EVENT_CLOSED  5 /* connection closed */
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/3] char: emit the OPENED event only when a new char connection is opened

[Amit Shah]

The OPENED event gets sent also when qemu resets its state initially.
The consumers of the event aren't interested in receiving this event
on reset.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8fde8cf..02c096c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -120,7 +120,10 @@ static void qemu_chr_event(CharDriverState *s, int event)
 static void qemu_chr_reset_bh(void *opaque)
 {
     CharDriverState *s = opaque;
-    qemu_chr_event(s, CHR_EVENT_OPENED);
+
+    if (initial_reset_issued) {
+        qemu_chr_event(s, CHR_EVENT_OPENED);
+    }
     qemu_bh_delete(s->bh);
     s->bh = NULL;
 }
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 1/3] char: check for initial_reset_issued unnecessary

[Amit Shah]

On (Wed) Oct 07 2009 [16:40:38], Amit Shah wrote:
> At init, qemu_chr_reset is always called with initial_reset_issued set to 1.
> So checking for it to be set is not necessary.
> 
> A later patch will make use of initial_reset_issued to send the reset event
> only if the char device is opened instead of each time when qemu resets it
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  qemu-char.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 8084a67..99ab471 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -127,7 +127,7 @@ static void qemu_chr_reset_bh(void *opaque)
>  
>  void qemu_chr_reset(CharDriverState *s)
>  {
> -    if (s->bh == NULL && initial_reset_issued) {
> +    if (s->bh == NULL) {
>  	s->bh = qemu_bh_new(qemu_chr_reset_bh, s);
>  	qemu_bh_schedule(s->bh);
>      }
> @@ -137,11 +137,10 @@ void qemu_chr_initial_reset(void)
>  {
>      CharDriverState *chr;
>  
> -    initial_reset_issued = 1;
> -
>      QTAILQ_FOREACH(chr, &chardevs, next) {
>          qemu_chr_reset(chr);
>      }
> +    initial_reset_issued = 1;
>  }

Actually this won't work since the actual reset is done in a
bh_schedule.

I'll have to find out another way of doing this.

		Amit