Skip to contentSkip to conversation

Fix bootloader handling when empty string is being output

PATCH In review Michal Novotny 5 participants 13 messages Aug 30, 2010 – Aug 31, 2010

lore
Share & tools
lore

Michal Novotny

author
#0 lore raw

PATCH 1/1 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 <[email protected]>

Michal

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
Quoted 1 line

From b145011be18b700e89234f6ddc12e32872c2c121 Mon Sep 17 00:00:00 2001

From: Michal Novotny <[email protected]>
Date: Mon, 30 Aug 2010 15:06:05 +0200
Subject: [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).

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 <[email protected]>

Michal

---
 tools/python/xen/xend/XendBootloader.py |   36 ++++++++++++++++++++++++++----
 1 files changed, 31 insertions(+), 5 deletions(-)
tools/python/xen/xend/XendBootloader.py +31 -5
index 0cef917..66a688f 100644
@@ -24,6 +24,24 @@ from XendError import VmError
 import pty, termios, fcntl
 from xen.lowlevel import ptsname
 
+def hasPidInProcFS(pid):
+    path = "/proc/%s" % pid
+    return os.path.exists(path)
+
+def isRunning(pid):
+    path = "/proc/%s" % pid
+    if not os.path.exists(path):
+        return False
+    fd = open("%s/stat" % path)
+    val = fd.readline().split()[2]
+    fd.close()
+    try:
+        val.index('Z')
+        ret = False
+    except:
+        ret = True
+    return ret
+
 def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
                ramdisk = '', kernel_args = ''):
     """Run the boot loader executable on the given disk and return a

@@ -128,16 +146,22 @@ def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
     if os.uname()[0] != 'SunOS':
         tty.setraw(m2);
 
-    fcntl.fcntl(m2, fcntl.F_SETFL, os.O_NDELAY);
+    # We check whether OS supports procfs in /proc like Linux does
+    canUseProcfs = hasPidInProcFS( os.getpid() )
+
+    fcntl.fcntl(m2, fcntl.F_SETFL, os.O_NDELAY)
     while True:
         try:
-            r = os.open(fifo, os.O_RDONLY)
+            mode = os.O_RDONLY
+            if canUseProcfs:
+                mode |= os.O_NDELAY
+            r = os.open(fifo, mode)
         except OSError, e:
             if e.errno == errno.EINTR:
                 continue
         break
 
-    fcntl.fcntl(r, fcntl.F_SETFL, os.O_NDELAY);
+    fcntl.fcntl(r, fcntl.F_SETFL, os.O_NDELAY)
 
     ret = ""
     inbuf=""; outbuf="";

@@ -156,13 +180,15 @@ def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
     # if there is actual data to write, otherwise this would loop too fast,
     # eating up CPU time.
 
-    while True:
+    while ((canUseProcfs and isRunning(child)) or not canUseProcfs):
         wsel = []
         if len(outbuf) != 0:
             wsel = wsel + [m1]
         if len(inbuf) != 0:
             wsel = wsel + [m2]
-        sel = select.select([r, m1, m2], wsel, [])
+
+        sel = select.select([r, m1, m2], wsel, [], 1)
+
         try: 
             if m1 in sel[0]:
                 s = os.read(m1, 16)
Signature · 1 line
1.5.5.6

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 5 lines · Michal Novotny

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.

#2 lore raw
Quoted 8 lines · Ian Campbell
Quoted 5 lines · Michal Novotny

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.

Quoted 6 lines

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

author
#3 lore raw
Quoted 25 lines · Paolo Bonzini
Quoted 8 lines · Ian Campbell
Quoted 5 lines · Michal Novotny

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.

Quoted 6 lines

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.

Quoted 8 lines

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

So, 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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
#4 lore raw
Quoted 8 lines · Michal Novotny

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.

author
#5 lore raw
Quoted 15 lines · Ian Campbell
Quoted 9 lines · Michal Novotny

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.

Quoted 7 lines

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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
#6 lore raw
Quoted 2 lines · Michal Novotny

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. :)

Quoted 2 lines

Isn't it hypervisor/kernel related that it requires some data that are not coming from there?

Huh???

Paolo

author
#7 lore raw
Quoted 5 lines · Paolo Bonzini
Quoted 2 lines · Michal Novotny

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.

Quoted 6 lines
Quoted 3 lines

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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
#8 lore raw
In reply to Michal Novotny
Quoted 20 lines · Michal Novotny
Quoted 19 lines · Ian Campbell
Quoted 9 lines · Michal Novotny

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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 2 lines

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 <[email protected]>
tools/libxl/libxl_bootloader.c +1 -1
diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c
@@ -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);
tools/python/xen/util/diagnose.py +1 -1
diff -r f77e54fadc18 tools/python/xen/util/diagnose.py
@@ -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."
tools/python/xen/xend/XendBootloader.py +1 -1
diff -r f77e54fadc18 tools/python/xen/xend/XendBootloader.py
@@ -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.
author
#9 lore raw
Quoted 85 lines · Ian Campbell
Quoted 27 lines · Michal Novotny
Quoted 17 lines · Ian Campbell
Quoted 10 lines · Michal Novotny

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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<[email protected]>

diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +1 -1
@@ -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

tools/python/xen/util/diagnose.py +1 -1
@@ -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

tools/python/xen/xend/XendBootloader.py def bootloader(blexec, disk, dom, quiet · line 91

Quoted 8 lines
tools/python/xen/xend/XendBootloader.py +1 -1
@@ -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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
author
#10 lore raw
In reply to Ian Campbell
Quoted 85 lines · Ian Campbell
Quoted 27 lines · Michal Novotny
Quoted 17 lines · Ian Campbell
Quoted 10 lines · Michal Novotny

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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<[email protected]>

diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +1 -1
@@ -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

tools/python/xen/util/diagnose.py +1 -1
@@ -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

tools/python/xen/xend/XendBootloader.py def bootloader(blexec, disk, dom, quiet · line 91

Quoted 8 lines
tools/python/xen/xend/XendBootloader.py +1 -1
@@ -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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
author
#11 lore raw
In reply to Ian Campbell
Quoted 85 lines · Ian Campbell
Quoted 27 lines · Michal Novotny
Quoted 17 lines · Ian Campbell
Quoted 10 lines · Michal Novotny

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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<[email protected]>

diff -r f77e54fadc18 tools/libxl/libxl_bootloader.c

tools/libxl/libxl_bootloader.c +1 -1
@@ -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

tools/python/xen/util/diagnose.py +1 -1
@@ -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

tools/python/xen/xend/XendBootloader.py def bootloader(blexec, disk, dom, quiet · line 91

Quoted 8 lines
tools/python/xen/xend/XendBootloader.py +1 -1
@@ -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

Signature · 2 lines
Michal Novotny<[email protected]>, RHCE
Virtualization Team (xen userspace), Red Hat
#12 lore raw
In reply to Michal Novotny

Michal Novotny writes ("[Xen-devel] [PATCH] Fix bootloader handling when empty string is being output"):

Quoted 9 lines

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.