[Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Shahar Havivi]

when exiting qemu that run with "-monitor /dev/tty", the launching
terminal get weird behaviour because no restore terminals action has
taken.
added chr_close and register atexit() code for tty devices (like stdio
does)

Signed-off-by: Shahar Havivi <shaharh@redhat.com>
---
 qemu-char.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 75dbf66..de16883 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
            speed, parity, data_bits, stop_bits);
 #endif
     tcgetattr (fd, &tty);
+    oldtty = tty;
 
 #define check_speed(val) if (speed <= val) { spd = B##val; break; }
     speed = speed * 10 / 11;
@@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
     return 0;
 }
 
+static void tty_exit(void)
+{
+    tcsetattr(0, TCSANOW, &oldtty);
+}
+
+static void qemu_chr_close_tty(struct CharDriverState *chr)
+{
+    tty_exit();
+    fd_chr_close(chr);
+}
+
 static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 {
     const char *filename = qemu_opt_get(opts, "path");
@@ -1190,6 +1202,8 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
         return NULL;
     }
     chr->chr_ioctl = tty_serial_ioctl;
+    chr->chr_close = qemu_chr_close_tty;
+    atexit(tty_exit);
     return chr;
 }
 #else  /* ! __linux__ && ! __sun__ */
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[David S. Ahern]

On 02/20/2010 01:30 AM, Shahar Havivi wrote:
> when exiting qemu that run with "-monitor /dev/tty", the launching
> terminal get weird behaviour because no restore terminals action has
> taken.
> added chr_close and register atexit() code for tty devices (like stdio
> does)
> 
> Signed-off-by: Shahar Havivi <shaharh@redhat.com>
> ---
>  qemu-char.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 75dbf66..de16883 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
>             speed, parity, data_bits, stop_bits);
>  #endif
>      tcgetattr (fd, &tty);
> +    oldtty = tty;
>  
>  #define check_speed(val) if (speed <= val) { spd = B##val; break; }
>      speed = speed * 10 / 11;
> @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
>      return 0;
>  }
>  
> +static void tty_exit(void)
> +{
> +    tcsetattr(0, TCSANOW, &oldtty);
> +}
> +
> +static void qemu_chr_close_tty(struct CharDriverState *chr)
> +{
> +    tty_exit();
> +    fd_chr_close(chr);
> +}


The close callback needs to close the fd for the device as well. I have
sent a patch to handle this; waiting for it to be included:

http://permalink.gmane.org/gmane.comp.emulators.qemu/63472

David


> +
>  static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>  {
>      const char *filename = qemu_opt_get(opts, "path");
> @@ -1190,6 +1202,8 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>          return NULL;
>      }
>      chr->chr_ioctl = tty_serial_ioctl;
> +    chr->chr_close = qemu_chr_close_tty;
> +    atexit(tty_exit);
>      return chr;
>  }
>  #else  /* ! __linux__ && ! __sun__ */

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Shahar Havivi]

