[Qemu-devel] char: remove init_reset handling, more data per write()

[Qemu-devel] char: remove init_reset handling, more data per write()

[Amit Shah]

Hello,

These patches, all independent of each other, make the char backend a
little better by removing the initial_reset handling that's unnecessary,
bump up the amount of data that's sent per write() call to 4k from the
current 1k and also renames the generic char open() command to reflect
reality.

Please apply.

		Amit

[Qemu-devel] [PATCH 1/3] char: don't limit data sent to backends to 1k per buffer

[Amit Shah]

chardevs have a 'can_read' function via which backends specify
the amount of data they can receive. When can_read returns > 0,
apps can start sending data. However, each chardev driver here
allows a max. of 1k bytes inspite of the backend being able to
receive more.

The best we can do here is to allocate s->max_size bytes from
the heap on each call (which is the number returned by the
backend from the can_read call).

This is an intermediate step to bump up the bytes written in
each call to 4k.

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

diff --git a/qemu-char.c b/qemu-char.c
index 40bd7e8..1f63019 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -97,6 +97,8 @@
 
 #include "qemu_socket.h"
 
+#define READ_BUF_LEN 4096
+
 /***********************************************************/
 /* character device */
 
@@ -172,7 +174,7 @@ void qemu_chr_accept_input(CharDriverState *s)
 
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
 {
-    char buf[4096];
+    char buf[READ_BUF_LEN];
     va_list ap;
     va_start(ap, fmt);
     vsnprintf(buf, sizeof(buf), fmt, ap);
@@ -555,7 +557,7 @@ static void fd_chr_read(void *opaque)
     CharDriverState *chr = opaque;
     FDCharDriver *s = chr->opaque;
     int size, len;
-    uint8_t buf[1024];
+    uint8_t buf[READ_BUF_LEN];
 
     len = sizeof(buf);
     if (len > s->max_size)
@@ -866,7 +868,7 @@ static void pty_chr_read(void *opaque)
     CharDriverState *chr = opaque;
     PtyCharDriver *s = chr->opaque;
     int size, len;
-    uint8_t buf[1024];
+    uint8_t buf[READ_BUF_LEN];
 
     len = sizeof(buf);
     if (len > s->read_bytes)
@@ -1554,7 +1556,7 @@ static void win_chr_readfile(CharDriverState *chr)
 {
     WinCharState *s = chr->opaque;
     int ret, err;
-    uint8_t buf[1024];
+    uint8_t buf[READ_BUF_LEN];
     DWORD size;
 
     ZeroMemory(&s->orecv, sizeof(s->orecv));
@@ -1760,7 +1762,7 @@ static CharDriverState *qemu_chr_open_win_file_out(QemuOpts *opts)
 
 typedef struct {
     int fd;
-    uint8_t buf[1024];
+    uint8_t buf[READ_BUF_LEN];
     int bufcnt;
     int bufptr;
     int max_size;
@@ -2020,7 +2022,7 @@ static void tcp_chr_read(void *opaque)
 {
     CharDriverState *chr = opaque;
     TCPCharDriver *s = chr->opaque;
-    uint8_t buf[1024];
+    uint8_t buf[READ_BUF_LEN];
     int len, size;
 
     if (!s->connected || s->max_size <= 0)
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

The initial_reset sent to chardevs doesn't do much other than setting
a bool to true. Char devices are interested in the open event and
that gets sent whenever the device is opened.

Moreover, the reset logic breaks as and when qemu's bh scheduling
changes.

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

diff --git a/qemu-char.c b/qemu-char.c
index 1f63019..d78bae3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -128,15 +128,6 @@ void qemu_chr_reset(CharDriverState *s)
     }
 }
 
-void qemu_chr_initial_reset(void)
-{
-    CharDriverState *chr;
-
-    QTAILQ_FOREACH(chr, &chardevs, next) {
-        qemu_chr_reset(chr);
-    }
-}
-
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len)
 {
     return s->chr_write(s, buf, len);
diff --git a/qemu-char.h b/qemu-char.h
index 05fe15d..b420111 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -83,7 +83,6 @@ void qemu_chr_add_handlers(CharDriverState *s,
                            void *opaque);
 int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
 void qemu_chr_reset(CharDriverState *s);
-void qemu_chr_initial_reset(void);
 int qemu_chr_can_read(CharDriverState *s);
 void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
 int qemu_chr_get_msgfd(CharDriverState *s);
diff --git a/vl.c b/vl.c
index e57f58f..5ad4e42 100644
--- a/vl.c
+++ b/vl.c
@@ -5750,7 +5750,6 @@ int main(int argc, char **argv, char **envp)
     }
 
     text_consoles_set_display(display_state);
-    qemu_chr_initial_reset();
 
     for (i = 0; i < MAX_MONITOR_DEVICES; i++) {
         if (monitor_devices[i] && monitor_hds[i]) {
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/3] char: rename qemu_chr_reset to qemu_chr_generic_open

[Amit Shah]

This function sends out the OPENED event to backends that
have drive the chardevs. The 'reset' is now a historical
artifact and we can now just call the function for what it
is.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 console.c   |    2 +-
 hw/baum.c   |    2 +-
 qemu-char.c |   22 +++++++++++-----------
 qemu-char.h |    2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/console.c b/console.c
index 9bbef59..82ddbe4 100644
--- a/console.c
+++ b/console.c
@@ -1384,7 +1384,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds, QemuOpt
     s->t_attrib = s->t_attrib_default;
     text_console_resize(s);
 
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
     if (chr->init)
         chr->init(chr);
 }
diff --git a/hw/baum.c b/hw/baum.c
index 8a12985..fa356ec 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -614,7 +614,7 @@ CharDriverState *chr_baum_init(QemuOpts *opts)
 
     qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
 
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
 
     return chr;
 
diff --git a/qemu-char.c b/qemu-char.c
index d78bae3..5a81e8f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -112,7 +112,7 @@ static void qemu_chr_event(CharDriverState *s, int event)
     s->chr_event(s->handler_opaque, event);
 }
 
