[Qemu-devel] [PATCH v2 0/3] Send out OPENED events only on chardev open

[Qemu-devel] [PATCH v2 0/3] Send out OPENED events only on chardev open

[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 and made sure the events are emitted
only when a char dev is actually opened.

v2:
- Introduce a per-CharDriverState initial_reset_sent state bool that
  tracks whether the reset is for an init reset or an open event


Amit Shah (3):
  char: check for initial_reset_issued unnecessary
  char: rename CHR_EVENT_RESET to CHR_EVENT_OPENED
  char: emit the OPENED event only when a new char connection is opened

 gdbstub.c       |    2 +-
 hw/baum.c       |    2 +-
 hw/usb-serial.c |    2 +-
 monitor.c       |    2 +-
 qemu-char.c     |   12 +++++++-----
 qemu-char.h     |    4 +++-
 6 files changed, 14 insertions(+), 10 deletions(-)

[Qemu-devel] [PATCH v2 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.

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

diff --git a/qemu-char.c b/qemu-char.c
index 8084a67..8f7f81c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -108,7 +108,6 @@
 
 static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
     QTAILQ_HEAD_INITIALIZER(chardevs);
-static int initial_reset_issued;
 
 static void qemu_chr_event(CharDriverState *s, int event)
 {
@@ -127,7 +126,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,8 +136,6 @@ void qemu_chr_initial_reset(void)
 {
     CharDriverState *chr;
 
-    initial_reset_issued = 1;
-
     QTAILQ_FOREACH(chr, &chardevs, next) {
         qemu_chr_reset(chr);
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH v2 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 8f7f81c..4757689 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -119,7 +119,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 v2 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 |    7 ++++++-
 qemu-char.h |    2 ++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 4757689..0fd402c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -119,7 +119,12 @@ 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 (s->initial_reset_issued) {
+        qemu_chr_event(s, CHR_EVENT_OPENED);
+    } else {
+        s->initial_reset_issued = true;
+    }
     qemu_bh_delete(s->bh);
     s->bh = NULL;
 }
diff --git a/qemu-char.h b/qemu-char.h
index 05fe15d..409961d 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -1,6 +1,7 @@
 #ifndef QEMU_CHAR_H
 #define QEMU_CHAR_H
 
+#include <stdbool.h>
 #include "qemu-common.h"
 #include "qemu-queue.h"
 #include "qemu-option.h"
@@ -66,6 +67,7 @@ struct CharDriverState {
     QEMUBH *bh;
     char *label;
     char *filename;
+    bool initial_reset_issued;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.6.2.5

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

[Jan Kiszka]

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

Amit Shah wrote:
> 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.

The monitor was. Now its initial prompt on activation is broken.

Does this patch fix/improve something for a different user? If not,
please let us revert it.

Jan

> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  qemu-char.c |    7 ++++++-
>  qemu-char.h |    2 ++
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 4757689..0fd402c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -119,7 +119,12 @@ 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 (s->initial_reset_issued) {
> +        qemu_chr_event(s, CHR_EVENT_OPENED);
> +    } else {
> +        s->initial_reset_issued = true;
> +    }
>      qemu_bh_delete(s->bh);
>      s->bh = NULL;
>  }
> diff --git a/qemu-char.h b/qemu-char.h
> index 05fe15d..409961d 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -1,6 +1,7 @@
>  #ifndef QEMU_CHAR_H
>  #define QEMU_CHAR_H
>  
> +#include <stdbool.h>
>  #include "qemu-common.h"
>  #include "qemu-queue.h"
>  #include "qemu-option.h"
> @@ -66,6 +67,7 @@ struct CharDriverState {
>      QEMUBH *bh;
>      char *label;
>      char *filename;
> +    bool initial_reset_issued;
>      QTAILQ_ENTRY(CharDriverState) next;
>  };
>  




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

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

[Amit Shah]

On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote:
> Amit Shah wrote:
> > 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.
> 
> The monitor was. Now its initial prompt on activation is broken.

The patch in Anthony's queue, titled

'console: call qemu_chr_reset() in text_console_init'

fixed that.

However, with the qcow2 synchronous patch, the monitor prompt doesn't
come up again -- which shows there is a problem with the way the bhs
work and also the initial resets.

I think the initial resets are a hack to work around something from my
reading of it; do you have a better idea of why it's there and how it's
all supposed to work?

> Does this patch fix/improve something for a different user? If not,
> please let us revert it.

There's another question too: is a separate 'reset' event needed in
addition to an 'opened' event?

I have a few apps (that are coming as part of the virtio-console work)
that need just an 'opened' event and are not interested in the 'reset'
event.

		Amit

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

[Jan Kiszka]

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

Amit Shah wrote:
> On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> 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.
>> The monitor was. Now its initial prompt on activation is broken.
> 
> The patch in Anthony's queue, titled
> 
> 'console: call qemu_chr_reset() in text_console_init'

You may also want to rename qemu_chr_reset - unless there is still a
need for real "reset".

> 
> fixed that.
> 
> However, with the qcow2 synchronous patch, the monitor prompt doesn't
> come up again -- which shows there is a problem with the way the bhs
> work and also the initial resets.

Then the qcow2 patch is already in? At least applying your patch doesn't
change the picture.

> 
> I think the initial resets are a hack to work around something from my
> reading of it; do you have a better idea of why it's there and how it's
> all supposed to work?

From the monitor's POV, it's not a hack, it's simply the requirement to
receive an indication that the console was opened.

> 
>> Does this patch fix/improve something for a different user? If not,
>> please let us revert it.
> 
> There's another question too: is a separate 'reset' event needed in
> addition to an 'opened' event?

Not for the monitor, but I cannot speak for other users. I think it
would be good to check them in details before changing the reset/open
semantic.

> 
> I have a few apps (that are coming as part of the virtio-console work)
> that need just an 'opened' event and are not interested in the 'reset'
> event.
> 
> 		Amit

Jan


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

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

[Amit Shah]

On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote:
> Amit Shah wrote:
> > On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote:
> >> Amit Shah wrote:
> >>> 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.
> >> The monitor was. Now its initial prompt on activation is broken.
> > 
> > The patch in Anthony's queue, titled
> > 
> > 'console: call qemu_chr_reset() in text_console_init'
> 
> You may also want to rename qemu_chr_reset - unless there is still a
> need for real "reset".

Yeah; there isn't a need after my patches -- I've been slowing working
towards renaming it all.

> > 
> > fixed that.
> > 
> > However, with the qcow2 synchronous patch, the monitor prompt doesn't
> > come up again -- which shows there is a problem with the way the bhs
> > work and also the initial resets.
> 
> Then the qcow2 patch is already in? At least applying your patch doesn't
> change the picture.

Yeah; that patch went in just before my patches. Can you try reverting

ef845c3bf421290153154635dc18eaa677cecb43

> > I think the initial resets are a hack to work around something from my
> > reading of it; do you have a better idea of why it's there and how it's
> > all supposed to work?
> 
> From the monitor's POV, it's not a hack, it's simply the requirement to
> receive an indication that the console was opened.

Just an indication that the monitor was opened -- agreed. But git
history shows you added that as 'reset', so I'm wondering if maybe you
wanted it to do something else as well (or you did it that way just
because of the way qemu's bhs are handled?).

> >> Does this patch fix/improve something for a different user? If not,
> >> please let us revert it.
> > 
> > There's another question too: is a separate 'reset' event needed in
> > addition to an 'opened' event?
> 
> Not for the monitor, but I cannot speak for other users. I think it
> would be good to check them in details before changing the reset/open
> semantic.

As far as I could see in the git history, the 'reset' was added for the
monitor. And the others could live with the double 'reset' events they
were getting -- one for the reset and one when the device was opened.

		Amit

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

[Jan Kiszka]

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

Amit Shah wrote:
> On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote:
>>>> Amit Shah wrote:
>>>>> 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.
>>>> The monitor was. Now its initial prompt on activation is broken.
>>> The patch in Anthony's queue, titled
>>>
>>> 'console: call qemu_chr_reset() in text_console_init'
>> You may also want to rename qemu_chr_reset - unless there is still a
>> need for real "reset".
> 
> Yeah; there isn't a need after my patches -- I've been slowing working
> towards renaming it all.
> 
>>> fixed that.
>>>
>>> However, with the qcow2 synchronous patch, the monitor prompt doesn't
>>> come up again -- which shows there is a problem with the way the bhs
>>> work and also the initial resets.
>> Then the qcow2 patch is already in? At least applying your patch doesn't
>> change the picture.
> 
> Yeah; that patch went in just before my patches. Can you try reverting
> 
> ef845c3bf421290153154635dc18eaa677cecb43

Makes no difference either - but it also does not touch any code that
code impact the open/reset signaling.

> 
>>> I think the initial resets are a hack to work around something from my
>>> reading of it; do you have a better idea of why it's there and how it's
>>> all supposed to work?
>> From the monitor's POV, it's not a hack, it's simply the requirement to
>> receive an indication that the console was opened.
> 
> Just an indication that the monitor was opened -- agreed. But git
> history shows you added that as 'reset', so I'm wondering if maybe you
> wanted it to do something else as well (or you did it that way just
> because of the way qemu's bhs are handled?).

I didn't add the reset hook for the monitor (it was Anthony), I just
made some improvements.

> 
>>>> Does this patch fix/improve something for a different user? If not,
>>>> please let us revert it.
>>> There's another question too: is a separate 'reset' event needed in
>>> addition to an 'opened' event?
>> Not for the monitor, but I cannot speak for other users. I think it
>> would be good to check them in details before changing the reset/open
>> semantic.
> 
> As far as I could see in the git history, the 'reset' was added for the
> monitor. And the others could live with the double 'reset' events they
> were getting -- one for the reset and one when the device was opened.	

OK.

However, the problems of your approach to avoid potential double resets
on startup is that the supposed two events for the monitor (first one
from qemu_chr_open_fs, second one via qemu_chr_initial_reset) actually
coalesce into one, thus are never delivered. So whatever may happen
later on, during startup the skipping of the first reset/open event is
bogus.

Jan


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

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

[Amit Shah]

On (Mon) Oct 26 2009 [21:15:57], Jan Kiszka wrote:
> >>> However, with the qcow2 synchronous patch, the monitor prompt doesn't
> >>> come up again -- which shows there is a problem with the way the bhs
> >>> work and also the initial resets.
> >> Then the qcow2 patch is already in? At least applying your patch doesn't
> >> change the picture.
> > 
> > Yeah; that patch went in just before my patches. Can you try reverting
> > 
> > ef845c3bf421290153154635dc18eaa677cecb43
> 
> Makes no difference either - but it also does not touch any code that
> code impact the open/reset signaling.

What happens is the BHs that are run get run in a different order.

Let me explain the two scenarios:

1. Current qemu tree, plus my patch to fix this issue that's in
Anthony's queue plus reverting the qcow2 patch -- the monitor prompt
appears fine.

2. Current qemu tree plus my patch from Anthony's queue - the monitor
prompt doesn't appear.

The difference is in the order the BHs are scheduled. In the case
without the qcow2 patch, the bhs get scheduled early on and results in
initial_reset_issued getting set. Later, when the char devs are
initialsed, the OPENED event is sent out.

In the case with the qcow2 patch, the bhs are run in a different order
-- the bhs are scheduled but not run, and when the char init happens,
the condition

if (s->bh == NULL)

is false and the bhs in effect get scheduled only once, not emitting the
opened event.

Of course, the qcow2 patch just causes some conditions for the bh
handling to happen differently which I've not examined yet. I just
tracked why this was happening.

All that said, I'm ok with reverting that patch now till I find some
kind of a solution to this.

		Amit

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

[Kevin Wolf]

Am 27.10.2009 08:46, schrieb Amit Shah:
> On (Mon) Oct 26 2009 [21:15:57], Jan Kiszka wrote:
>>>>> However, with the qcow2 synchronous patch, the monitor prompt doesn't
>>>>> come up again -- which shows there is a problem with the way the bhs
>>>>> work and also the initial resets.
>>>> Then the qcow2 patch is already in? At least applying your patch doesn't
>>>> change the picture.
>>>
>>> Yeah; that patch went in just before my patches. Can you try reverting
>>>
>>> ef845c3bf421290153154635dc18eaa677cecb43
>>
>> Makes no difference either - but it also does not touch any code that
>> code impact the open/reset signaling.
> 
> What happens is the BHs that are run get run in a different order.
> 
> Let me explain the two scenarios:
> 
> 1. Current qemu tree, plus my patch to fix this issue that's in
> Anthony's queue plus reverting the qcow2 patch -- the monitor prompt
> appears fine.

Try it with no disk or with a disk in a format other than qcow2. It's
still broken. With a qcow2 image (and the qcow2 patch reverted), two
bugs just cancel each other out.

If you really need to run BHs during initialization (which doesn't
really sound like the clean solution), call qemu_bh_poll() manually.

> 2. Current qemu tree plus my patch from Anthony's queue - the monitor
> prompt doesn't appear.
> 
> The difference is in the order the BHs are scheduled. In the case
> without the qcow2 patch, the bhs get scheduled early on and results in
> initial_reset_issued getting set. Later, when the char devs are
> initialsed, the OPENED event is sent out.
> 
> In the case with the qcow2 patch, the bhs are run in a different order
> -- the bhs are scheduled but not run, and when the char init happens,
> the condition
> 
> if (s->bh == NULL)
> 
> is false and the bhs in effect get scheduled only once, not emitting the
> opened event.
> 
> Of course, the qcow2 patch just causes some conditions for the bh
> handling to happen differently which I've not examined yet. I just
> tracked why this was happening.
> 
> All that said, I'm ok with reverting that patch now till I find some
> kind of a solution to this.

Which patch do you want to revert? You're aware that the qcow2 patch is
a data corruption fix?

Kevin

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

[Amit Shah]

On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote:
> > 
> > All that said, I'm ok with reverting that patch now till I find some
> > kind of a solution to this.
> 
> Which patch do you want to revert? You're aware that the qcow2 patch is
> a data corruption fix?

Ah, no. Reverting my patch that causes this problem. I know the qcow2
patch only exposes the bh handling issue. I intend to fix that
appropriately elsewhere :-)

		Amit

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

[Anthony Liguori]

Amit Shah wrote:
> On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote:
>   
>>> All that said, I'm ok with reverting that patch now till I find some
>>> kind of a solution to this.
>>>       
>> Which patch do you want to revert? You're aware that the qcow2 patch is
>> a data corruption fix?
>>     
>
> Ah, no. Reverting my patch that causes this problem. I know the qcow2
> patch only exposes the bh handling issue. I intend to fix that
> appropriately elsewhere :-)
>   

How does Kevin's latest patches that introduces new BHs semantics affect 
all of this?

-- 
Regards,

Anthony Liguori

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

[Amit Shah]

On (Tue) Oct 27 2009 [09:04:50], Anthony Liguori wrote:
> Amit Shah wrote:
>> On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote:
>>   
>>>> All that said, I'm ok with reverting that patch now till I find some
>>>> kind of a solution to this.
>>>>       
>>> Which patch do you want to revert? You're aware that the qcow2 patch is
>>> a data corruption fix?
>>>     
>>
>> Ah, no. Reverting my patch that causes this problem. I know the qcow2
>> patch only exposes the bh handling issue. I intend to fix that
>> appropriately elsewhere :-)
>>   
>
> How does Kevin's latest patches that introduces new BHs semantics affect  
> all of this?

I explained that a couple of mails ago in this thread in a reply to Jan.
Basically, the order in which the BHs run causes the problem, so it's a
race, and Kevin's patch make the BHs run later.

		Amit

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

[Kevin Wolf]

Am 27.10.2009 15:04, schrieb Anthony Liguori:
> Amit Shah wrote:
>> On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote:
>>   
>>>> All that said, I'm ok with reverting that patch now till I find some
>>>> kind of a solution to this.
>>>>       
>>> Which patch do you want to revert? You're aware that the qcow2 patch is
>>> a data corruption fix?
>>>     
>>
>> Ah, no. Reverting my patch that causes this problem. I know the qcow2
>> patch only exposes the bh handling issue. I intend to fix that
>> appropriately elsewhere :-)
>>   
> 
> How does Kevin's latest patches that introduces new BHs semantics affect 
> all of this?

I'd expect that it will make the bug visible it in the very same way as
the qcow2 workaround currently does. In both cases we don't run BHs that
the monitor/char code relies on to be run early.

Kevin