PATCH [xenconsoled]: makes pty slave raw early

PATCH [xenconsoled]: makes pty slave raw early

[tgingold]

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

Hi,

on my system (Linux 2.6.18.8 - ia64), if a domain write on the xencons
before xenconsole is initialized the domain gets back what it wrote.

This patch fixes this issue by making raw the pty slave very early.

(I suppose it doesn't happen with linux as a guest because it takes a little
bit of time before writing to xencons).

Tristan.

[-- Attachment #2: cons.patch --]
[-- Type: text/plain, Size: 1219 bytes --]

# HG changeset patch
# User Tristan Gingold <tgingold@free.fr>
# Date 1200461143 -3600
# Node ID 0353ead264baea1eac7fb35c622c69f45f3935df
# Parent  ca2828a46217bec2096144d2bc96b1d610272f30
Make slave pty raw during initialization.
(This avoids echo).

Signed-off-by: Tristan Gingold <tgingold@free.fr>

diff -r ca2828a46217 -r 0353ead264ba tools/console/daemon/io.c
--- a/tools/console/daemon/io.c	Wed Jan 16 06:21:50 2008 +0100
+++ b/tools/console/daemon/io.c	Wed Jan 16 06:25:43 2008 +0100
@@ -278,8 +278,10 @@ static int openpty(int *amaster, int *as
 	if (winp)
 		ioctl(sfd, TIOCSWINSZ, winp);
 
+	if (termp)
+		tcsetattr(sfd, TCSAFLUSH, termp);
+
 	assert(name == NULL);
-	assert(termp == NULL);
 
 	return 0;
 
@@ -299,11 +301,14 @@ static int domain_create_tty(struct doma
 	bool success;
 	char *data;
 	unsigned int len;
+	struct termios term;
 
 	assert(dom->slave_fd == -1);
 	assert(dom->master_fd == -1);
 
-	if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
+	cfmakeraw(&term);
+
+	if (openpty(&dom->master_fd, &dom->slave_fd, NULL, &term, NULL) < 0) {
 		err = errno;
 		dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, %s)",
 		      dom->domid, err, strerror(err));

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: PATCH [xenconsoled]: makes pty slave raw early

[Samuel Thibault]

tgingold@free.fr, le Wed 16 Jan 2008 06:30:04 +0100, a écrit :
> on my system (Linux 2.6.18.8 - ia64), if a domain write on the xencons
> before xenconsole is initialized the domain gets back what it wrote.

Argh, indeed, and it seems to fix things for mini-os as well.  John,
could you please check that it works correctly on Solaris before Keir
applies it?
(From the information of the thread we had in December, it should).

Thanks,
Samuel

Re: PATCH [xenconsoled]: makes pty slave raw early

[John Levon]

On Wed, Jan 16, 2008 at 10:57:07AM +0000, Samuel Thibault wrote:

> > on my system (Linux 2.6.18.8 - ia64), if a domain write on the xencons
> > before xenconsole is initialized the domain gets back what it wrote.
> 
> Argh, indeed, and it seems to fix things for mini-os as well.  John,
> could you please check that it works correctly on Solaris before Keir
> applies it?

Works fine. Below is the version of the patch that compiles on Solaris.

cheers
john

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -246,7 +246,6 @@ static void domain_close_tty(struct doma
 }
 
 #ifdef __sun__
-/* Once Solaris has openpty(), this is going to be removed. */
 static int openpty(int *amaster, int *aslave, char *name,
                    struct termios *termp, struct winsize *winp)
 {
@@ -278,8 +277,10 @@ static int openpty(int *amaster, int *as
 	if (winp)
 		ioctl(sfd, TIOCSWINSZ, winp);
 
+	if (termp)
+		tcsetattr(sfd, TCSAFLUSH, termp);
+
 	assert(name == NULL);
-	assert(termp == NULL);
 
 	return 0;
 
@@ -289,7 +290,20 @@ err:
 	close(mfd);
 	return -1;
 }
-#endif
+
+void cfmakeraw(struct termios *termios_p)
+{
+	termios_p->c_iflag &=
+	    ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
+	termios_p->c_oflag &= ~OPOST;
+	termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
+	termios_p->c_cflag &= ~(CSIZE|PARENB);
+	termios_p->c_cflag |= CS8;
+
+	termios_p->c_cc[VMIN] = 0;
+	termios_p->c_cc[VTIME] = 0;
+}
+#endif /* __sun__ */
 
 static int domain_create_tty(struct domain *dom)
 {
@@ -299,11 +313,14 @@ static int domain_create_tty(struct doma
 	bool success;
 	char *data;
 	unsigned int len;
+	struct termios term;
 
 	assert(dom->slave_fd == -1);
 	assert(dom->master_fd == -1);
 
-	if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
+	cfmakeraw(&term);
+
+	if (openpty(&dom->master_fd, &dom->slave_fd, NULL, &term, NULL) < 0) {
 		err = errno;
 		dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, %s)",
 		      dom->domid, err, strerror(err));

Re: PATCH [xenconsoled]: makes pty slave raw early

[Keir Fraser]

Is this plausible to slip in for 3.2.0?

 -- Keir

On 16/1/08 12:20, "John Levon" <levon@movementarian.org> wrote:

> On Wed, Jan 16, 2008 at 10:57:07AM +0000, Samuel Thibault wrote:
> 
>>> on my system (Linux 2.6.18.8 - ia64), if a domain write on the xencons
>>> before xenconsole is initialized the domain gets back what it wrote.
>> 
>> Argh, indeed, and it seems to fix things for mini-os as well.  John,
>> could you please check that it works correctly on Solaris before Keir
>> applies it?
> 
> Works fine. Below is the version of the patch that compiles on Solaris.
> 
> cheers
> john
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -246,7 +246,6 @@ static void domain_close_tty(struct doma
>  }
>  
>  #ifdef __sun__
> -/* Once Solaris has openpty(), this is going to be removed. */
>  static int openpty(int *amaster, int *aslave, char *name,
>                     struct termios *termp, struct winsize *winp)
>  {
> @@ -278,8 +277,10 @@ static int openpty(int *amaster, int *as
> if (winp)
> ioctl(sfd, TIOCSWINSZ, winp);
>  
> + if (termp)
> +  tcsetattr(sfd, TCSAFLUSH, termp);
> +
> assert(name == NULL);
> - assert(termp == NULL);
>  
> return 0;
>  
> @@ -289,7 +290,20 @@ err:
> close(mfd);
> return -1;
>  }
> -#endif
> +
> +void cfmakeraw(struct termios *termios_p)
> +{
> + termios_p->c_iflag &=
> +     ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
> + termios_p->c_oflag &= ~OPOST;
> + termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
> + termios_p->c_cflag &= ~(CSIZE|PARENB);
> + termios_p->c_cflag |= CS8;
> +
> + termios_p->c_cc[VMIN] = 0;
> + termios_p->c_cc[VTIME] = 0;
> +}
> +#endif /* __sun__ */
>  
>  static int domain_create_tty(struct domain *dom)
>  {
> @@ -299,11 +313,14 @@ static int domain_create_tty(struct doma
> bool success;
> char *data;
> unsigned int len;
> + struct termios term;
>  
> assert(dom->slave_fd == -1);
> assert(dom->master_fd == -1);
>  
> - if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
> + cfmakeraw(&term);
> +
> + if (openpty(&dom->master_fd, &dom->slave_fd, NULL, &term, NULL) < 0) {
> err = errno;
> dolog(LOG_ERR, "Failed to create tty for domain-%d (errno = %i, %s)",
>      dom->domid, err, strerror(err));
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: PATCH [xenconsoled]: makes pty slave raw early

[Samuel Thibault]

Keir Fraser, le Wed 16 Jan 2008 13:20:53 +0000, a écrit :
> Is this plausible to slip in for 3.2.0?

Yes.

Samuel

Re: PATCH [xenconsoled]: makes pty slave raw early

[tgingold]

Quoting John Levon <levon@movementarian.org>:

> On Wed, Jan 16, 2008 at 10:57:07AM +0000, Samuel Thibault wrote:
>
> > > on my system (Linux 2.6.18.8 - ia64), if a domain write on the xencons
> > > before xenconsole is initialized the domain gets back what it wrote.
> >
> > Argh, indeed, and it seems to fix things for mini-os as well.  John,
> > could you please check that it works correctly on Solaris before Keir
> > applies it?
>
> Works fine. Below is the version of the patch that compiles on Solaris.

Thanks.  I tried to do my best from Solaris, but I forgot cfmakeraw.

Tristan.

Re: PATCH [xenconsoled]: makes pty slave raw early

[John Levon]

On Wed, Jan 16, 2008 at 06:30:04AM +0100, tgingold@free.fr wrote:

> diff -r ca2828a46217 -r 0353ead264ba tools/console/daemon/io.c
> --- a/tools/console/daemon/io.c	Wed Jan 16 06:21:50 2008 +0100
> +++ b/tools/console/daemon/io.c	Wed Jan 16 06:25:43 2008 +0100
> @@ -278,8 +278,10 @@ static int openpty(int *amaster, int *as
>  	if (winp)
>  		ioctl(sfd, TIOCSWINSZ, winp);
>  
> +	if (termp)
> +		tcsetattr(sfd, TCSAFLUSH, termp);
> +

This is wrong. openpty() only retrieves term settings, not sets it. You
must do the cfmakeraw after openpty().

Also, if you're re-introducing cfmakeraw() call, you need to include
this:

#ifdef  __sun__
void cfmakeraw (struct termios *termios_p)
{
        termios_p->c_iflag &=
            ~(IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
        termios_p->c_oflag &= ~OPOST;
        termios_p->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
        termios_p->c_cflag &= ~(CSIZE|PARENB);
        termios_p->c_cflag |= CS8;

        termios_p->c_cc[VMIN] = 0;
        termios_p->c_cc[VTIME] = 0;
}
#endif

Re: PATCH [xenconsoled]: makes pty slave raw early

[Samuel Thibault]

John Levon, le Wed 16 Jan 2008 10:58:53 +0000, a écrit :
> On Wed, Jan 16, 2008 at 06:30:04AM +0100, tgingold@free.fr wrote:
> 
> > diff -r ca2828a46217 -r 0353ead264ba tools/console/daemon/io.c
> > --- a/tools/console/daemon/io.c	Wed Jan 16 06:21:50 2008 +0100
> > +++ b/tools/console/daemon/io.c	Wed Jan 16 06:25:43 2008 +0100
> > @@ -278,8 +278,10 @@ static int openpty(int *amaster, int *as
> >  	if (winp)
> >  		ioctl(sfd, TIOCSWINSZ, winp);
> >  
> > +	if (termp)
> > +		tcsetattr(sfd, TCSAFLUSH, termp);
> > +
> 
> This is wrong. openpty() only retrieves term settings, not sets it.

Please re-read manpages:

« If termp is not NULL, the terminal parameters of the slave will be
set to the values in termp. »

And in glibc the termp structure recently got const.

Samuel

Re: PATCH [xenconsoled]: makes pty slave raw early

[John Levon]

On Wed, Jan 16, 2008 at 11:06:17AM +0000, Samuel Thibault wrote:

> Please re-read manpages:
> 
> « If termp is not NULL, the terminal parameters of the slave will be
> set to the values in termp. »

Yep, I got it totally the wrong way around (sorry). I'll try the patch

regards
john