-static void qemu_chr_reset_bh(void *opaque)
+static void qemu_chr_generic_open_bh(void *opaque)
 {
     CharDriverState *s = opaque;
     qemu_chr_event(s, CHR_EVENT_OPENED);
@@ -120,10 +120,10 @@ static void qemu_chr_reset_bh(void *opaque)
     s->bh = NULL;
 }
 
-void qemu_chr_reset(CharDriverState *s)
+void qemu_chr_generic_open(CharDriverState *s)
 {
     if (s->bh == NULL) {
-	s->bh = qemu_bh_new(qemu_chr_reset_bh, s);
+	s->bh = qemu_bh_new(qemu_chr_generic_open_bh, s);
 	qemu_bh_schedule(s->bh);
     }
 }
@@ -610,7 +610,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
     chr->chr_update_read_handler = fd_chr_update_read_handler;
     chr->chr_close = fd_chr_close;
 
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
 
     return chr;
 }
@@ -910,7 +910,7 @@ static void pty_chr_state(CharDriverState *chr, int connected)
         qemu_mod_timer(s->timer, qemu_get_clock(rt_clock) + 1000);
     } else {
         if (!s->connected)
-            qemu_chr_reset(chr);
+            qemu_chr_generic_open(chr);
         s->connected = 1;
     }
 }
@@ -1184,7 +1184,7 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
         return NULL;
     }
     chr->chr_ioctl = tty_serial_ioctl;
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
     return chr;
 }
 #else  /* ! __linux__ && ! __sun__ */
@@ -1330,7 +1330,7 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
     chr->chr_close = pp_close;
     chr->opaque = drv;
 
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
 
     return chr;
 }
@@ -1611,7 +1611,7 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
         free(chr);
         return NULL;
     }
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
     return chr;
 }
 
@@ -1711,7 +1711,7 @@ static CharDriverState *qemu_chr_open_win_pipe(QemuOpts *opts)
         free(chr);
         return NULL;
     }
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
     return chr;
 }
 
