diff mbox

Restore terminal attributes for tty based monitor

Message ID 20100226200414.GA25277@redhat.com
State New
Headers show

Commit Message

Shahar Havivi Feb. 26, 2010, 8:04 p.m. UTC 
Fix codding style
---
 qemu-char.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

--
1.6.3.3

Comments

David S. Ahern Feb. 26, 2010, 10:19 p.m. UTC | #1 
On 02/26/2010 01:04 PM, Shahar Havivi wrote:
> Fix codding style
> ---
>  qemu-char.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 4169492..4533887 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,11 @@ 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(CharDriverState *chr)
>  {
>      FDCharDriver *s = chr->opaque;
> @@ -1207,6 +1213,9 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>      }
>      chr->chr_ioctl = tty_serial_ioctl;
>      chr->chr_close = qemu_chr_close_tty;
> +    if (!term_atexit_done++) {
> +        atexit(tty_exit);
> +    }
>      return chr;
>  }
>  #else  /* ! __linux__ && ! __sun__ */
> --
> 1.6.3.3
> 

If qemu is invoked with both stdio and one or more host tty's only the
last one referenced is reset one exit. Also, shouldn't the attributes be
reset when the device is closed as opposed to exit? ie., a device
connected to a tty chardev is removed via the monitor.

David
Shahar Havivi Feb. 27, 2010, 8:41 a.m. UTC | #2 
On Fri, Feb 26, 2010 at 03:19:19PM -0700, David S. Ahern wrote:
> Date: Fri, 26 Feb 2010 15:19:19 -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] Restore terminal attributes for tty based
>  monitor
> 
> 
> 
> 
> 
> On 02/26/2010 01:04 PM, Shahar Havivi wrote:
> > Fix codding style
> > ---
> >  qemu-char.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> > 
> > diff --git a/qemu-char.c b/qemu-char.c
> > index 4169492..4533887 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,11 @@ 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(CharDriverState *chr)
> >  {
> >      FDCharDriver *s = chr->opaque;
> > @@ -1207,6 +1213,9 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >      }
> >      chr->chr_ioctl = tty_serial_ioctl;
> >      chr->chr_close = qemu_chr_close_tty;
> > +    if (!term_atexit_done++) {
> > +        atexit(tty_exit);
> > +    }
> >      return chr;
> >  }
> >  #else  /* ! __linux__ && ! __sun__ */
> > --
> > 1.6.3.3
> > 
> 
> If qemu is invoked with both stdio and one or more host tty's only the
> last one referenced is reset one exit. Also, shouldn't the attributes be
> reset when the device is closed as opposed to exit? ie., a device
> connected to a tty chardev is removed via the monitor.
> 
> David
stdio have the same handling code:
http://git.savannah.gnu.org/cgit/qemu.git/tree/qemu-char.c#n738

Shahar
David S. Ahern Feb. 27, 2010, 4:20 p.m. UTC | #3 
On 02/27/2010 01:41 AM, Shahar Havivi wrote:
> On Fri, Feb 26, 2010 at 03:19:19PM -0700, David S. Ahern wrote:
>> Date: Fri, 26 Feb 2010 15:19:19 -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] Restore terminal attributes for tty based
>>  monitor
>>
>>
>>
>>
>>
>> On 02/26/2010 01:04 PM, Shahar Havivi wrote:
>>> Fix codding style
>>> ---
>>>  qemu-char.c |    9 +++++++++
>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index 4169492..4533887 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,11 @@ 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(CharDriverState *chr)
>>>  {
>>>      FDCharDriver *s = chr->opaque;
>>> @@ -1207,6 +1213,9 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>>>      }
>>>      chr->chr_ioctl = tty_serial_ioctl;
>>>      chr->chr_close = qemu_chr_close_tty;
>>> +    if (!term_atexit_done++) {
>>> +        atexit(tty_exit);
>>> +    }
>>>      return chr;
>>>  }
>>>  #else  /* ! __linux__ && ! __sun__ */
>>> --
>>> 1.6.3.3
>>>
>>
>> If qemu is invoked with both stdio and one or more host tty's only the
>> last one referenced is reset one exit. Also, shouldn't the attributes be
>> reset when the device is closed as opposed to exit? ie., a device
>> connected to a tty chardev is removed via the monitor.
>>
>> David
> stdio have the same handling code:
> http://git.savannah.gnu.org/cgit/qemu.git/tree/qemu-char.c#n738
> 
> Shahar

