[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [PATCH]fix VMX "xm console" issue



Thanks all for the comments, and I will investigate qemu code and revise the 
Patch. 
>
>Looking at the qemu code to create a PTY device, it's not quite optimal.
>
>For instance, it's not suggested that you pass a name parameter to
>openpty() b/c it's unclear what's a safe size of the buffer.  Instead,
>ptsname() should be called on the slave fd.
>
>I think the right fix is to tcsetattr() a cfmakeraw() on the slave PTY
>fd after openpty() is called in vl.c.  Otherwise, it's quite possible
>(and likely) to get strange buffering behavior depending on the terminal
>attributes that are inherited when ioemu is spawned.
>
>Regards,
>
>Anthony Liguori
>
>Daniel Stekloff wrote:
>
>>There's a problem with xm console and VMX guests, the handshaking isn't
>>exactly right. I can run xm console on the VMX guest in one window - seeing
>>the handshaking being off, run minicom on the /dev/pts/? in another window,
>>and see the original xm console window correct itself so it's usable. Before
>>minicom, you can't interact with the xm console - it doesn't seem to receive
>>the input.
>>
>>I don't know about the patch here... but I've seen some odd things trying to
>>discover what's wrong. I see where xenconsoled sets the cfmakeraw() - but
>>it's like those settings aren't taking. I do an stty -f /dev/pts/? -a and the
>>settings don't match.
>>
>>I've done a stty -f /dev/pts/? -a before and after minicom is invoked and seen
>>the differences. I've tried changing some of the terminos settings to match
>>minicom's in xenconsoled, but it doesn't seem to make a difference or even
>>seem to be set at all.
>>
>>I'm probably doing something wrong.
>>
>>I'll look at this more this week. I started on it last week and was off for
>>the holiday.
>>
>>Thanks,
>>
>>Dan
>>
>>
>>
>>On Tuesday 29 November 2005 06:10, Anthony Liguori wrote:
>>
>>
>>>I'm not sure what this patch does.  cfmakeraw() will already set all of
>>>those flags.  You shouldn't have to set a baud rate on a PTY either.  Is
>>>the real bug that somehow init_term() wasn't being called when it should
>>>have been?
>>>
>>>What flag, in particular, did you need set that wasn't being set by
>>>cfmakeraw()?  xenconsoled sets it's end of the PTY with cfmakeraw() so
>>>if we modify the client to use different termios settings we probably
>>>need to update the server too.
>>>
>>>Regards,
>>>
>>>Anthony Liguori
>>>
>>>Ian Pratt wrote:
>>>
>>>
>>>>>>Please expand upon what this issue with the console client is. Why
>>>>>>hasn't the issue been seen with paravirtualized domains?
>>>>>>
>>>>>>
>>>>>"xm console" in VMX could not work well now, as we know. We
>>>>>can see the output of the console, but it keeps scrolling
>>>>>like hitting enter.
>>>>>When I investigate this bug, I found that the serial
>>>>>parameter has not been set anywhere, which might be the cause
>>>>>of this issue.
>>>>>
>>>>>
>>>>I'd prefer to understand the issue a bit more first/
>>>>
>>>>
>>>>
>>>>>The VMX console model for VMX is somewhat different with
>>>>>domU's. When the serial port is redirect to pty, write the
>>>>>allocated device to xenstore, then xm console can get it,
>>>>>thus xenconsole is independent of xenconsoled in VMX.
>>>>>
>>>>>
>>>>But xenconsole is used by paravirt guests too. What effect does this
>>>>patch have on them?
>>>>
>>>>Ian
>>>>
>>>>
>>>>
>>>>>>What behaviour does the tcsetattr alter, and why not just set that
>>>>>>particular bit?
>>>>>>
>>>>>>
>>>>>I found that Minicom can work well in "xm console" VMX, and
>>>>>borrow minicom's behavior to set some parameter for "xen
>>>>>console" and domU is working well too.
>>>>>
>>>>>
>>>>>
>>>>>>Thanks,
>>>>>>Ian
>>>>>>
>>>>>>
>>>>>>
>>>>>>># HG changeset patch
>>>>>>># User Ping Yu <ping.y.yu@xxxxxxxxx>
>>>>>>># Node ID 71111788840f0dd557d7cfc4e919beb511c0237c
>>>>>>># Parent  8451c65671238604e2678a1f44c2f582ec6ebf96
>>>>>>>Fix an issue for VMX console.
>>>>>>>Add a function to set serial parameter, thus make "xm
>>>>>>>
>>>>>>>
>>>>>console" work
>>>>>
>>>>>
>>>>>
>>>>>>>well both in domU and VMX
>>>>>>>
>>>>>>>Signed-off-by: Ping Yu <ping.y.yu@xxxxxxxxx>
>>>>>>>
>>>>>>>diff -r 8451c6567123 -r 71111788840f tools/console/client/main.c
>>>>>>>--- a/tools/console/client/main.c       Wed Nov 23 19:37:33 2005
>>>>>>>+++ b/tools/console/client/main.c       Mon Nov 28 15:53:17 2005
>>>>>>>@@ -87,6 +87,92 @@
>>>>>>> tcsetattr(fd, TCSAFLUSH, &new_term);  }
>>>>>>>
>>>>>>>+/* Code borrowed from Minicom
>>>>>>>+ * Set baudrate, parity and number of bits  */ static void
>>>>>>>+set_serial_argument(int fd, char *baudr, char *par,
>>>>>>>+                char *bits, char *stopb, int hwf, int swf) {
>>>>>>>+        int spd = -1;
>>>>>>>+        int newbaud;
>>>>>>>+        int bit = bits[0];
>>>>>>>+
>>>>>>>+        struct termios tty;
>>>>>>>+
>>>>>>>+        tcgetattr(fd, &tty);
>>>>>>>+
>>>>>>>+          /* We generate mark and space parity ourself. */
>>>>>>>+        if (bit == '7' && (par[0] == 'M' || par[0] == 'S'))
>>>>>>>+                bit = '8';
>>>>>>>+
>>>>>>>+         /* Check if 'baudr' is really a number */
>>>>>>>+        if ((newbaud = (atol(baudr) / 100)) == 0 && baudr[0] != '0')
>>>>>>>+                newbaud = -1;
>>>>>>>+
>>>>>>>+        switch(newbaud) {
>>>>>>>+        case 0:
>>>>>>>+        case 3:         spd = B300;     break;
>>>>>>>+        case 6:         spd = B600;     break;
>>>>>>>+        case 12:        spd = B1200;    break;
>>>>>>>+        case 24:        spd = B2400;    break;
>>>>>>>+        case 48:        spd = B4800;    break;
>>>>>>>+        case 96:        spd = B9600;    break;
>>>>>>>+        case 192:       spd = B19200;   break;
>>>>>>>+        case 384:       spd = B38400;   break;
>>>>>>>+        case 576:       spd = B57600;   break;
>>>>>>>+        case 1152:      spd = B115200;  break;
>>>>>>>+        case 2304:      spd = B230400;  break;
>>>>>>>+        }
>>>>>>>+
>>>>>>>+        if (spd != -1) {
>>>>>>>+                cfsetospeed(&tty, (speed_t)spd);
>>>>>>>+                cfsetispeed(&tty, (speed_t)spd);
>>>>>>>+        }
>>>>>>>+
>>>>>>>+        switch (bit) {
>>>>>>>+        case '5':
>>>>>>>+                tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS5;
>>>>>>>+                break;
>>>>>>>+        case '6':
>>>>>>>+                tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS6;
>>>>>>>+                break;
>>>>>>>+        case '7':
>>>>>>>+                tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS7;
>>>>>>>+                break;
>>>>>>>+        case '8':
>>>>>>>+        default:
>>>>>>>+                tty.c_cflag = (tty.c_cflag & ~CSIZE) | CS8;
>>>>>>>+                break;
>>>>>>>+        }
>>>>>>>+
>>>>>>>+          /* Set into raw, no echo mode */
>>>>>>>+        tty.c_iflag =  IGNBRK;
>>>>>>>+        tty.c_lflag = 0;
>>>>>>>+        tty.c_oflag = 0;
>>>>>>>+        tty.c_cflag |= CLOCAL | CREAD;
>>>>>>>+        tty.c_cc[VMIN] = 1;
>>>>>>>+        tty.c_cc[VTIME] = 5;
>>>>>>>+
>>>>>>>+        if (swf)
>>>>>>>+                tty.c_iflag |= IXON | IXOFF;
>>>>>>>+        else
>>>>>>>+                tty.c_iflag &= ~(IXON|IXOFF|IXANY);
>>>>>>>+
>>>>>>>+        tty.c_cflag &= ~(PARENB | PARODD);
>>>>>>>+        if (par[0] == 'E')
>>>>>>>+                tty.c_cflag |= PARENB;
>>>>>>>+        else if (par[0] == 'O')
>>>>>>>+                tty.c_cflag |= (PARENB | PARODD);
>>>>>>>+
>>>>>>>+        if (stopb[0] == '2')
>>>>>>>+                tty.c_cflag |= CSTOPB;
>>>>>>>+        else
>>>>>>>+                tty.c_cflag &= ~CSTOPB;
>>>>>>>+
>>>>>>>+        tcsetattr(fd, TCSANOW, &tty);
>>>>>>>+
>>>>>>>+}
>>>>>>>+
>>>>>>>static void restore_term(int fd, struct termios *old)  {
>>>>>>> tcsetattr(fd, TCSAFLUSH, old);
>>>>>>>@@ -260,6 +346,7 @@
>>>>>>> free(path);
>>>>>>>
>>>>>>> init_term(STDIN_FILENO, &attr);
>>>>>>>+        set_serial_argument(spty, "115200", "N", "8", "1", 1, 0);
>>>>>>> console_loop(xc_handle, domid, spty);
>>>>>>> restore_term(STDIN_FILENO, &attr);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>Sincerely Yours
>>>>>>>
>>>>>>>Yu Ping
>>>>>>>
>>>>>>>
>>>>_______________________________________________
>>>>Xen-devel mailing list
>>>>Xen-devel@xxxxxxxxxxxxxxxxxxx
>>>>http://lists.xensource.com/xen-devel
>>>>
>>>>
>>>_______________________________________________
>>>Xen-devel mailing list
>>>Xen-devel@xxxxxxxxxxxxxxxxxxx
>>>http://lists.xensource.com/xen-devel
>>>
>>>
>>
>>
>>


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.