On Sat, Feb 20, 2010 at 08:18:54AM -0700, David S. Ahern wrote:
> Date: Sat, 20 Feb 2010 08:18:54 -0700
> From: "David S. Ahern" <daahern@cisco.com>
> To: Shahar Havivi <shaharh@redhat.com>
> CC: qemu-devel@nongnu.org, Dor Laor <dlaor@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH] terminal attributes is not restored when
>  using /dev/tty monitor
> 
> 
> On 02/20/2010 01:30 AM, Shahar Havivi wrote:
> > when exiting qemu that run with "-monitor /dev/tty", the launching
> > terminal get weird behaviour because no restore terminals action has
> > taken.
> > added chr_close and register atexit() code for tty devices (like stdio
> > does)
> > 
> > Signed-off-by: Shahar Havivi <shaharh@redhat.com>
> > ---
> >  qemu-char.c |   14 ++++++++++++++
> >  1 files changed, 14 insertions(+), 0 deletions(-)
> > 
> > diff --git a/qemu-char.c b/qemu-char.c
> > index 75dbf66..de16883 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
> >             speed, parity, data_bits, stop_bits);
> >  #endif
> >      tcgetattr (fd, &tty);
> > +    oldtty = tty;
> >  
> >  #define check_speed(val) if (speed <= val) { spd = B##val; break; }
> >      speed = speed * 10 / 11;
> > @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> >      return 0;
> >  }
> >  
> > +static void tty_exit(void)
> > +{
> > +    tcsetattr(0, TCSANOW, &oldtty);
> > +}
> > +
> > +static void qemu_chr_close_tty(struct CharDriverState *chr)
> > +{
> > +    tty_exit();
> > +    fd_chr_close(chr);
> > +}
> 
> 
> The close callback needs to close the fd for the device as well. I have
> sent a patch to handle this; waiting for it to be included:
> 
> http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
> 
> David
Sure you right about the fd but the chr->chr_close not called all the
time that is why I added the atexit() call, and the restore tty flags.
Shaahr.
> 
> 
> > +
> >  static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >  {
> >      const char *filename = qemu_opt_get(opts, "path");
> > @@ -1190,6 +1202,8 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >          return NULL;
> >      }
> >      chr->chr_ioctl = tty_serial_ioctl;
> > +    chr->chr_close = qemu_chr_close_tty;
> > +    atexit(tty_exit);
> >      return chr;
> >  }
> >  #else  /* ! __linux__ && ! __sun__ */

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Anthony Liguori]

On 02/20/2010 09:18 AM, David S. Ahern wrote:
> On 02/20/2010 01:30 AM, Shahar Havivi wrote:
>    
>> when exiting qemu that run with "-monitor /dev/tty", the launching
>> terminal get weird behaviour because no restore terminals action has
>> taken.
>> added chr_close and register atexit() code for tty devices (like stdio
>> does)
>>
>> Signed-off-by: Shahar Havivi<shaharh@redhat.com>
>> ---
>>   qemu-char.c |   14 ++++++++++++++
>>   1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 75dbf66..de16883 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
>>              speed, parity, data_bits, stop_bits);
>>   #endif
>>       tcgetattr (fd,&tty);
>> +    oldtty = tty;
>>
>>   #define check_speed(val) if (speed<= val) { spd = B##val; break; }
>>       speed = speed * 10 / 11;
>> @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
>>       return 0;
>>   }
>>
>> +static void tty_exit(void)
>> +{
>> +    tcsetattr(0, TCSANOW,&oldtty);
>> +}
>> +
>> +static void qemu_chr_close_tty(struct CharDriverState *chr)
>> +{
>> +    tty_exit();
>> +    fd_chr_close(chr);
>> +}
>>      
>
> The close callback needs to close the fd for the device as well. I have
> sent a patch to handle this; waiting for it to be included:
>
> http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
>    

It didn't apply with git-am.  I'm not sure why, am investigating now.

Regards,

Anthony Liguori

> David
>
>
>    
>> +
>>   static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>>   {
>>       const char *filename = qemu_opt_get(opts, "path");
>> @@ -1190,6 +1202,8 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>>           return NULL;
>>       }
>>       chr->chr_ioctl = tty_serial_ioctl;
>> +    chr->chr_close = qemu_chr_close_tty;
>> +    atexit(tty_exit);
>>       return chr;
>>   }
>>   #else  /* ! __linux__&&  ! __sun__ */
>>      
>
>
>    

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Shahar Havivi]