@@ -1725,7 +1725,7 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
     s->hcom = fd_out;
     chr->opaque = s;
     chr->chr_write = win_chr_write;
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
     return chr;
 }
 
@@ -2052,7 +2052,7 @@ static void tcp_chr_connect(void *opaque)
     s->connected = 1;
     qemu_set_fd_handler2(s->fd, tcp_chr_read_poll,
                          tcp_chr_read, NULL, chr);
-    qemu_chr_reset(chr);
+    qemu_chr_generic_open(chr);
 }
 
 #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
diff --git a/qemu-char.h b/qemu-char.h
index b420111..9957db1 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -82,7 +82,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
                            IOEventHandler *fd_event,
                            void *opaque);
 int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
-void qemu_chr_reset(CharDriverState *s);
+void qemu_chr_generic_open(CharDriverState *s);
 int qemu_chr_can_read(CharDriverState *s);
 void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
 int qemu_chr_get_msgfd(CharDriverState *s);
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Jan Kiszka]

Amit Shah wrote:
> The initial_reset sent to chardevs doesn't do much other than setting
> a bool to true. Char devices are interested in the open event and
> that gets sent whenever the device is opened.

Have you checked with the monitor in all use cases (dedicated & muxed
console, stdio & SDL console, etc.)? It was introduced once to fix a
corner case, I think it's even documented...

Jan