Right now stdio is the only user of the oldtty global, so it holds the
atributes for stdio. If you reuse the variable you are setting the tty
(ttyN or ttySN) to the attributes retrieved from stdin - assuming it was
even used.

I think you want to save the attributes for the terminal that is in use
and reset that terminal on close / exit.

David
Shahar Havivi March 1, 2010, 12:19 p.m. UTC | #4 
On Sat, Feb 27, 2010 at 09:20:28AM -0700, David S. Ahern wrote:
> Date: Sat, 27 Feb 2010 09:20:28 -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] Restore terminal attributes for tty based
>  monitor
> 
> 
> 
> 
> 
> On 02/27/2010 01:41 AM, Shahar Havivi wrote:
> > On Fri, Feb 26, 2010 at 03:19:19PM -0700, David S. Ahern wrote:
> >> Date: Fri, 26 Feb 2010 15:19:19 -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] Restore terminal attributes for tty based
> >>  monitor
> >>
> >>
> >>
> >>
> >>
> >> On 02/26/2010 01:04 PM, Shahar Havivi wrote:
> >>> Fix codding style
> >>> ---
> >>>  qemu-char.c |    9 +++++++++
> >>>  1 files changed, 9 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/qemu-char.c b/qemu-char.c
> >>> index 4169492..4533887 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,11 @@ 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(CharDriverState *chr)
> >>>  {
> >>>      FDCharDriver *s = chr->opaque;
> >>> @@ -1207,6 +1213,9 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> >>>      }
> >>>      chr->chr_ioctl = tty_serial_ioctl;
> >>>      chr->chr_close = qemu_chr_close_tty;
> >>> +    if (!term_atexit_done++) {
> >>> +        atexit(tty_exit);
> >>> +    }
> >>>      return chr;
> >>>  }
> >>>  #else  /* ! __linux__ && ! __sun__ */
> >>> --
> >>> 1.6.3.3
> >>>
> >>
> >> If qemu is invoked with both stdio and one or more host tty's only the
> >> last one referenced is reset one exit. Also, shouldn't the attributes be
> >> reset when the device is closed as opposed to exit? ie., a device
> >> connected to a tty chardev is removed via the monitor.
> >>
> >> David
> > stdio have the same handling code:
> > http://git.savannah.gnu.org/cgit/qemu.git/tree/qemu-char.c#n738
> > 
> > Shahar
> 
> Right now stdio is the only user of the oldtty global, so it holds the
> atributes for stdio. If you reuse the variable you are setting the tty
> (ttyN or ttySN) to the attributes retrieved from stdin - assuming it was
> even used.
> 
> I think you want to save the attributes for the terminal that is in use
> and reset that terminal on close / exit.
The oldtty global can be reuse, as far for the ttnN and ttySN you right,
it may be wrong for them, I will recheck.
Shahar.
> 
> David
diff mbox

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 4169492..4533887 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,11 @@  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(CharDriverState *chr)
 {
     FDCharDriver *s = chr->opaque;
@@ -1207,6 +1213,9 @@  static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
     }
     chr->chr_ioctl = tty_serial_ioctl;
     chr->chr_close = qemu_chr_close_tty;
+    if (!term_atexit_done++) {
+        atexit(tty_exit);
+    }
     return chr;
 }
 #else  /* ! __linux__ && ! __sun__ */