Patchwork [3/8] qemu-char: Add fe_open tracking

login
register
mail settings
Submitter Hans de Goede
Date March 24, 2013, 12:39 p.m.
Message ID <1364128793-12689-4-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/230445/
State New
Headers show

Comments

Hans de Goede - March 24, 2013, 12:39 p.m.
Add tracking of the fe_open state to struct CharDriverState.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/char/char.h | 1 +
 qemu-char.c         | 2 ++
 2 files changed, 3 insertions(+)
Anthony Liguori - March 25, 2013, 12:45 p.m.
Hans de Goede <hdegoede@redhat.com> writes:

> Add tracking of the fe_open state to struct CharDriverState.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  include/char/char.h | 1 +
>  qemu-char.c         | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/char/char.h b/include/char/char.h
> index dd8f39a..3174575 100644
> --- a/include/char/char.h
> +++ b/include/char/char.h
> @@ -75,6 +75,7 @@ struct CharDriverState {
>      char *label;
>      char *filename;
>      int be_open;
> +    int fe_open;
>      int avail_connections;
>      QemuOpts *opts;
>      QTAILQ_ENTRY(CharDriverState) next;
> diff --git a/qemu-char.c b/qemu-char.c
> index 55795d7..554d72f 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3385,6 +3385,7 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo)
>  
>  void qemu_chr_fe_open(struct CharDriverState *chr)
>  {
> +    chr->fe_open = 1;
>      if (chr->chr_guest_open) {
>          chr->chr_guest_open(chr);
>      }
> @@ -3392,6 +3393,7 @@ void qemu_chr_fe_open(struct CharDriverState *chr)
>  
>  void qemu_chr_fe_close(struct CharDriverState *chr)
>  {
> +    chr->fe_open = 0;

Even though this gets rewritten later on, you should avoid calling the
callback when open is called when fe_open=1.  Something like

if (!chr->fe_open) {
    return;
}

Then later when this becomes set_fe_open() the backend doesn't need to
deal with double open/close.

Regards,

Anthony Liguori

>      if (chr->chr_guest_close) {
>          chr->chr_guest_close(chr);
>      }
> -- 
> 1.8.1.4
Hans de Goede - March 25, 2013, 1:07 p.m.
Hi,

On 03/25/2013 01:45 PM, Anthony Liguori wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>
>> Add tracking of the fe_open state to struct CharDriverState.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   include/char/char.h | 1 +
>>   qemu-char.c         | 2 ++
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/include/char/char.h b/include/char/char.h
>> index dd8f39a..3174575 100644
>> --- a/include/char/char.h
>> +++ b/include/char/char.h
>> @@ -75,6 +75,7 @@ struct CharDriverState {
>>       char *label;
>>       char *filename;
>>       int be_open;
>> +    int fe_open;
>>       int avail_connections;
>>       QemuOpts *opts;
>>       QTAILQ_ENTRY(CharDriverState) next;
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 55795d7..554d72f 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -3385,6 +3385,7 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo)
>>
>>   void qemu_chr_fe_open(struct CharDriverState *chr)
>>   {
>> +    chr->fe_open = 1;
>>       if (chr->chr_guest_open) {
>>           chr->chr_guest_open(chr);
>>       }
>> @@ -3392,6 +3393,7 @@ void qemu_chr_fe_open(struct CharDriverState *chr)
>>
>>   void qemu_chr_fe_close(struct CharDriverState *chr)
>>   {
>> +    chr->fe_open = 0;
>
> Even though this gets rewritten later on, you should avoid calling the
> callback when open is called when fe_open=1.  Something like
>
> if (!chr->fe_open) {
>      return;
> }
>
> Then later when this becomes set_fe_open() the backend doesn't need to
> deal with double open/close.

Ok, will fix in the next iteration of this patchset.

Regards,

Hans

Patch

diff --git a/include/char/char.h b/include/char/char.h
index dd8f39a..3174575 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -75,6 +75,7 @@  struct CharDriverState {
     char *label;
     char *filename;
     int be_open;
+    int fe_open;
     int avail_connections;
     QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
diff --git a/qemu-char.c b/qemu-char.c
index 55795d7..554d72f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3385,6 +3385,7 @@  void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo)
 
 void qemu_chr_fe_open(struct CharDriverState *chr)
 {
+    chr->fe_open = 1;
     if (chr->chr_guest_open) {
         chr->chr_guest_open(chr);
     }
@@ -3392,6 +3393,7 @@  void qemu_chr_fe_open(struct CharDriverState *chr)
 
 void qemu_chr_fe_close(struct CharDriverState *chr)
 {
+    chr->fe_open = 0;
     if (chr->chr_guest_close) {
         chr->chr_guest_close(chr);
     }