From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frank van der Linden Subject: Re: [PATCH] pygrub: make it work Date: Thu, 19 Feb 2009 11:16:36 -0700 Message-ID: <499DA204.5080408@Sun.COM> References: <200902191048.24789.Christoph.Egger@amd.com> <18845.39159.846108.995200@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=ISO-8859-1 Content-Transfer-Encoding: 7BIT Return-path: In-reply-to: <18845.39159.846108.995200@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: Christoph Egger , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org Ian Jackson wrote: > Surely this dependence on uname is wrong. Why would set the termios > flags on the pty master ? They should be set on the slave > unconditionally. It's been a while since I originally did this patch, so my memory is a little rusty, but as far as I remember, I simply wanted to avoid breaking Linux after an earlier version of the patch did so. I tested the final version of the patch on Linux, and it worked fine. It could be that the uname tests aren't needed anymore. > > And keeping the slave fd around for a while is harmless too. So I > guess I'm asking why these changes need to be conditional. > >> + # filedescriptors: >> + # r - input from the bootloader (bootstring output) >> + # m1 - input/output from/to xenconsole >> + # m2 - input/output from/to pty that controls the bootloader > > Can you explain what the purpose of this new code is or relevantly how > it differs from the old (ie, what was wrong with the old code) ? The comments I added explain it: + # The filedescriptors are NDELAY, so it's ok to try to read + # bigger chunks than may be available, to keep e.g. curses + # screen redraws in the bootloader efficient. m1 is the side that + # gets xenconsole input, which will be keystrokes, so a small number + # is sufficient. m2 is pygrub output, which will be curses screen + # updates, so a larger number (1024) is appropriate there. + # + # For writeable descriptors, only include them in the set for select + # if there is actual data to write, otherwise this would loop too fast, + # eating up CPU time. - Frank