> 
> Moreover, the reset logic breaks as and when qemu's bh scheduling
> changes.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  qemu-char.c |    9 ---------
>  qemu-char.h |    1 -
>  vl.c        |    1 -
>  3 files changed, 0 insertions(+), 11 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 1f63019..d78bae3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -128,15 +128,6 @@ void qemu_chr_reset(CharDriverState *s)
>      }
>  }
>  
> -void qemu_chr_initial_reset(void)
> -{
> -    CharDriverState *chr;
> -
> -    QTAILQ_FOREACH(chr, &chardevs, next) {
> -        qemu_chr_reset(chr);
> -    }
> -}
> -
>  int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len)
>  {
>      return s->chr_write(s, buf, len);
> diff --git a/qemu-char.h b/qemu-char.h
> index 05fe15d..b420111 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -83,7 +83,6 @@ void qemu_chr_add_handlers(CharDriverState *s,
>                             void *opaque);
>  int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
>  void qemu_chr_reset(CharDriverState *s);
> -void qemu_chr_initial_reset(void);
>  int qemu_chr_can_read(CharDriverState *s);
>  void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
>  int qemu_chr_get_msgfd(CharDriverState *s);
> diff --git a/vl.c b/vl.c
> index e57f58f..5ad4e42 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5750,7 +5750,6 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      text_consoles_set_display(display_state);
> -    qemu_chr_initial_reset();
>  
>      for (i = 0; i < MAX_MONITOR_DEVICES; i++) {
>          if (monitor_devices[i] && monitor_hds[i]) {

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> Amit Shah wrote:
> > The initial_reset sent to chardevs doesn't do much other than setting
> > a bool to true. Char devices are interested in the open event and
> > that gets sent whenever the device is opened.
> 
> Have you checked with the monitor in all use cases (dedicated & muxed
> console, stdio & SDL console, etc.)? It was introduced once to fix a

I've checked with -monitor stdio, monitor in SDL and also chardevs using
unix sockets.

I've not tried mux yet; I'll try that and report back. BTW if it ends up
not working with this patch, it'd be broken in the current master as
well.

> corner case, I think it's even documented...

Hm; I'll have to look really deep; haven't found any such thing yet.

		Amit

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> > Amit Shah wrote:
> > > The initial_reset sent to chardevs doesn't do much other than setting
> > > a bool to true. Char devices are interested in the open event and
> > > that gets sent whenever the device is opened.
> > 
> > Have you checked with the monitor in all use cases (dedicated & muxed
> > console, stdio & SDL console, etc.)? It was introduced once to fix a
> 
> I've checked with -monitor stdio, monitor in SDL and also chardevs using
> unix sockets.
> 
> I've not tried mux yet; I'll try that and report back. BTW if it ends up
> not working with this patch, it'd be broken in the current master as
> well.

I tried with:

-chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux

The monitor prompt shows up as does the serial output.

(btw I've also tried closing and opening char devs and that works fine
too)

		Amit

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

Amit Shah wrote:
> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
>> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
>>> Amit Shah wrote:
>>>> The initial_reset sent to chardevs doesn't do much other than setting
>>>> a bool to true. Char devices are interested in the open event and
>>>> that gets sent whenever the device is opened.
>>> Have you checked with the monitor in all use cases (dedicated & muxed
>>> console, stdio & SDL console, etc.)? It was introduced once to fix a
>> I've checked with -monitor stdio, monitor in SDL and also chardevs using
>> unix sockets.
>>
>> I've not tried mux yet; I'll try that and report back. BTW if it ends up
>> not working with this patch, it'd be broken in the current master as
>> well.
> 
> I tried with:
> 
> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
> 
> The monitor prompt shows up as does the serial output.
> 
> (btw I've also tried closing and opening char devs and that works fine
> too)
> 

That sounds good. Then something must have changed since 2970a6c943, do
you see what?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
> Amit Shah wrote:
> > On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
> >> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> >>> Amit Shah wrote:
> >>>> The initial_reset sent to chardevs doesn't do much other than setting
> >>>> a bool to true. Char devices are interested in the open event and
> >>>> that gets sent whenever the device is opened.
> >>> Have you checked with the monitor in all use cases (dedicated & muxed
> >>> console, stdio & SDL console, etc.)? It was introduced once to fix a
> >> I've checked with -monitor stdio, monitor in SDL and also chardevs using
> >> unix sockets.
> >>
> >> I've not tried mux yet; I'll try that and report back. BTW if it ends up
> >> not working with this patch, it'd be broken in the current master as
> >> well.
> > 
> > I tried with:
> > 
> > -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
> > 
> > The monitor prompt shows up as does the serial output.
> > 
> > (btw I've also tried closing and opening char devs and that works fine
> > too)
> 
> That sounds good. Then something must have changed since 2970a6c943, do
> you see what?

I think that depended on the resets being sent. I've now removed the
need for resets altogether.

		Amit

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]

Amit Shah wrote:
> On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
>>>> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
>>>>> Amit Shah wrote:
>>>>>> The initial_reset sent to chardevs doesn't do much other than setting
>>>>>> a bool to true. Char devices are interested in the open event and
>>>>>> that gets sent whenever the device is opened.
>>>>> Have you checked with the monitor in all use cases (dedicated & muxed
>>>>> console, stdio & SDL console, etc.)? It was introduced once to fix a
>>>> I've checked with -monitor stdio, monitor in SDL and also chardevs using
>>>> unix sockets.
>>>>
>>>> I've not tried mux yet; I'll try that and report back. BTW if it ends up
>>>> not working with this patch, it'd be broken in the current master as
>>>> well.
>>> I tried with:
>>>
>>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
>>>
>>> The monitor prompt shows up as does the serial output.
>>>
>>> (btw I've also tried closing and opening char devs and that works fine
>>> too)
>> That sounds good. Then something must have changed since 2970a6c943, do
>> you see what?
> 
> I think that depended on the resets being sent. I've now removed the
> need for resets altogether.

No, this is in fact the reason why we no longer need it:

	9a1e948129 (Introduce contexts for asynchronous callbacks)

As the initial reset of the char device that is marked pending on open
is now no longer consumed by the IDE initialization, we can actually
drop the later regeneration via qemu_chr_initial_reset. I just hope this
stays like it is...

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote:
> Amit Shah wrote:
> > On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
> >> Amit Shah wrote:
> >>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
> >>>> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> >>>>> Amit Shah wrote:
> >>>>>> The initial_reset sent to chardevs doesn't do much other than setting
> >>>>>> a bool to true. Char devices are interested in the open event and
> >>>>>> that gets sent whenever the device is opened.
> >>>>> Have you checked with the monitor in all use cases (dedicated & muxed
> >>>>> console, stdio & SDL console, etc.)? It was introduced once to fix a
> >>>> I've checked with -monitor stdio, monitor in SDL and also chardevs using
> >>>> unix sockets.
> >>>>
> >>>> I've not tried mux yet; I'll try that and report back. BTW if it ends up
> >>>> not working with this patch, it'd be broken in the current master as
> >>>> well.
> >>> I tried with:
> >>>
> >>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
> >>>
> >>> The monitor prompt shows up as does the serial output.
> >>>
> >>> (btw I've also tried closing and opening char devs and that works fine
> >>> too)
> >> That sounds good. Then something must have changed since 2970a6c943, do
> >> you see what?
> > 
> > I think that depended on the resets being sent. I've now removed the
> > need for resets altogether.
> 
> No, this is in fact the reason why we no longer need it:
> 
> 	9a1e948129 (Introduce contexts for asynchronous callbacks)
> 
> As the initial reset of the char device that is marked pending on open
> is now no longer consumed by the IDE initialization, we can actually
> drop the later regeneration via qemu_chr_initial_reset. I just hope this
> stays like it is...

I tested this even on a tree that doesn't have this patch.

I haven't really delved deep to see why this was added earlier -- the
commit log only says very little. Plus my testing with the current tree
works fine so I'm happy to mention these things in the commit log.

		Amit

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2566 bytes --]

Amit Shah wrote:
> On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
>>>> Amit Shah wrote:
>>>>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
>>>>>> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
>>>>>>> Amit Shah wrote:
>>>>>>>> The initial_reset sent to chardevs doesn't do much other than setting
>>>>>>>> a bool to true. Char devices are interested in the open event and
>>>>>>>> that gets sent whenever the device is opened.
>>>>>>> Have you checked with the monitor in all use cases (dedicated & muxed
>>>>>>> console, stdio & SDL console, etc.)? It was introduced once to fix a
>>>>>> I've checked with -monitor stdio, monitor in SDL and also chardevs using
>>>>>> unix sockets.
>>>>>>
>>>>>> I've not tried mux yet; I'll try that and report back. BTW if it ends up
>>>>>> not working with this patch, it'd be broken in the current master as
>>>>>> well.
>>>>> I tried with:
>>>>>
>>>>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
>>>>>
>>>>> The monitor prompt shows up as does the serial output.
>>>>>
>>>>> (btw I've also tried closing and opening char devs and that works fine
>>>>> too)
>>>> That sounds good. Then something must have changed since 2970a6c943, do
>>>> you see what?
>>> I think that depended on the resets being sent. I've now removed the
>>> need for resets altogether.
>> No, this is in fact the reason why we no longer need it:
>>
>> 	9a1e948129 (Introduce contexts for asynchronous callbacks)
>>
>> As the initial reset of the char device that is marked pending on open
>> is now no longer consumed by the IDE initialization, we can actually
>> drop the later regeneration via qemu_chr_initial_reset. I just hope this
>> stays like it is...
> 
> I tested this even on a tree that doesn't have this patch.

The whole things was (and maybe still is) fragile. So you have to take
care what further patches with side effects you include. If you revert
9a1e948129 and remove qemu_chr_initial_reset, you get the old bug back
again, ie. no prompt.

> 
> I haven't really delved deep to see why this was added earlier -- the
> commit log only says very little. Plus my testing with the current tree
> works fine so I'm happy to mention these things in the commit log.

I've nothing against your patch, specifically as it removes an obviously
no longer needed workaround, not a feature. I just want to make sure
that the current behavior is not only there by chance.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

[Amit Shah]

On (Wed) Nov 04 2009 [15:30:21], Jan Kiszka wrote:
> 
> I've nothing against your patch, specifically as it removes an obviously
> no longer needed workaround, not a feature. I just want to make sure
> that the current behavior is not only there by chance.

Sure; I understand that. I'll be looking more at this code to free it
from any surprises.

		Amit