On Sat, Feb 20, 2010 at 11:03:41AM -0600, Anthony Liguori wrote:
> Date: Sat, 20 Feb 2010 11:03:41 -0600
> From: Anthony Liguori <anthony@codemonkey.ws>
> To: "David S. Ahern" <daahern@cisco.com>
> Cc: Dor Laor <dlaor@redhat.com>, Shahar Havivi <shaharh@redhat.com>,
> 	qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH] terminal attributes is not restored when
> 	using /dev/tty monitor
> 
> On 02/20/2010 09:18 AM, David S. Ahern wrote:
> >On 02/20/2010 01:30 AM, Shahar Havivi wrote:
> >>when exiting qemu that run with "-monitor /dev/tty", the launching
> >>terminal get weird behaviour because no restore terminals action has
> >>taken.
> >>added chr_close and register atexit() code for tty devices (like stdio
> >>does)
> >>
> >>Signed-off-by: Shahar Havivi<shaharh@redhat.com>
> >>---
> >>  qemu-char.c |   14 ++++++++++++++
> >>  1 files changed, 14 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/qemu-char.c b/qemu-char.c
> >>index 75dbf66..de16883 100644
> >>--- a/qemu-char.c
> >>+++ b/qemu-char.c
> >>@@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
> >>             speed, parity, data_bits, stop_bits);
> >>  #endif
> >>      tcgetattr (fd,&tty);
> >>+    oldtty = tty;
> >>
> >>  #define check_speed(val) if (speed<= val) { spd = B##val; break; }
> >>      speed = speed * 10 / 11;
> >>@@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> >>      return 0;
> >>  }
> >>
> >>+static void tty_exit(void)
> >>+{
> >>+    tcsetattr(0, TCSANOW,&oldtty);
> >>+}
> >>+
> >>+static void qemu_chr_close_tty(struct CharDriverState *chr)
> >>+{
> >>+    tty_exit();
> >>+    fd_chr_close(chr);
> >>+}
> >
> >The close callback needs to close the fd for the device as well. I have
> >sent a patch to handle this; waiting for it to be included:
> >
> >http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
> 
> It didn't apply with git-am.  I'm not sure why, am investigating now.
> 
> Regards,
> 
> Anthony Liguori
> 
Note that the method fd_chr_close() is closing the fd_in, no need to the
close logic again, and when opening a monitor with /dev/tty the
chr->chr_close not called that is why you need to register with
atexit(). (same as stdio monitor does).
Shahar.
> >David
> >
> >
> >>+
> >>  static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >>  {
> >>      const char *filename = qemu_opt_get(opts, "path");
> >>@@ -1190,6 +1202,8 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >>          return NULL;
> >>      }
> >>      chr->chr_ioctl = tty_serial_ioctl;
> >>+    chr->chr_close = qemu_chr_close_tty;
> >>+    atexit(tty_exit);
> >>      return chr;
> >>  }
> >>  #else  /* ! __linux__&&  ! __sun__ */
> >
> >
> 
> 
> 

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[David S. Ahern]

On 02/20/2010 10:03 AM, Anthony Liguori wrote:
> On 02/20/2010 09:18 AM, David S. Ahern wrote:
>> On 02/20/2010 01:30 AM, Shahar Havivi wrote:
>>   
>>> when exiting qemu that run with "-monitor /dev/tty", the launching
>>> terminal get weird behaviour because no restore terminals action has
>>> taken.
>>> added chr_close and register atexit() code for tty devices (like stdio
>>> does)
>>>
>>> Signed-off-by: Shahar Havivi<shaharh@redhat.com>
>>> ---
>>>   qemu-char.c |   14 ++++++++++++++
>>>   1 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index 75dbf66..de16883 100644
>>> --- a/qemu-char.c
>>> +++ b/qemu-char.c
>>> @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
>>>              speed, parity, data_bits, stop_bits);
>>>   #endif
>>>       tcgetattr (fd,&tty);
>>> +    oldtty = tty;
>>>
>>>   #define check_speed(val) if (speed<= val) { spd = B##val; break; }
>>>       speed = speed * 10 / 11;
>>> @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState
>>> *chr, int cmd, void *arg)
>>>       return 0;
>>>   }
>>>
>>> +static void tty_exit(void)
>>> +{
>>> +    tcsetattr(0, TCSANOW,&oldtty);
>>> +}
>>> +
>>> +static void qemu_chr_close_tty(struct CharDriverState *chr)
>>> +{
>>> +    tty_exit();
>>> +    fd_chr_close(chr);
>>> +}
>>>      
>>
>> The close callback needs to close the fd for the device as well. I have
>> sent a patch to handle this; waiting for it to be included:
>>
>> http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
>>    
> 
> It didn't apply with git-am.  I'm not sure why, am investigating now.
> 
> Regards,
> 
> Anthony Liguori
> 

