Michal Novotny
2010-Aug-30 13:10 UTC
[Xen-devel] [PATCH] Fix bootloader handling when empty string is being output
Hi,
this is the patch to fix empty string as the output value
of the bootloader string. If there is no output then
xend is being hung indefinitely unless you press Ctrl + C keys
to trigger SIGINT. This patch fixes the issue by implementing
timeout to the select() function for reading from pipe and also
checking for state of the process - i.e. whether the process is
running or whether it''s dead or zombie (because fork() call does
leave zombies when the process is done but the parent process is
still running).
I tried it with the bogus bootloader that just returns with
error code 0 and also the bogus bootloader that sleeps for 10
seconds and then returns with error code 0. According to my
testing when the bootloader process has finished and the output
of the process is empty it fails with "bootloader didn''t return
any data" message which is the expected behaviour. This patch
has also been tested with the various timeout values (incl. no
timeout specified) for pyGrub and everything was working fine
since it was failing *only* in the case both output from pyGrub
was empty and the bootloader process was not running according
the pid.
The check for bootloader running is implemented by checking
the presence of /proc/{$PID} and also state in the stat file
(/proc/{$PID}/stat).
Testing: The patch has been tested on RHEL-5.5 x86_64 dom0 with
RHEL-5.4 PV guest both with good and bad bootloader setup. By bad
bootloader I mean just the shell script exiting with code 0 and
also shell script sleeping for 10 seconds and exiting with code 0.
Before my patch applied it got stuck indefinitely in the xend
because of waiting for some data on select() call and after my
patch applied it''s opening the FIFO with O_NDELAY and tries to
read the data indefinitely every second using the select() call
with 1 second as the timeout argument. If there were no data
output using the read call it just fails after the child
(bootloader) process exited. Otherwise, when bootloader did
return some data, the guest started successfully no matter
what the timeout was defined in pyGrub.
Signed-off-by: Michal Novotny <minovotn@redhat.com>
Michal
--
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Aug-31 08:47 UTC
Re: [Xen-devel] [PATCH] Fix bootloader handling when empty string is being output
On Mon, 2010-08-30 at 14:10 +0100, Michal Novotny wrote:> Hi, > this is the patch to fix empty string as the output value > of the bootloader string. If there is no output then > xend is being hung indefinitely unless you press Ctrl + C keys > to trigger SIGINT.I think a similar fix will be needed to libxl_bootloader.c, right? A couple of questions: Couldn''t the non-portable usage of /proc be portably replaced with something like waitpid+WNOHANG? I guess it would be sane to put one of these inside the loop which performs the bootloader interaction to check the bootloader still exists. Could we potentially avoid the need to use a select timeout to poll for bootloader exit by including the bootloader FIFO FD and/or the PTY FDs in the select''s exceptfds array? Presumably if the process on the other end of such an FD exits that causes some sort of exceptional condition (although historically I''ve had trouble finding the actual specified behaviour in cases like this). Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Paolo Bonzini
2010-Aug-31 09:16 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 10:47 AM, Ian Campbell wrote:> On Mon, 2010-08-30 at 14:10 +0100, Michal Novotny wrote: >> Hi, >> this is the patch to fix empty string as the output value >> of the bootloader string. If there is no output then >> xend is being hung indefinitely unless you press Ctrl + C keys >> to trigger SIGINT. > > I think a similar fix will be needed to libxl_bootloader.c, right?Yes.> Could we potentially avoid the need to use a select timeout to poll for > bootloader exit by including the bootloader FIFO FD and/or the PTY FDs > in the select''s exceptfds array? Presumably if the process on the other > end of such an FD exits that causes some sort of exceptional condition > (although historically I''ve had trouble finding the actual specified > behaviour in cases like this).Using the FIFO is not possible in general because you cannot be sure that the bootloader is opening it at all. In fact, in this case both libxl and xend will hang at "fifo_fd = open(fifo, O_RDONLY);" so you have to apply O_NDELAY already when you open the FIFO. Also, in the case the bootloader is not opening the FIFO at all, doing the waitpid in the same thread has a race if the process exits between the waitpid and select. Fixing the race requires pselect and, especially in Python, introduces more complications than it removes. So, for RHEL5 xend, Michal has a better version of the patch that will run the waitpid in a separate thread, and use a pipe to wake up the select. This does the same thing as /proc, but portably and without the need for polling. However, for both current xend and libxl, using the PTY sounds better, and it looks like an even simpler patch will work that: - uses O_RDONLY|O_NDELAY when opening the FIFO, and - exits bootloader_interact when bootloader_fd was in the returned rsel but ret == 0. Paolo _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 10:00 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 11:16 AM, Paolo Bonzini wrote:> On 08/31/2010 10:47 AM, Ian Campbell wrote: >> On Mon, 2010-08-30 at 14:10 +0100, Michal Novotny wrote: >>> Hi, >>> this is the patch to fix empty string as the output value >>> of the bootloader string. If there is no output then >>> xend is being hung indefinitely unless you press Ctrl + C keys >>> to trigger SIGINT. >> >> I think a similar fix will be needed to libxl_bootloader.c, right? > > Yes. > >> Could we potentially avoid the need to use a select timeout to poll for >> bootloader exit by including the bootloader FIFO FD and/or the PTY FDs >> in the select''s exceptfds array? Presumably if the process on the other >> end of such an FD exits that causes some sort of exceptional condition >> (although historically I''ve had trouble finding the actual specified >> behaviour in cases like this). > > Using the FIFO is not possible in general because you cannot be sure > that the bootloader is opening it at all. In fact, in this case both > libxl and xend will hang at "fifo_fd = open(fifo, O_RDONLY);" so you > have to apply O_NDELAY already when you open the FIFO. > > Also, in the case the bootloader is not opening the FIFO at all, doing > the waitpid in the same thread has a race if the process exits between > the waitpid and select. Fixing the race requires pselect and, > especially in Python, introduces more complications than it removes. > > So, for RHEL5 xend, Michal has a better version of the patch that will > run the waitpid in a separate thread, and use a pipe to wake up the > select. This does the same thing as /proc, but portably and without > the need for polling. > > However, for both current xend and libxl, using the PTY sounds better, > and it looks like an even simpler patch will work that: > > - uses O_RDONLY|O_NDELAY when opening the FIFO, and > > - exits bootloader_interact when bootloader_fd was in the returned > rsel but ret == 0. > > PaoloSo, Paolo, what do you recommend for upstream version? There''s the PTY thing already so what should we do ? Ian, also, I don''t know how it''s working with upstream version since I found out that syntax like `xm create -c PVguest` with default settings (pyGrub bootloader) doesn''t show the pyGrub at all so I don''t know what''s wrong with my setup. I''m using 2.6.32.15-xen kernel/hypervisor version with latest unstable user-space tools. Any hint how this should be working Ian? Thanks, Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Aug-31 10:10 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote:> I don''t know how it''s > working with upstream version since I found out that syntax like `xm > create -c PVguest` with default settings (pyGrub bootloader) doesn''t > show the pyGrub at all so I don''t know what''s wrong with my setup. I''m > using 2.6.32.15-xen kernel/hypervisor version with latest unstable > user-space tools. > > Any hint how this should be working Ian?It should be working as you expect, e.g. "xm create -c xxx" should show you the pygrub output, unless you have used something like "--entry=x" or "-q" which disable interactive mode in your bootloader_args. I''m afraid I don''t know what is broken, I''m reasonably sure it was working for me when I developed libxl_bootloader.c since I was comparing the two. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 10:18 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:10 PM, Ian Campbell wrote:> On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote: > >> I don''t know how it''s >> working with upstream version since I found out that syntax like `xm >> create -c PVguest` with default settings (pyGrub bootloader) doesn''t >> show the pyGrub at all so I don''t know what''s wrong with my setup. I''m >> using 2.6.32.15-xen kernel/hypervisor version with latest unstable >> user-space tools. >> >> Any hint how this should be working Ian? >> > It should be working as you expect, e.g. "xm create -c xxx" should show > you the pygrub output, unless you have used something like "--entry=x" > or "-q" which disable interactive mode in your bootloader_args. > > I''m afraid I don''t know what is broken, I''m reasonably sure it was > working for me when I developed libxl_bootloader.c since I was comparing > the two. > > Ian. > >No Ian, it''s not working. The config file is having: ... bootloader = "/usr/bin/pygrub" ... So it should show the pyGrub ncurses screen, right? But it doesn''t show anything on version cloned from git yesterday. Any ideas what may be wrong? Maybe a configuration file should be somewhat different, I don''t know but this is working fine to get the data from image - it just doesn''t show the ncurses pyGrub screen. Isn''t it possible this is somehow connected to the PTY patch ? How should the PTY c/s 13540 ? This is the rewrite that was there and honestly I don''t remember whether I tried to run `xm create -c PVguest` on upstream Xen-4.1 ever so I don''t know. Isn''t it hypervisor/kernel related that it requires some data that are not coming from there? Thanks, Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Paolo Bonzini
2010-Aug-31 10:30 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:18 PM, Michal Novotny wrote:> Isn''t it possible this is > somehow connected to the PTY patch ? How should the PTY c/s 13540 ?This one has been there since Xen 3.1, and it''s, ehm, unlikely that it broke that much time ago. :)> Isn''t it hypervisor/kernel related that it requires some data that > are not coming from there?Huh??? Paolo _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Aug-31 10:37 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On Tue, 2010-08-31 at 11:18 +0100, Michal Novotny wrote:> On 08/31/2010 12:10 PM, Ian Campbell wrote: > > On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote: > > > >> I don''t know how it''s > >> working with upstream version since I found out that syntax like `xm > >> create -c PVguest` with default settings (pyGrub bootloader) doesn''t > >> show the pyGrub at all so I don''t know what''s wrong with my setup. I''m > >> using 2.6.32.15-xen kernel/hypervisor version with latest unstable > >> user-space tools. > >> > >> Any hint how this should be working Ian? > >> > > It should be working as you expect, e.g. "xm create -c xxx" should show > > you the pygrub output, unless you have used something like "--entry=x" > > or "-q" which disable interactive mode in your bootloader_args. > > > > I''m afraid I don''t know what is broken, I''m reasonably sure it was > > working for me when I developed libxl_bootloader.c since I was comparing > > the two. > > > > Ian. > > > > > No Ian, it''s not working. The config file is having: > ... > bootloader = "/usr/bin/pygrub" > ... > So it should show the pyGrub ncurses screen, right?Correct.> But it doesn''t show > anything on version cloned from git yesterday.xen-unstable or xen-4.0-testing or something else? It looks like this was broken in xen-unstable with 21994:2e08ec0028e4. The patch below should fix it. Ian. Subject: libxl+xend: use correct paths for PV console when running bootloader Makes "{xl,xm} create -c GUEST" work again with pygrub in interactive mode which was broken by 21994:2e08ec0028e4 Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c --- a/tools/libxl/libxl_bootloader.c Tue Aug 31 09:54:18 2010 +0100 +++ b/tools/libxl/libxl_bootloader.c Tue Aug 31 11:34:20 2010 +0100 @@ -383,7 +383,7 @@ int libxl_run_bootloader(libxl_ctx *ctx, goto out_close; } - dom_console_xs_path = libxl_sprintf(&gc, "%s/serial/0/tty", libxl_xs_get_dompath(&gc, domid)); + dom_console_xs_path = libxl_sprintf(&gc, "%s/console/tty", libxl_xs_get_dompath(&gc, domid)); libxl_xs_write(&gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path); pid = fork_exec_bootloader(&bootloader_fd, (char *)info->u.pv.bootloader, args); diff -r f77e54fadc18 tools/python/xen/util/diagnose.py --- a/tools/python/xen/util/diagnose.py Tue Aug 31 09:54:18 2010 +0100 +++ b/tools/python/xen/util/diagnose.py Tue Aug 31 11:34:20 2010 +0100 @@ -77,7 +77,7 @@ def diagnose_console(): def diagnose_console(): port = xstransact.Read(dompath + ''/console/port'') ringref = xstransact.Read(dompath + ''/console/ring-ref'') - tty = xstransact.Read(dompath + ''/serial/0/tty'') + tty = xstransact.Read(dompath + ''/console/tty'') if not port: print "Console port is missing; Xend has failed." diff -r f77e54fadc18 tools/python/xen/xend/XendBootloader.py --- a/tools/python/xen/xend/XendBootloader.py Tue Aug 31 09:54:18 2010 +0100 +++ b/tools/python/xen/xend/XendBootloader.py Tue Aug 31 11:34:20 2010 +0100 @@ -85,7 +85,7 @@ def bootloader(blexec, disk, dom, quiet fcntl.fcntl(m1, fcntl.F_SETFL, os.O_NDELAY) slavename = ptsname.ptsname(m1) - dom.storeDom("serial/0/tty", slavename) + dom.storeDom("console/tty", slavename) # Release the domain lock here, because we definitely don''t want # a stuck bootloader to deny service to other xend clients. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 11:12 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:30 PM, Paolo Bonzini wrote:> On 08/31/2010 12:18 PM, Michal Novotny wrote: >> Isn''t it possible this is >> somehow connected to the PTY patch ? How should the PTY c/s 13540 ? > > This one has been there since Xen 3.1, and it''s, ehm, unlikely that it > broke that much time ago. :)Oh, maybe but in fact I don''t know what version I used it last time to use syntax like `xm create -c PVguest`.> >> Isn''t it hypervisor/kernel related that it requires some data that >> are not coming from there? > > Huh??? >Oh, nevermind if it''s been there since Xen 3.1. Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 11:14 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:37 PM, Ian Campbell wrote:> On Tue, 2010-08-31 at 11:18 +0100, Michal Novotny wrote: > >> On 08/31/2010 12:10 PM, Ian Campbell wrote: >> >>> On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote: >>> >>> >>>> I don''t know how it''s >>>> working with upstream version since I found out that syntax like `xm >>>> create -c PVguest` with default settings (pyGrub bootloader) doesn''t >>>> show the pyGrub at all so I don''t know what''s wrong with my setup. I''m >>>> using 2.6.32.15-xen kernel/hypervisor version with latest unstable >>>> user-space tools. >>>> >>>> Any hint how this should be working Ian? >>>> >>>> >>> It should be working as you expect, e.g. "xm create -c xxx" should show >>> you the pygrub output, unless you have used something like "--entry=x" >>> or "-q" which disable interactive mode in your bootloader_args. >>> >>> I''m afraid I don''t know what is broken, I''m reasonably sure it was >>> working for me when I developed libxl_bootloader.c since I was comparing >>> the two. >>> >>> Ian. >>> >>> >>> >> No Ian, it''s not working. The config file is having: >> ... >> bootloader = "/usr/bin/pygrub" >> ... >> So it should show the pyGrub ncurses screen, right? >> > Correct. > > >> But it doesn''t show >> anything on version cloned from git yesterday. >> > xen-unstable or xen-4.0-testing or something else? > > It looks like this was broken in xen-unstable with 21994:2e08ec0028e4. > The patch below should fix it. > > Ian. > > Subject: libxl+xend: use correct paths for PV console when running bootloader > > Makes "{xl,xm} create -c GUEST" work again with pygrub in interactive > mode which was broken by 21994:2e08ec0028e4 > > Signed-off-by: Ian Campbell<ian.campbell@citrix.com> > diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c > --- a/tools/libxl/libxl_bootloader.c Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/libxl/libxl_bootloader.c Tue Aug 31 11:34:20 2010 +0100 > @@ -383,7 +383,7 @@ int libxl_run_bootloader(libxl_ctx *ctx, > goto out_close; > } > > - dom_console_xs_path = libxl_sprintf(&gc, "%s/serial/0/tty", libxl_xs_get_dompath(&gc, domid)); > + dom_console_xs_path = libxl_sprintf(&gc, "%s/console/tty", libxl_xs_get_dompath(&gc, domid)); > libxl_xs_write(&gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path); > > pid = fork_exec_bootloader(&bootloader_fd, (char *)info->u.pv.bootloader, args); > diff -r f77e54fadc18 tools/python/xen/util/diagnose.py > --- a/tools/python/xen/util/diagnose.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/util/diagnose.py Tue Aug 31 11:34:20 2010 +0100 > @@ -77,7 +77,7 @@ def diagnose_console(): > def diagnose_console(): > port = xstransact.Read(dompath + ''/console/port'') > ringref = xstransact.Read(dompath + ''/console/ring-ref'') > - tty = xstransact.Read(dompath + ''/serial/0/tty'') > + tty = xstransact.Read(dompath + ''/console/tty'') > > if not port: > print "Console port is missing; Xend has failed." > diff -r f77e54fadc18 tools/python/xen/xend/XendBootloader.py > --- a/tools/python/xen/xend/XendBootloader.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/xend/XendBootloader.py Tue Aug 31 11:34:20 2010 +0100 > @@ -85,7 +85,7 @@ def bootloader(blexec, disk, dom, quiet > fcntl.fcntl(m1, fcntl.F_SETFL, os.O_NDELAY) > > slavename = ptsname.ptsname(m1) > - dom.storeDom("serial/0/tty", slavename) > + dom.storeDom("console/tty", slavename) > > # Release the domain lock here, because we definitely don''t want > # a stuck bootloader to deny service to other xend clients. > > >Oh, this may be the one. What I was using was staging xen-unstable. I''ll try to revert this one. Thanks, Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 11:18 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:37 PM, Ian Campbell wrote:> On Tue, 2010-08-31 at 11:18 +0100, Michal Novotny wrote: > >> On 08/31/2010 12:10 PM, Ian Campbell wrote: >> >>> On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote: >>> >>> >>>> I don''t know how it''s >>>> working with upstream version since I found out that syntax like `xm >>>> create -c PVguest` with default settings (pyGrub bootloader) doesn''t >>>> show the pyGrub at all so I don''t know what''s wrong with my setup. I''m >>>> using 2.6.32.15-xen kernel/hypervisor version with latest unstable >>>> user-space tools. >>>> >>>> Any hint how this should be working Ian? >>>> >>>> >>> It should be working as you expect, e.g. "xm create -c xxx" should show >>> you the pygrub output, unless you have used something like "--entry=x" >>> or "-q" which disable interactive mode in your bootloader_args. >>> >>> I''m afraid I don''t know what is broken, I''m reasonably sure it was >>> working for me when I developed libxl_bootloader.c since I was comparing >>> the two. >>> >>> Ian. >>> >>> >>> >> No Ian, it''s not working. The config file is having: >> ... >> bootloader = "/usr/bin/pygrub" >> ... >> So it should show the pyGrub ncurses screen, right? >> > Correct. > > >> But it doesn''t show >> anything on version cloned from git yesterday. >> > xen-unstable or xen-4.0-testing or something else? > > It looks like this was broken in xen-unstable with 21994:2e08ec0028e4. > The patch below should fix it. > > Ian. > > Subject: libxl+xend: use correct paths for PV console when running bootloader > > Makes "{xl,xm} create -c GUEST" work again with pygrub in interactive > mode which was broken by 21994:2e08ec0028e4 > > Signed-off-by: Ian Campbell<ian.campbell@citrix.com> > diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c > --- a/tools/libxl/libxl_bootloader.c Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/libxl/libxl_bootloader.c Tue Aug 31 11:34:20 2010 +0100 > @@ -383,7 +383,7 @@ int libxl_run_bootloader(libxl_ctx *ctx, > goto out_close; > } > > - dom_console_xs_path = libxl_sprintf(&gc, "%s/serial/0/tty", libxl_xs_get_dompath(&gc, domid)); > + dom_console_xs_path = libxl_sprintf(&gc, "%s/console/tty", libxl_xs_get_dompath(&gc, domid)); > libxl_xs_write(&gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path); > > pid = fork_exec_bootloader(&bootloader_fd, (char *)info->u.pv.bootloader, args); > diff -r f77e54fadc18 tools/python/xen/util/diagnose.py > --- a/tools/python/xen/util/diagnose.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/util/diagnose.py Tue Aug 31 11:34:20 2010 +0100 > @@ -77,7 +77,7 @@ def diagnose_console(): > def diagnose_console(): > port = xstransact.Read(dompath + ''/console/port'') > ringref = xstransact.Read(dompath + ''/console/ring-ref'') > - tty = xstransact.Read(dompath + ''/serial/0/tty'') > + tty = xstransact.Read(dompath + ''/console/tty'') > > if not port: > print "Console port is missing; Xend has failed." > diff -r f77e54fadc18 tools/python/xen/xend/XendBootloader.py > --- a/tools/python/xen/xend/XendBootloader.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/xend/XendBootloader.py Tue Aug 31 11:34:20 2010 +0100 > @@ -85,7 +85,7 @@ def bootloader(blexec, disk, dom, quiet > fcntl.fcntl(m1, fcntl.F_SETFL, os.O_NDELAY) > > slavename = ptsname.ptsname(m1) > - dom.storeDom("serial/0/tty", slavename) > + dom.storeDom("console/tty", slavename) > > # Release the domain lock here, because we definitely don''t want > # a stuck bootloader to deny service to other xend clients. > > >Well, the XendBootloader.py part seems to be the one (since I use xend with xm, no xl). I''ll test it later. Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Michal Novotny
2010-Aug-31 11:33 UTC
[Xen-devel] Re: [PATCH] Fix bootloader handling when empty string is being output
On 08/31/2010 12:37 PM, Ian Campbell wrote:> On Tue, 2010-08-31 at 11:18 +0100, Michal Novotny wrote: > >> On 08/31/2010 12:10 PM, Ian Campbell wrote: >> >>> On Tue, 2010-08-31 at 11:00 +0100, Michal Novotny wrote: >>> >>> >>>> I don''t know how it''s >>>> working with upstream version since I found out that syntax like `xm >>>> create -c PVguest` with default settings (pyGrub bootloader) doesn''t >>>> show the pyGrub at all so I don''t know what''s wrong with my setup. I''m >>>> using 2.6.32.15-xen kernel/hypervisor version with latest unstable >>>> user-space tools. >>>> >>>> Any hint how this should be working Ian? >>>> >>>> >>> It should be working as you expect, e.g. "xm create -c xxx" should show >>> you the pygrub output, unless you have used something like "--entry=x" >>> or "-q" which disable interactive mode in your bootloader_args. >>> >>> I''m afraid I don''t know what is broken, I''m reasonably sure it was >>> working for me when I developed libxl_bootloader.c since I was comparing >>> the two. >>> >>> Ian. >>> >>> >>> >> No Ian, it''s not working. The config file is having: >> ... >> bootloader = "/usr/bin/pygrub" >> ... >> So it should show the pyGrub ncurses screen, right? >> > Correct. > > >> But it doesn''t show >> anything on version cloned from git yesterday. >> > xen-unstable or xen-4.0-testing or something else? > > It looks like this was broken in xen-unstable with 21994:2e08ec0028e4. > The patch below should fix it. > > Ian. > > Subject: libxl+xend: use correct paths for PV console when running bootloader > > Makes "{xl,xm} create -c GUEST" work again with pygrub in interactive > mode which was broken by 21994:2e08ec0028e4 > > Signed-off-by: Ian Campbell<ian.campbell@citrix.com> > diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c > --- a/tools/libxl/libxl_bootloader.c Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/libxl/libxl_bootloader.c Tue Aug 31 11:34:20 2010 +0100 > @@ -383,7 +383,7 @@ int libxl_run_bootloader(libxl_ctx *ctx, > goto out_close; > } > > - dom_console_xs_path = libxl_sprintf(&gc, "%s/serial/0/tty", libxl_xs_get_dompath(&gc, domid)); > + dom_console_xs_path = libxl_sprintf(&gc, "%s/console/tty", libxl_xs_get_dompath(&gc, domid)); > libxl_xs_write(&gc, XBT_NULL, dom_console_xs_path, "%s", dom_console_slave_tty_path); > > pid = fork_exec_bootloader(&bootloader_fd, (char *)info->u.pv.bootloader, args); > diff -r f77e54fadc18 tools/python/xen/util/diagnose.py > --- a/tools/python/xen/util/diagnose.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/util/diagnose.py Tue Aug 31 11:34:20 2010 +0100 > @@ -77,7 +77,7 @@ def diagnose_console(): > def diagnose_console(): > port = xstransact.Read(dompath + ''/console/port'') > ringref = xstransact.Read(dompath + ''/console/ring-ref'') > - tty = xstransact.Read(dompath + ''/serial/0/tty'') > + tty = xstransact.Read(dompath + ''/console/tty'') > > if not port: > print "Console port is missing; Xend has failed." > diff -r f77e54fadc18 tools/python/xen/xend/XendBootloader.py > --- a/tools/python/xen/xend/XendBootloader.py Tue Aug 31 09:54:18 2010 +0100 > +++ b/tools/python/xen/xend/XendBootloader.py Tue Aug 31 11:34:20 2010 +0100 > @@ -85,7 +85,7 @@ def bootloader(blexec, disk, dom, quiet > fcntl.fcntl(m1, fcntl.F_SETFL, os.O_NDELAY) > > slavename = ptsname.ptsname(m1) > - dom.storeDom("serial/0/tty", slavename) > + dom.storeDom("console/tty", slavename) > > # Release the domain lock here, because we definitely don''t want > # a stuck bootloader to deny service to other xend clients. > > >Thanks Ian! I''ve tested the patch and it''s working fine. You can push it to the tree since it helped me with this one and `xm create -c PVguest` syntax was working fine when applied. Michal -- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Aug-31 17:56 UTC
Re: [Xen-devel] [PATCH] Fix bootloader handling when empty string is being output
Michal Novotny writes ("[Xen-devel] [PATCH] Fix bootloader handling when
empty string is being output"):> this is the patch to fix empty string as the output value
> of the bootloader string. If there is no output then
> xend is being hung indefinitely unless you press Ctrl + C keys
> to trigger SIGINT. This patch fixes the issue by implementing
> timeout to the select() function for reading from pipe and also
> checking for state of the process - i.e. whether the process is
> running or whether it''s dead or zombie (because fork() call does
> leave zombies when the process is done but the parent process is
> still running).
This is a rather strange way to go about collecting the exit status
from a process. What is pygrub''s parent in this situation ?
Isn''t it
xend ? In which case xend has no need to grobbling around in /proc,
it can just call waitpid.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel