[PATCH] Fix bootloader handling when bootloader exits too early or didn't return any data

[Michal Novotny <minovotn@redhat.com>]

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

Hi,
this is the patch to fix empty string handling as the output value of 
the bootloader string or when the bootloader process exists too early. 
Without this patch applied the xend process has been stuck indefinitely 
on the read() function in the bootloader() function since it was waiting 
for the bootloader process to open the write end of the pipe but if the 
bootloader process is bogus (e.g. just a user shell script returning 
with no output and code 0) that doesn't open a FIFO file the read() call 
was stuck. Therefore this patch is opening the FIFO with O_NDELAY flag 
to make it non-blocking and it uses pipe() and creates a separate thread 
for the wait on bootloader process to exit and when bootloader has 
exited the select() intercepts the end of bootloader process and breaks 
the loop.

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 opening a pipe and 
passing the write end of the pipe to the new thread
that's waiting for the bootloader process to be terminated. When it's 
terminated it's writing to the write end of the pipe
which is being read by the bootloader() method to terminate the loop.

Testing: The patch has been tested on RHEL-5.5 x86_64 dom0 with latest 
version of upstream Xen installed (staging) and 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. All the tests passed successfully when 
having following Ian's patch applied: libxl+xend: use correct paths for 
PV console when running bootloader

Signed-off-by: Michal Novotny <minovotn@redhat.com>

Michal

Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat


[-- Attachment #2: fix-bootloader-empty-output-handling.patch --]
[-- Type: text/x-patch, Size: 5015 bytes --]

>From ca210417d31409659ce65c93aa045f85482ae50e Mon Sep 17 00:00:00 2001
From: Michal Novotny <minovotn@redhat.com>
Date: Tue, 31 Aug 2010 14:04:44 +0200
Subject: [PATCH] Fix bootloader handling when bootloader exits too early or didn't return any data

Hi,
this is the patch to fix empty string handling as the output
value of the bootloader string or when the bootloader process
exists too early. Without this patch applied the xend process
has been stuck indefinitely on the read() function in the
bootloader() function since it was waiting for the bootloader
process to open the write end of the pipe but if the bootloader
process is bogus (e.g. just a user shell script returning with
no output and code 0) that doesn't open a FIFO file the read()
call was stuck. Therefore this patch is opening the FIFO with
O_NDELAY flag to make it non-blocking and it uses pipe()
instead and creates a separate thread for the wait on bootloader
process to exit and when bootloader has exited the select()
intercepts the end of bootloader process and breaks the loop.

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 opening a
pipe and passing the write end of the pipe to the new thread
that's waiting for the bootloader process to be terminated.
When it's terminated it's writing to the write end of the pipe
which is being read by the bootloader() method to terminate
the loop.

Testing: The patch has been tested on RHEL-5.5 x86_64 dom0 with
latest version of upstream Xen installed (staging) and 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.
All the tests passed successfully when having following Ian's patch
applied: libxl+xend: use correct paths for PV console when running bootloader

Signed-off-by: Michal Novotny <minovotn@redhat.com>

Michal

---
 tools/python/xen/xend/XendBootloader.py |   36 ++++++++++++++++++++++--------
 1 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/tools/python/xen/xend/XendBootloader.py b/tools/python/xen/xend/XendBootloader.py
index 74c9a2a..d83f498 100644
--- a/tools/python/xen/xend/XendBootloader.py
+++ b/tools/python/xen/xend/XendBootloader.py
@@ -15,6 +15,7 @@
 import os, select, errno, stat, signal, tty
 import random
 import shlex
+import threading
 from xen.xend import sxp
 
 from xen.util import mkdir, oshelp
@@ -129,15 +130,11 @@ def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
         tty.setraw(m2);
 
     fcntl.fcntl(m2, fcntl.F_SETFL, os.O_NDELAY);
-    while True:
-        try:
-            r = os.open(fifo, os.O_RDONLY)
-        except OSError, e:
-            if e.errno == errno.EINTR:
-                continue
-        break
+    r = os.open(fifo, os.O_RDONLY | os.O_NDELAY)
+    (pr, pw) = os.pipe()
 
-    fcntl.fcntl(r, fcntl.F_SETFL, os.O_NDELAY);
+    thread = threading.Thread(target = bootloader_wait, args = (child, pw, ))
+    thread.start()
 
     ret = ""
     inbuf=""; outbuf="";
@@ -162,7 +159,7 @@ def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
             wsel = wsel + [m1]
         if len(inbuf) != 0:
             wsel = wsel + [m2]
-        sel = select.select([r, m1, m2], wsel, [])
+        sel = select.select([r, m1, m2, pr], wsel, [])
         try: 
             if m1 in sel[0]:
                 s = os.read(m1, 16)
@@ -188,9 +185,11 @@ def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
             ret = ret + s
             if len(s) == 0:
                 break
+        if pr in sel[0]:
+            break
     del inbuf
     del outbuf
-    os.waitpid(child, 0)
+    os.close(pr)
     os.close(r)
     os.close(m2)
     os.close(m1)
@@ -227,4 +226,21 @@ def bootloader_tidy(dom):
         dom.bootloader_pid = None
         os.kill(pid, signal.SIGKILL)
 
+def bootloader_wait(pid, write_end):
+    while 1:
+        try:
+            os.waitpid(pid, 0)
+        except OSError, e:
+            if e.errno == errno.EINTR:
+                continue
+            raise
+        # Loop if EINTR is returned
+        break
 
+    # Terminate the while loop in function bootloader
+    try:
+        os.write(write_end, "t")
+    except OSError, e:
+        if e.errno != errno.EPIPE:
+            raise
+    os.close(write_end)
-- 
1.5.5.6


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

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