Are you referring to my patch? I'm still learning the git commands, so
maybe I messed something up. I used git format-patch followed git send-mail.

If I save the email (Thunderbird client, saving the copy I received of
what I sent using git send-mail), strip out the mail headers and use
patch it applies fine - but with the warning "(Stripping trailing CRs
from patch.)"

David

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[David S. Ahern]

On 02/20/2010 12:42 PM, Shahar Havivi wrote:
> On Sat, Feb 20, 2010 at 11:03:41AM -0600, Anthony Liguori wrote:
>> Date: Sat, 20 Feb 2010 11:03:41 -0600
>> From: Anthony Liguori <anthony@codemonkey.ws>
>> To: "David S. Ahern" <daahern@cisco.com>
>> Cc: Dor Laor <dlaor@redhat.com>, Shahar Havivi <shaharh@redhat.com>,
>> 	qemu-devel@nongnu.org
>> Subject: Re: [Qemu-devel] [PATCH] terminal attributes is not restored when
>> 	using /dev/tty monitor
>>
>> On 02/20/2010 09:18 AM, David S. Ahern wrote:
>>> On 02/20/2010 01:30 AM, Shahar Havivi wrote:
>>>> when exiting qemu that run with "-monitor /dev/tty", the launching
>>>> terminal get weird behaviour because no restore terminals action has
>>>> taken.
>>>> added chr_close and register atexit() code for tty devices (like stdio
>>>> does)
>>>>
>>>> Signed-off-by: Shahar Havivi<shaharh@redhat.com>
>>>> ---
>>>>  qemu-char.c |   14 ++++++++++++++
>>>>  1 files changed, 14 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/qemu-char.c b/qemu-char.c
>>>> index 75dbf66..de16883 100644
>>>> --- a/qemu-char.c
>>>> +++ b/qemu-char.c
>>>> @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
>>>>             speed, parity, data_bits, stop_bits);
>>>>  #endif
>>>>      tcgetattr (fd,&tty);
>>>> +    oldtty = tty;
>>>>
>>>>  #define check_speed(val) if (speed<= val) { spd = B##val; break; }
>>>>      speed = speed * 10 / 11;
>>>> @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
>>>>      return 0;
>>>>  }
>>>>
>>>> +static void tty_exit(void)
>>>> +{
>>>> +    tcsetattr(0, TCSANOW,&oldtty);
>>>> +}
>>>> +
>>>> +static void qemu_chr_close_tty(struct CharDriverState *chr)
>>>> +{
>>>> +    tty_exit();
>>>> +    fd_chr_close(chr);
>>>> +}
>>>
>>> The close callback needs to close the fd for the device as well. I have
>>> sent a patch to handle this; waiting for it to be included:
>>>
>>> http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
>>
>> It didn't apply with git-am.  I'm not sure why, am investigating now.
>>
>> Regards,
>>
>> Anthony Liguori
>>
> Note that the method fd_chr_close() is closing the fd_in, no need to the
> close logic again, and when opening a monitor with /dev/tty the
> chr->chr_close not called that is why you need to register with
> atexit(). (same as stdio monitor does).
> Shahar.

I don't see that fd_chr_close() closes the fd; it only unregisters the
handler.

David

