| Message ID | 20100220083052.GA3582@redhat.com |
|---|---|
| State | New |
| Headers | show |
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__ */
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__ */
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__ */ >> > > >
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__ */ > > > > > > >
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
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
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.
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__ */
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(-)