Re: [Qemu-devel] [PATCH] terminal attributes is not restored when using /dev/tty monitor

[Shahar Havivi]

On Sun, Feb 21, 2010 at 07:32:41AM -0700, David S. Ahern wrote:
> Date: Sun, 21 Feb 2010 07:32:41 -0700
> From: "David S. Ahern" <daahern@cisco.com>
> To: Shahar Havivi <shaharh@redhat.com>
> CC: Anthony Liguori <anthony@codemonkey.ws>, Dor Laor <dlaor@redhat.com>,
> 	qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH] terminal attributes is not restored when
>  using /dev/tty monitor
> 
> 
> 
> On 02/20/2010 12:42 PM, Shahar Havivi wrote:
> > On Sat, Feb 20, 2010 at 11:03:41AM -0600, Anthony Liguori wrote:
> >> Date: Sat, 20 Feb 2010 11:03:41 -0600
> >> From: Anthony Liguori <anthony@codemonkey.ws>
> >> To: "David S. Ahern" <daahern@cisco.com>
> >> Cc: Dor Laor <dlaor@redhat.com>, Shahar Havivi <shaharh@redhat.com>,
> >> 	qemu-devel@nongnu.org
> >> Subject: Re: [Qemu-devel] [PATCH] terminal attributes is not restored when
> >> 	using /dev/tty monitor
> >>
> >> On 02/20/2010 09:18 AM, David S. Ahern wrote:
> >>> On 02/20/2010 01:30 AM, Shahar Havivi wrote:
> >>>> when exiting qemu that run with "-monitor /dev/tty", the launching
> >>>> terminal get weird behaviour because no restore terminals action has
> >>>> taken.
> >>>> added chr_close and register atexit() code for tty devices (like stdio
> >>>> does)
> >>>>
> >>>> Signed-off-by: Shahar Havivi<shaharh@redhat.com>
> >>>> ---
> >>>>  qemu-char.c |   14 ++++++++++++++
> >>>>  1 files changed, 14 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/qemu-char.c b/qemu-char.c
> >>>> index 75dbf66..de16883 100644
> >>>> --- a/qemu-char.c
> >>>> +++ b/qemu-char.c
> >>>> @@ -1002,6 +1002,7 @@ static void tty_serial_init(int fd, int speed,
> >>>>             speed, parity, data_bits, stop_bits);
> >>>>  #endif
> >>>>      tcgetattr (fd,&tty);
> >>>> +    oldtty = tty;
> >>>>
> >>>>  #define check_speed(val) if (speed<= val) { spd = B##val; break; }
> >>>>      speed = speed * 10 / 11;
> >>>> @@ -1173,6 +1174,17 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> >>>>      return 0;
> >>>>  }
> >>>>
> >>>> +static void tty_exit(void)
> >>>> +{
> >>>> +    tcsetattr(0, TCSANOW,&oldtty);
> >>>> +}
> >>>> +
> >>>> +static void qemu_chr_close_tty(struct CharDriverState *chr)
> >>>> +{
> >>>> +    tty_exit();
> >>>> +    fd_chr_close(chr);
> >>>> +}
> >>>
> >>> The close callback needs to close the fd for the device as well. I have
> >>> sent a patch to handle this; waiting for it to be included:
> >>>
> >>> http://permalink.gmane.org/gmane.comp.emulators.qemu/63472
> >>
> >> It didn't apply with git-am.  I'm not sure why, am investigating now.
> >>
> >> Regards,
> >>
> >> Anthony Liguori
> >>
> > Note that the method fd_chr_close() is closing the fd_in, no need to the
> > close logic again, and when opening a monitor with /dev/tty the
> > chr->chr_close not called that is why you need to register with
> > atexit(). (same as stdio monitor does).
> > Shahar.
> 
> I don't see that fd_chr_close() closes the fd; it only unregisters the
> handler.
> 
> David
you right, my bad.