Xen devel - Nov 2006 - [RFC] bootloader improvements

This is a set of patches we''re using to pygrub/bootloading. The aim of
the patchset as a whole is to allow xm/xend to just "do the right
thing"
by default, hopefully making it easier to configure, use, and deploy
Solaris and Linux domU''s. The patches are:

pygrub-automatic

	- Default to using pygrub if there''s no kernel and no bootloader
	  specified.

pygrub-default

	- Make pygrub be the "default" bootloader and understand that
	  keyword.

pygrub-kernel-config
pygrub-no-grub

	- The combination of these patches allows pygrub FS code to be
	  used in a hands-off manner. Note that this can also be used to
	  fix the problem pygrub currently has with restart, by
	  specifying the kernel/ramdisk explicitly in the config. The
	  split between "boot kernel" and "configured kernel" might
need
	  some xen api/lifecycle changes too?

pygrub-solaris

	- Solaris domU''s will typically never change the kernel/ramdisk
	  path they use, except if someone''s debugging etc. Add some
	  logic to pygrub so it knows what to do.

All comments welcome.

regards
john

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

# HG changeset patch
# User john.levon@sun.com
# Date 1163095747 28800
# Node ID 6750c8c56b5f139c2b227472d8b9e1f9555fdb6f
# Parent  e2c100dfcb05d8f29d612c29af2edf1dcfd3f7fc
Use pygrub automatically if no kernel is specified.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/python/xen/xend/osdep.py b/tools/python/xen/xend/osdep.py
--- a/tools/python/xen/xend/osdep.py
+++ b/tools/python/xen/xend/osdep.py
@@ -20,17 +20,20 @@ import os
 import os
 
 _scripts_dir = {
-    "Linux": "/etc/xen/scripts",
     "SunOS": "/usr/lib/xen/scripts",
 }
 
 _xend_autorestart = {
-    "Linux": True,
     "SunOS": False,
+}
+
+_pygrub_path = {
+    "SunOS": "/usr/lib/xen/bin/pygrub"
 }
 
 def _get(var, default=None):
     return var.get(os.uname()[0], default)
 
 scripts_dir = _get(_scripts_dir, "/etc/xen/scripts")
-xend_autorestart = _get(_xend_autorestart)
+xend_autorestart = _get(_xend_autorestart, True)
+pygrub_path = _get(_pygrub_path, "/usr/bin/pygrub")
diff --git a/tools/python/xen/xm/create.py b/tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py
+++ b/tools/python/xen/xm/create.py
@@ -27,6 +27,7 @@ import xmlrpclib
 import xmlrpclib
 
 from xen.xend import sxp
+from xen.xend import osdep
 from xen.xend import PrettyPrint
 import xen.xend.XendClient
 from xen.xend.XendClient import server
@@ -951,7 +952,7 @@ def preprocess_vnc(vals):
     
 def preprocess(vals):
     if not vals.kernel and not vals.bootloader:
-        err("No kernel specified")
+        vals.bootloader = osdep.pygrub_path
     preprocess_disk(vals)
     preprocess_pci(vals)
     preprocess_ioports(vals)

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

# HG changeset patch
# User john.levon@sun.com
# Date 1163095795 28800
# Node ID 7c80aaffe16f5e4037d10660d262ed7b22894b01
# Parent  6750c8c56b5f139c2b227472d8b9e1f9555fdb6f
Make bootloader = "default" use pygrub. This avoids the need to know
the pygrub path, which differs between OS''s.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/python/xen/xm/create.py b/tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py
+++ b/tools/python/xen/xm/create.py
@@ -951,7 +951,8 @@ def preprocess_vnc(vals):
             vals.extra = vnc + '' '' + vals.extra
     
 def preprocess(vals):
-    if not vals.kernel and not vals.bootloader:
+    if ((not vals.kernel and not vals.bootloader) or
+        vals.bootloader == "default"):
         vals.bootloader = osdep.pygrub_path
     preprocess_disk(vals)
     preprocess_pci(vals)

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

# HG changeset patch
# User john.levon@sun.com
# Date 1163095800 28800
# Node ID 0da173ee886e13bd4116c7d085cd4a4704ffe279
# Parent  7c80aaffe16f5e4037d10660d262ed7b22894b01
Split out bootloader-derived parameters separately from config-file-specified
ones. This allows a config file to still specify kernel/ramdisk, even if a
bootloader is being used.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -514,17 +514,17 @@ if __name__ == "__main__":
     fs = fsimage.open(file, offset)
 
     kernel = fs.open_file(img.kernel[1],).read()
-    (tfd, fn) = tempfile.mkstemp(prefix="vmlinuz.",
dir="/var/lib/xen")
+    (tfd, fn) = tempfile.mkstemp(prefix="boot_kernel.",
dir="/var/lib/xen")
     os.write(tfd, kernel)
     os.close(tfd)
-    sxp = "linux (kernel %s)" %(fn,)
+    sxp = "linux (boot_kernel %s)" %(fn,)
 
     if img.initrd:
         initrd = fs.open_file(img.initrd[1],).read()
-        (tfd, fn) = tempfile.mkstemp(prefix="initrd.",
dir="/var/lib/xen")
+        (tfd, fn) = tempfile.mkstemp(prefix="boot_ramdisk.",
dir="/var/lib/xen")
         os.write(tfd, initrd)
         os.close(tfd)
-        sxp += "(ramdisk %s)" %(fn,)
+        sxp += "(boot_ramdisk %s)" %(fn,)
     else:
         initrd = None
     sxp += "(args ''%s'')" %(img.args,)
diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py
+++ b/tools/python/xen/xend/image.py
@@ -70,7 +70,9 @@ class ImageHandler:
         self.vm = vm
 
         self.kernel = None
+        self.boot_kernel = None
         self.ramdisk = None
+        self.boot_ramdisk = None
         self.cmdline = None
 
         self.configure(imageConfig, deviceConfig)
@@ -82,6 +84,14 @@ class ImageHandler:
             return sxp.child_value(imageConfig, name, default)
 
         self.kernel = get_cfg("kernel")
+        self.ramdisk = get_cfg("ramdisk", '''')
+        self.boot_kernel = get_cfg("boot_kernel")
+        if not self.boot_kernel:
+            self.boot_kernel = self.kernel
+        self.boot_ramdisk = get_cfg("boot_ramdisk")
+        if not self.boot_ramdisk:
+            self.boot_ramdisk = self.ramdisk
+
         self.cmdline = ""
         ip = get_cfg("ip")
         if ip:
@@ -92,7 +102,7 @@ class ImageHandler:
         args = get_cfg("args")
         if args:
             self.cmdline += " " + args
-        self.ramdisk = get_cfg("ramdisk", '''')
+
         
         self.vm.storeVm(("image/ostype", self.ostype),
                         ("image/kernel", self.kernel),
@@ -101,8 +111,8 @@ class ImageHandler:
 
 
     def cleanupBootloading(self):
-        self.unlink(self.kernel)
-        self.unlink(self.ramdisk)
+        self.unlink(self.boot_kernel)
+        self.unlink(self.boot_ramdisk)
 
 
     def unlink(self, f):
@@ -125,10 +135,11 @@ class ImageHandler:
         """
         # Set params and call buildDomain().
 
-        if not os.path.isfile(self.kernel):
-            raise VmError(''Kernel image does not exist: %s'' %
self.kernel)
-        if self.ramdisk and not os.path.isfile(self.ramdisk):
-            raise VmError(''Kernel ramdisk does not exist: %s''
% self.ramdisk)
+        if not os.path.isfile(self.boot_kernel):
+            raise VmError(''Kernel image does not exist: %s'' %
self.boot_kernel)
+        if self.boot_ramdisk and not os.path.isfile(self.boot_ramdisk):
+            raise VmError(''Kernel ramdisk does not exist: %s''
%
+                          self.boot_ramdisk)
         if len(self.cmdline) >= MAX_GUEST_CMDLINE:
             log.warning(''kernel cmdline too long, domain %d'',
                         self.vm.getDomid())
@@ -193,21 +204,21 @@ class LinuxImageHandler(ImageHandler):
 
         log.debug("domid          = %d", self.vm.getDomid())
         log.debug("memsize        = %d", mem_mb)
-        log.debug("image          = %s", self.kernel)
+        log.debug("image          = %s", self.boot_kernel)
         log.debug("store_evtchn   = %d", store_evtchn)
         log.debug("console_evtchn = %d", console_evtchn)
         log.debug("cmdline        = %s", self.cmdline)
-        log.debug("ramdisk        = %s", self.ramdisk)
+        log.debug("ramdisk        = %s", self.boot_ramdisk)
         log.debug("vcpus          = %d", self.vm.getVCpuCount())
         log.debug("features       = %s", self.vm.getFeatures())
 
         return xc.linux_build(domid          = self.vm.getDomid(),
                               memsize        = mem_mb,
-                              image          = self.kernel,
+                              image          = self.boot_kernel,
                               store_evtchn   = store_evtchn,
                               console_evtchn = console_evtchn,
                               cmdline        = self.cmdline,
-                              ramdisk        = self.ramdisk,
+                              ramdisk        = self.boot_ramdisk,
                               features       = self.vm.getFeatures())
 
 class PPC_LinuxImageHandler(LinuxImageHandler):
@@ -226,11 +237,11 @@ class PPC_LinuxImageHandler(LinuxImageHa
 
         log.debug("domid          = %d", self.vm.getDomid())
         log.debug("memsize        = %d", mem_mb)
-        log.debug("image          = %s", self.kernel)
+        log.debug("image          = %s", self.boot_kernel)
         log.debug("store_evtchn   = %d", store_evtchn)
         log.debug("console_evtchn = %d", console_evtchn)
         log.debug("cmdline        = %s", self.cmdline)
-        log.debug("ramdisk        = %s", self.ramdisk)
+        log.debug("ramdisk        = %s", self.boot_ramdisk)
         log.debug("vcpus          = %d", self.vm.getVCpuCount())
         log.debug("features       = %s", self.vm.getFeatures())
 
@@ -238,11 +249,11 @@ class PPC_LinuxImageHandler(LinuxImageHa
 
         return xc.linux_build(domid          = self.vm.getDomid(),
                               memsize        = mem_mb,
-                              image          = self.kernel,
+                              image          = self.boot_kernel,
                               store_evtchn   = store_evtchn,
                               console_evtchn = console_evtchn,
                               cmdline        = self.cmdline,
-                              ramdisk        = self.ramdisk,
+                              ramdisk        = self.boot_ramdisk,
                               features       = self.vm.getFeatures(),
                               arch_args      = devtree.to_bin())
 
@@ -287,7 +298,7 @@ class HVMImageHandler(ImageHandler):
         mem_mb = self.getRequiredInitialReservation() / 1024
 
         log.debug("domid          = %d", self.vm.getDomid())
-        log.debug("image          = %s", self.kernel)
+        log.debug("image          = %s", self.boot_kernel)
         log.debug("store_evtchn   = %d", store_evtchn)
         log.debug("memsize        = %d", mem_mb)
         log.debug("vcpus          = %d", self.vm.getVCpuCount())
@@ -297,7 +308,7 @@ class HVMImageHandler(ImageHandler):
         self.register_shutdown_watch()
 
         return xc.hvm_build(domid          = self.vm.getDomid(),
-                            image          = self.kernel,
+                            image          = self.boot_kernel,
                             store_evtchn   = store_evtchn,
                             memsize        = mem_mb,
                             vcpus          = self.vm.getVCpuCount(),

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

# HG changeset patch
# User john.levon@sun.com
# Date 1163095809 28800
# Node ID aebe98c33a6bc0cd633d915b9e0bad40718c29d6
# Parent  0da173ee886e13bd4116c7d085cd4a4704ffe279
Pass in kernel/ramdisk settings to pygrub; if specified, don''t try to
use
grub.conf; this allows hands-off bootloading in the absence of a grub.conf.
It''s also useful for specifying temporary changes etc.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -431,19 +431,56 @@ def get_entry_idx(cf, entry):
 
     return None
 
+def run_grub(file, isconfig, entry):
+    global g
+
+    def run_main(scr, *args):
+        global sel
+        global g
+        sel = g.run()
+
+    g = Grub(file, isconfig)
+    if interactive:
+        curses.wrapper(run_main)
+    else:
+        sel = g.cf.default
+
+    # set the entry to boot as requested
+    if entry is not None:
+        idx = get_entry_idx(g.cf, entry)
+        if idx is not None and idx > 0 and idx < len(g.cf.images):
+           sel = idx
+
+    if sel == -1:
+        print "No kernel image selected!"
+        sys.exit(1)
+
+    img = g.cf.images[sel]
+
+    grubcfg["kernel"] = img.kernel[1]
+    grubcfg["ramdisk"] = img.initrd[1]
+    grubcfg["args"] = img.args[1]
+
+    print "Going to boot %s" %(img.title)
+    print "  kernel: %s" % grubcfg["kernel"]
+    if img.initrd:
+        print "  initrd: %s" % grubcfg["ramdisk"]
+
+    if isconfig:
+        print "  args: %s" % grubcfg["args"]
+        sys.exit(0)
+        
+    return grubcfg
+
 if __name__ == "__main__":
     sel = None
     
-    def run_main(scr, *args):
-        global sel
-        sel = g.run()
-
     def usage():
-        print >> sys.stderr, "Usage: %s [-q|--quiet] [--output=]
[--entry=] <image>" %(sys.argv[0],)
+        print >> sys.stderr, "Usage: %s [-q|--quiet] [--output=]
[--kernel=] [--ramdisk=] [--args=] [--entry=] <image>"
%(sys.argv[0],)
 
     try:
         opts, args = getopt.gnu_getopt(sys.argv[1:], ''qh::'',
-                                   ["quiet", "help",
"output=", "entry=",
+                                   ["quiet", "help",
"output=", "entry=", "kernel=",
"ramdisk=", "args=",
                                     "isconfig"])
     except getopt.GetoptError:
         usage()
@@ -458,6 +495,14 @@ if __name__ == "__main__":
     entry = None
     interactive = True
     isconfig = False
+
+    # what was passed in
+    incfg = { "kernel": None, "ramdisk": None,
"args": None };
+    # what grub chose
+    chosencfg = { "kernel": None, "ramdisk": None,
"args": None };
+    # what to boot
+    bootcfg = { "kernel": None, "ramdisk": None,
"args": None };
+
     for o, a in opts:
         if o in ("-q", "--quiet"):
             interactive = False
@@ -466,6 +511,12 @@ if __name__ == "__main__":
             sys.exit()
         elif o in ("--output",):
             output = a
+        elif o in ("--kernel",):
+            incfg["kernel"] = a
+        elif o in ("--ramdisk",):
+            incfg["ramdisk"] = a
+        elif o in ("--args",):
+            incfg["args"] = a
         elif o in ("--entry",):
             entry = a
             # specifying the entry to boot implies non-interactive
@@ -473,37 +524,17 @@ if __name__ == "__main__":
         elif o in ("--isconfig",):
             isconfig = True
 
+
     if output is None or output == "-":
         fd = sys.stdout.fileno()
     else:
         fd = os.open(output, os.O_WRONLY)
 
-    g = Grub(file, isconfig)
-    if interactive:
-        curses.wrapper(run_main)
+    if not incfg["kernel"]:
+        chosencfg = run_grub(file, isconfig, entry)
     else:
-        sel = g.cf.default
-
-    # set the entry to boot as requested
-    if entry is not None:
-        idx = get_entry_idx(g.cf, entry)
-        if idx is not None and idx > 0 and idx < len(g.cf.images):
-            sel = idx
-
-    if sel == -1:
-        print "No kernel image selected!"
-        sys.exit(1)
-
-    img = g.cf.images[sel]
-    print "Going to boot %s" %(img.title)
-    print "  kernel: %s" %(img.kernel[1],)
-    if img.initrd:
-        print "  initrd: %s" %(img.initrd[1],)
-
-    if isconfig:
-        print "  args: %s" %(img.args,)
-        sys.exit(0)
-        
+        chosencfg = incfg
+     
     offset = 0
     if is_disk_image(file):
         offset = get_active_offset(file)
@@ -513,22 +544,26 @@ if __name__ == "__main__":
     # read the kernel and initrd onto the hostfs
     fs = fsimage.open(file, offset)
 
-    kernel = fs.open_file(img.kernel[1],).read()
-    (tfd, fn) = tempfile.mkstemp(prefix="boot_kernel.",
dir="/var/lib/xen")
-    os.write(tfd, kernel)
+    data = fs.open_file(chosencfg["kernel"]).read()
+    (tfd, bootcfg["kernel"]) =
tempfile.mkstemp(prefix="boot_kernel.", dir="/var/lib/xen")
+    os.write(tfd, data)
     os.close(tfd)
-    sxp = "linux (boot_kernel %s)" %(fn,)
-
-    if img.initrd:
-        initrd = fs.open_file(img.initrd[1],).read()
-        (tfd, fn) = tempfile.mkstemp(prefix="boot_ramdisk.",
dir="/var/lib/xen")
-        os.write(tfd, initrd)
+
+    if chosencfg["ramdisk"]:
+        data = fs.open_file(chosencfg["ramdisk"],).read()
+        (tfd, bootcfg["ramdisk"]) =
tempfile.mkstemp(prefix="boot_ramdisk.", dir="/var/lib/xen")
+        os.write(tfd, data)
         os.close(tfd)
-        sxp += "(boot_ramdisk %s)" %(fn,)
-    else:
-        initrd = None
-    sxp += "(args ''%s'')" %(img.args,)
+
+    sxp = "linux (boot_kernel %s)" % bootcfg["kernel"]
+    if bootcfg["ramdisk"]:
+        sxp += "(boot_ramdisk %s)" % bootcfg["ramdisk"]
+    if incfg["kernel"]:
+        sxp += "(kernel %s)" % incfg["kernel"]
+    if incfg["ramdisk"]:
+        sxp += "(ramdisk %s)" % incfg["ramdisk"]
+    if chosencfg["args"]:
+        sxp += "(args \"%s\")" %
chosencfg["args"]
 
     sys.stdout.flush()
     os.write(fd, sxp)
-    
diff --git a/tools/python/xen/xend/XendBootloader.py
b/tools/python/xen/xend/XendBootloader.py
--- a/tools/python/xen/xend/XendBootloader.py
+++ b/tools/python/xen/xend/XendBootloader.py
@@ -46,6 +46,16 @@ def bootloader(blexec, disk, quiet = 0, 
     child = os.fork()
     if (not child):
         args = [ blexec ]
+        if imgcfg:
+            kernel = sxp.child_value(imgcfg, "kernel")
+            ramdisk = sxp.child_value(imgcfg, "ramdisk")
+            kargs = sxp.child_value(imgcfg, "args")
+            if kernel:
+                args.append("--kernel=%s" % kernel)
+            if ramdisk:
+                args.append("--ramdisk=%s" % ramdisk)
+            if kargs:
+                args.append("--args=%s" % kargs)
         if quiet:
             args.append("-q")
         args.append("--output=%s" % fifo)

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

# HG changeset patch
# User john.levon@sun.com
# Date 1163095817 28800
# Node ID 6d2a4f2c5d6c23e1c5a096eef0112303047b2c78
# Parent  aebe98c33a6bc0cd633d915b9e0bad40718c29d6
Sniff for Solaris if not specified. This allows Solaris domU''s to
automatically
boot correctly without fiddling with domain configs.

Signed-off-by: John Levon <john.levon@sun.com>

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -65,6 +65,15 @@ def get_active_offset(file):
     # the first partition
     P1 = 446
     return struct.unpack("<L", buf[P1+8:P1+12])[0] * SECTOR_SIZE
+
+def open_fs(file):
+    offset = 0
+    if is_disk_image(file):
+        offset = get_active_offset(file)
+        if offset == -1:
+            raise RuntimeError, "Unable to find active partition on
disk"
+
+    return fsimage.open(file, offset)
 
 class GrubLineEditor(curses.textpad.Textbox):
     def __init__(self, screen, startx, starty, line = ""):
@@ -143,12 +152,12 @@ class GrubLineEditor(curses.textpad.Text
         
 
 class Grub:
-    def __init__(self, file, isconfig = False):
+    def __init__(self, file, fs = None):
         self.screen = None
         self.entry_win = None
         self.text_win = None
         if file:
-            self.read_config(file, isconfig)
+            self.read_config(file, fs)
 
     def draw_main_windows(self):
         if self.screen is None: #only init stuff once
@@ -295,8 +304,8 @@ class Grub:
             # else, we cancelled and should just go back
             break
 
-    def read_config(self, fn, isConfig = False):
-        """Read the given file to parse the config.  If
isconfig, then
+    def read_config(self, fn, fs = None):
+        """Read the given file to parse the config.  If fs =
None, then
         we''re being given a raw config file rather than a disk
image."""
         
         if not os.access(fn, os.R_OK):
@@ -304,38 +313,25 @@ class Grub:
 
         self.cf = grub.GrubConf.GrubConfigFile()
 
-        if isConfig:
+        if not fs:
             # set the config file and parse it
             self.cf.filename = fn
             self.cf.parse()
             return
 
-        offset = 0
-        if is_disk_image(fn):
-            offset = get_active_offset(fn)
-            if offset == -1:
-                raise RuntimeError, "Unable to find active partition on
disk"
-
-        # open the image and read the grub config
-        fs = fsimage.open(fn, offset)
-
-        if fs is not None:
-            grubfile = None
-            for f in ("/boot/grub/menu.lst",
"/boot/grub/grub.conf",
-                      "/grub/menu.lst", "/grub/grub.conf"):
-                if fs.file_exists(f):
-                    grubfile = f
-                    break
-            if grubfile is None:
-                raise RuntimeError, "we couldn''t find grub config
file in the image provided."
-            f = fs.open_file(grubfile)
-            buf = f.read()
-            del f
-            del fs
-            # then parse the grub config
-            self.cf.parse(buf)
-        else:
-            raise RuntimeError, "Unable to read filesystem" 
+        grubfile = None
+        for f in ("/boot/grub/menu.lst",
"/boot/grub/grub.conf",
+                  "/grub/menu.lst", "/grub/grub.conf"):
+            if fs.file_exists(f):
+                grubfile = f
+                break
+        if grubfile is None:
+            raise RuntimeError, "we couldn''t find grub config
file in the image provided."
+        f = fs.open_file(grubfile)
+        buf = f.read()
+        del f
+        # then parse the grub config
+        self.cf.parse(buf)
 
     def run(self):
         timeout = int(self.cf.timeout)
@@ -431,15 +427,16 @@ def get_entry_idx(cf, entry):
 
     return None
 
-def run_grub(file, isconfig, entry):
+def run_grub(file, entry, fs):
     global g
+    global sel
 
     def run_main(scr, *args):
         global sel
         global g
         sel = g.run()
 
-    g = Grub(file, isconfig)
+    g = Grub(file, fs)
     if interactive:
         curses.wrapper(run_main)
     else:
@@ -457,21 +454,48 @@ def run_grub(file, isconfig, entry):
 
     img = g.cf.images[sel]
 
+    grubcfg = { "kernel": None, "ramdisk": None,
"args": None }
+
     grubcfg["kernel"] = img.kernel[1]
-    grubcfg["ramdisk"] = img.initrd[1]
-    grubcfg["args"] = img.args[1]
-
-    print "Going to boot %s" %(img.title)
-    print "  kernel: %s" % grubcfg["kernel"]
     if img.initrd:
-        print "  initrd: %s" % grubcfg["ramdisk"]
-
-    if isconfig:
-        print "  args: %s" % grubcfg["args"]
-        sys.exit(0)
-        
+        grubcfg["ramdisk"] = img.initrd[1]
+    if img.args:
+        grubcfg["args"] = img.args
+
     return grubcfg
 
+# If nothing has been specified, look for a Solaris domU. If found, perform the
+# necessary tweaks.
+def sniff_solaris(fs, cfg):
+    if not fs.file_exists("/platform/i86xen/kernel/unix"):
+        return cfg
+    
+    # darned python
+    longmode = (sys.maxint != 2147483647L)
+    if not longmode:
+        longmode = os.uname()[4] == "x86_64"
+    if not longmode:
+        if (os.access("/usr/bin/isainfo", os.R_OK) and
+            os.popen("/usr/bin/isainfo -b").read() ==
"64\n"):
+            longmode = True
+
+    if not cfg["kernel"]:
+        cfg["kernel"] = "/platform/i86xen/kernel/unix";
+        if longmode:
+            cfg["kernel"] =
"/platform/i86xen/kernel/amd64/unix";
+        cfg["ramdisk"] = "/platform/i86pc/boot_archive"
+
+    # we need to pass the path to the kernel as the first argument, but only if
+    # it''s not present in args already.
+    args = cfg["args"]
+    cfg["args"] = cfg["kernel"] + " "
+    if args:
+        if string.split(args)[0] == cfg["kernel"]:
+            cfg["args"] = ""
+        cfg["args"] += args
+
+    return cfg
+ 
 if __name__ == "__main__":
     sel = None
     
@@ -497,11 +521,11 @@ if __name__ == "__main__":
     isconfig = False
 
     # what was passed in
-    incfg = { "kernel": None, "ramdisk": None,
"args": None };
-    # what grub chose
-    chosencfg = { "kernel": None, "ramdisk": None,
"args": None };
+    incfg = { "kernel": None, "ramdisk": None,
"args": None }
+    # what grub or sniffing chose
+    chosencfg = { "kernel": None, "ramdisk": None,
"args": None }
     # what to boot
-    bootcfg = { "kernel": None, "ramdisk": None,
"args": None };
+    bootcfg = { "kernel": None, "ramdisk": None,
"args": None }
 
     for o, a in opts:
         if o in ("-q", "--quiet"):
@@ -524,25 +548,26 @@ if __name__ == "__main__":
         elif o in ("--isconfig",):
             isconfig = True
 
-
     if output is None or output == "-":
         fd = sys.stdout.fileno()
     else:
         fd = os.open(output, os.O_WRONLY)
 
-    if not incfg["kernel"]:
-        chosencfg = run_grub(file, isconfig, entry)
-    else:
-        chosencfg = incfg
-     
-    offset = 0
-    if is_disk_image(file):
-        offset = get_active_offset(file)
-        if offset == -1:
-            raise RuntimeError, "Unable to find active partition on
disk"
-
-    # read the kernel and initrd onto the hostfs
-    fs = fsimage.open(file, offset)
+    # debug
+    if isconfig:
+        chosencfg = run_grub(file, entry)
+        print "  kernel: %s" % chosencfg["kernel"]
+        if img.initrd:
+            print "  initrd: %s" % chosencfg["ramdisk"]
+        print "  args: %s" % chosencfg["args"]
+        sys.exit(0)
+
+    fs = open_fs(file)
+
+    chosencfg = sniff_solaris(fs, incfg)
+
+    if not chosencfg["kernel"]:
+        chosencfg = run_grub(file, entry, fs)
 
     data = fs.open_file(chosencfg["kernel"]).read()
     (tfd, bootcfg["kernel"]) =
tempfile.mkstemp(prefix="boot_kernel.", dir="/var/lib/xen")

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

At 19:20 +0000 on 09 Nov (1163100017), John Levon wrote:
> pygrub-automatic
> 
> 	- Default to using pygrub if there''s no kernel and no bootloader
> 	  specified.
Sure, that seems like a good idea.  This should be documented in 
tools/examples/xmexample* too.
 
> pygrub-default
> 
> 	- Make pygrub be the "default" bootloader and understand that
> 	  keyword.
I don''t understand why this is needed.  Why not just not set a
bootloader= option if you want the default?
> pygrub-kernel-config
> pygrub-no-grub
> 
> 	- The combination of these patches allows pygrub FS code to be
> 	  used in a hands-off manner. Note that this can also be used to
> 	  fix the problem pygrub currently has with restart, by
> 	  specifying the kernel/ramdisk explicitly in the config. The
> 	  split between "boot kernel" and "configured kernel"
might need
> 	  some xen api/lifecycle changes too?
Hmmm.  I like the idea of splitting the bootloader''s suggested
kernel/etc from the config file''s one, but I don''t think the
bootloader
should be involved in that at all.  I''d rather have Xend be able to
make
a decision based on all the information from both sources.   This needs
a bit more thought generally.
> pygrub-solaris
> 
> 	- Solaris domU''s will typically never change the kernel/ramdisk
> 	  path they use, except if someone''s debugging etc. Add some
> 	  logic to pygrub so it knows what to do.
Autodetecting Solaris seems fine, but from that patch it looks like
pygrub will always choose Solaris even if there''s a grub menu.lst in
the
guest.  I''d prefer that to be the other way around: always use the menu
file, even if we think we can see Solaris.

Could this code, instead of just overriding the menu code entirely, just
add another line to the grub menu that contains the default Solaris
config?  That way if there''s no menu.lst, you get a single-item menu
for
Solaris, and even if there is a menu.lst, you still get the choice of a
"vanilla" Solaris boot.  Something like:

  sol_lines = ["title Default Solaris boot",
               "kernel %s %s" % (sol_krnl, sol_args),
	       "ramdisk /platform/i86pc/boot_archive" ]
  sol_img = grub.GrubConf.GrubImage(sol_lines)
  g.cf.images.append(sol_img)

Cheers,

Tim.

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

On Fri, Nov 10, 2006 at 11:05:27AM +0000, Tim Deegan wrote:
> > 	- Make pygrub be the "default" bootloader and understand
that
> > 	  keyword.
> 
> I don''t understand why this is needed.  Why not just not set a
> bootloader= option if you want the default?
I don''t understand your point. The name and location of
''pygrub'' should
be an internal detail unless somebody /wants/ to use a different
bootloader. It''s a natural follow-on for the first patch.
> > pygrub-kernel-config
> > pygrub-no-grub
> > 
> > 	- The combination of these patches allows pygrub FS code to be
> > 	  used in a hands-off manner. Note that this can also be used to
> > 	  fix the problem pygrub currently has with restart, by
> > 	  specifying the kernel/ramdisk explicitly in the config. The
> > 	  split between "boot kernel" and "configured
kernel" might need
> > 	  some xen api/lifecycle changes too?
> 
> Hmmm.  I like the idea of splitting the bootloader''s suggested
> kernel/etc from the config file''s one, but I don''t think
the bootloader
> should be involved in that at all.  I''d rather have Xend be able
to make
> a decision based on all the information from both sources.   This needs
> a bit more thought generally.
Could you expand a bit more on what you mean here?
> > 	- Solaris domU''s will typically never change the
kernel/ramdisk
> > 	  path they use, except if someone''s debugging etc. Add some
> > 	  logic to pygrub so it knows what to do.
> 
> Autodetecting Solaris seems fine, but from that patch it looks like
> pygrub will always choose Solaris even if there''s a grub menu.lst
in the
> guest.  I''d prefer that to be the other way around: always use the
menu
> file, even if we think we can see Solaris.
There /is/ no menu file on Solaris domU.
> Could this code, instead of just overriding the menu code entirely, just
> add another line to the grub menu that contains the default Solaris
> config?  That way if there''s no menu.lst, you get a single-item
menu for
> Solaris, and even if there is a menu.lst, you still get the choice of a
> "vanilla" Solaris boot.  Something like:
> 
>   sol_lines = ["title Default Solaris boot",
>                "kernel %s %s" % (sol_krnl, sol_args),
> 	       "ramdisk /platform/i86pc/boot_archive" ]
>   sol_img = grub.GrubConf.GrubImage(sol_lines)
>   g.cf.images.append(sol_img)
Why? I don''t understand why replacing a hands-off method with a load of
interactive gook is a step forward.

regards,
john

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

On Thu, Nov 09, 2006 at 07:22:40PM +0000, John Levon
wrote:
> 
> # HG changeset patch
> # User john.levon@sun.com
> # Date 1163095800 28800
> # Node ID 0da173ee886e13bd4116c7d085cd4a4704ffe279
> # Parent  7c80aaffe16f5e4037d10660d262ed7b22894b01
> Split out bootloader-derived parameters separately from
config-file-specified
> ones. This allows a config file to still specify kernel/ramdisk, even if a
> bootloader is being used.
I''m not sure this is a good idea - its essentially completely inverting
the current semantics for bootloader/kernel params in the config files.
Currently a ''kernel'' parameter will always override a
''bootloader'' param,
but with this change a ''bootloader'' param will always override
a ''kernel''
param. I can see that both approaches have their merits, I don''t think
we can ever pick one prioritization rule which satisfies everyone here
and thus for sake of compatability we should keep the current semantics
for kernel/booloader prioritization in the configs.
> diff --git a/tools/python/xen/xend/image.py
b/tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py
> +++ b/tools/python/xen/xend/image.py
> @@ -70,7 +70,9 @@ class ImageHandler:
>          self.vm = vm
>  
>          self.kernel = None
> +        self.boot_kernel = None
>          self.ramdisk = None
> +        self.boot_ramdisk = None
>          self.cmdline = None
>  
>          self.configure(imageConfig, deviceConfig)
> @@ -82,6 +84,14 @@ class ImageHandler:
>              return sxp.child_value(imageConfig, name, default)
>  
>          self.kernel = get_cfg("kernel")
> +        self.ramdisk = get_cfg("ramdisk", '''')
> +        self.boot_kernel = get_cfg("boot_kernel")
> +        if not self.boot_kernel:
> +            self.boot_kernel = self.kernel
> +        self.boot_ramdisk = get_cfg("boot_ramdisk")
> +        if not self.boot_ramdisk:
> +            self.boot_ramdisk = self.ramdisk
> +
This is also changing the semantics of the SEXPR exposed by XenD - previously
the (''vmlinux'') and (''initrd'') parameters
would refer to the live kernel
in the guest, but now you''d have to look at the boot_XXX variants
instead.

If we did want to change the prioritzation such that bootloader overrides
any explicit kernel/initrd, then I think we should just have the values
from bootloader directly overwrite the existing named SXPR fields rather
than adding new boot_XX fields.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

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

On Fri, Nov 10, 2006 at 08:14:28PM +0000, Daniel P. Berrange wrote:
> I''m not sure this is a good idea - its essentially completely
inverting
> the current semantics for bootloader/kernel params in the config files.
> Currently a ''kernel'' parameter will always override a
''bootloader'' param,
> but with this change a ''bootloader'' param will always
override a ''kernel''
> param. I can see that both approaches have their merits, I don''t
think
> we can ever pick one prioritization rule which satisfies everyone here
> and thus for sake of compatability we should keep the current semantics
> for kernel/booloader prioritization in the configs.
Taken in concert with the other patch, the ''kernel'' choice
will always
be respected if it''s set.
> >          self.kernel = get_cfg("kernel")
> > +        self.ramdisk = get_cfg("ramdisk",
'''')
> > +        self.boot_kernel = get_cfg("boot_kernel")
> > +        if not self.boot_kernel:
> > +            self.boot_kernel = self.kernel
> > +        self.boot_ramdisk = get_cfg("boot_ramdisk")
> > +        if not self.boot_ramdisk:
> > +            self.boot_ramdisk = self.ramdisk
> > +
> 
> This is also changing the semantics of the SEXPR exposed by XenD -
previously
> the (''vmlinux'') and (''initrd'')
parameters would refer to the live kernel
> in the guest, but now you''d have to look at the boot_XXX variants
instead.
>
> If we did want to change the prioritzation such that bootloader overrides
> any explicit kernel/initrd, then I think we should just have the values
> from bootloader directly overwrite the existing named SXPR fields rather
> than adding new boot_XX fields.
See the other patch. We could make it so kernel/initrd are always the
''live'' ones, than we have cfg_kernel/cfg_initrd or something,
but I
don''t really see the advantage in that, and the disadvantage is
reasonably obvious: if I want to lookup to see "what kernel is this",
I''d have to check first cfg_kernel then kernel. With boot_*, I can
always look in ''kernel'' and know that this is the one that the
.py
config chose.

regards
john

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

At 17:25 +0000 on 10 Nov (1163179547), John Levon wrote:
> On Fri, Nov 10, 2006 at 11:05:27AM +0000, Tim Deegan wrote:
> > I don''t understand why this is needed.  Why not just not set
a
> > bootloader= option if you want the default?
> 
> I don''t understand your point. The name and location of
''pygrub'' should
> be an internal detail unless somebody /wants/ to use a different
> bootloader. It''s a natural follow-on for the first patch.
Sorry, what I meant was, isn''t not setting the bootloader option at all
equivalent to setting it to "default"?  If you want to control whether
the bootloader is called at all, there could be two config options, say,
use_bootloader (default 0) and bootloader_path (default wherever pygrub is).
> > Hmmm.  I like the idea of splitting the bootloader''s
suggested
> > kernel/etc from the config file''s one, but I don''t
think the bootloader
> > should be involved in that at all.  I''d rather have Xend be
able to make
> > a decision based on all the information from both sources.   This
needs
> > a bit more thought generally.
> 
> Could you expand a bit more on what you mean here?
I think that it would be better not to pass the config file options into
pygrub and back out again: pygrub presumably has no business changing
those fields anyway.  In future, if pygrub supplies more configuration
options, will we have to add a command-line option for each one so it
can pass the config file versions through?  It just seems a bit messy.

Could Xend not just run the bootloader and then, say, fall back to the
values it got from the config file if it doesn''t get anything from the 
bootloader?
> Why? I don''t understand why replacing a hands-off method with a
load of
> interactive gook is a step forward.
It would let you dual-boot a linux/Solaris guest. :)  It would mean that
if you untarred a Solaris kernel in your linux guest your Grub menu
woudn''t just go away forever.  It would let you choose a
"backup" Solaris
kernel if your main one got toasted.   All the usual things a boot menu
is useful for. 

Cheers,

Tim. 

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

On Mon, Nov 13, 2006 at 10:09:01AM +0000, Tim Deegan wrote:
> > > I don''t understand why this is needed.  Why not just not
set a
> > > bootloader= option if you want the default?
> > 
> > I don''t understand your point. The name and location of
''pygrub'' should
> > be an internal detail unless somebody /wants/ to use a different
> > bootloader. It''s a natural follow-on for the first patch.
> 
> Sorry, what I meant was, isn''t not setting the bootloader option
at all
> equivalent to setting it to "default"?  If you want to control
whether
> the bootloader is called at all, there could be two config options, say,
> use_bootloader (default 0) and bootloader_path (default wherever pygrub
is).
No, if you specify kernel then it''s picked up from the dom0 filesystem.
And I want to avoid having to insist on the bootloader_path at all: the
common case (using pygrub) should just work.
> > > Hmmm.  I like the idea of splitting the bootloader''s
suggested
> > > kernel/etc from the config file''s one, but I
don''t think the bootloader
> > > should be involved in that at all.  I''d rather have Xend
be able to make
> > > a decision based on all the information from both sources.   This
needs
> > > a bit more thought generally.
> > 
> > Could you expand a bit more on what you mean here?
> 
> I think that it would be better not to pass the config file options into
> pygrub and back out again: pygrub presumably has no business changing
> those fields anyway.  In future, if pygrub supplies more configuration
> options, will we have to add a command-line option for each one so it
> can pass the config file versions through?  It just seems a bit messy.
I agree it''s messy. In fact I''d rather like to rework the
interface to
read the config from an fd and write it back to another. It''s a
requirement to pass the details in, otherwise we cannot say "load me
/this/ kernel". Equally I''d like to split the code up a bit
further into
"load this file" code and "run interactive pygrub" code.
> Could Xend not just run the bootloader and then, say, fall back to the
> values it got from the config file if it doesn''t get anything from
the
> bootloader?
That would enforce interactive usage.
> > Why? I don''t understand why replacing a hands-off method with
a load of
> > interactive gook is a step forward.
> 
> It would let you dual-boot a linux/Solaris guest. :)  It would mean that
> if you untarred a Solaris kernel in your linux guest your Grub menu
> woudn''t just go away forever.
hmm :)
> It would let you choose a "backup" Solaris kernel if your main
one got
> toasted.   All the usual things a boot menu is useful for. 
Plus all the problems. It''s trading the 99.9% case on Solaris
("boot the
kernel you always boot") for the rare exception ("I totally screwed up
my machine", "I''m a Solaris kernel developer running a
different
kernel") which can be handled much better via the .py config file.

regards,
john

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

At 13:42 +0000 on 13 Nov (1163425323), John Levon wrote:
> No, if you specify kernel then it''s picked up from the dom0
filesystem.
> And I want to avoid having to insist on the bootloader_path at all: the
> common case (using pygrub) should just work.
If we have a use-a-bootloader flag, which defaults to off if a kernel is
specified, and a which-bootloader-should-i-use path, which defaults to
wherever pygrub lives on this system, does that covers all the angles?

I find both the idea of having a "default" keyword at all and the idea
that explicitly requesting the default (!) has side-effects to be 
unintuitive.
> I agree it''s messy. In fact I''d rather like to rework the
interface to
> read the config from an fd and write it back to another. It''s a
> requirement to pass the details in, otherwise we cannot say "load me
> /this/ kernel".
Yes, I see.  So now I like the pygrub-nogrub patch more. :)
As it stands, passing a kernel= option causes pygrub not to bother
looking for menu options, yes?
> Equally I''d like to split the code up a bit further into
> "load this file" code and "run interactive pygrub"
code.
Hmmm.  Let me try and untangle what we require now...

1) Receive kernel, args and initrd choices from xm config.
2) Find and parse a GRUB menu.lst in a filesystem image.
3) Automatically detect a Solaris installation in a filesystem image.
4) Let the user choose and modify their boot options.
5) Extract a particular file from a filesystem image for booting.

We should always do all three of 1, 2 and 3: no such thing as too many
options.  I would like to see stage 4 available regardless of which of
stages 1, 2 and 3 provide boot options (including none).  Letting the
user choose seems like it should always be the right idea.

There''s also a question of weighting: if we run non-interactively (or
time out in interactive mode) what should we do by default?  My
inclination is to listen to menu.lst first, then autodetected kernels,
and then the config-file-provided kernel. 
> Plus all the problems. It''s trading the 99.9% case on Solaris
("boot the
> kernel you always boot") for the rare exception ("I totally
screwed up
> my machine", "I''m a Solaris kernel developer running a
different
> kernel") which can be handled much better via the .py config file.
I have a particular dislike of having to make changes to the xm config
file; as far as I''m concerned, a major goal of having all this
bootloader code is that eventually only "hardware" changes will need
to
be made in dom0 and *all* software configuration will be inside the
guest filesystem.

Cheers,

Tim.

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

On Mon, Nov 13, 2006 at 06:01:40PM +0000, Tim Deegan wrote:
> > No, if you specify kernel then it''s picked up from the dom0
filesystem.
> > And I want to avoid having to insist on the bootloader_path at all:
the
> > common case (using pygrub) should just work.
> 
> If we have a use-a-bootloader flag, which defaults to off if a kernel is
> specified, and a which-bootloader-should-i-use path, which defaults to
> wherever pygrub lives on this system, does that covers all the angles?
> 
> I find both the idea of having a "default" keyword at all and the
idea
> that explicitly requesting the default (!) has side-effects to be 
> unintuitive.
Hrm. I''d be OK with a use_bootloader {true,false} and a follow-on
bootload_path, yes. But this would break compatibility for .py scripts,
no? They''ve already broken (a little) in 3.0.3 so I''m not sure
if this
is a concern.
> > I agree it''s messy. In fact I''d rather like to
rework the interface to
> > read the config from an fd and write it back to another. It''s
a
> > requirement to pass the details in, otherwise we cannot say "load
me
> > /this/ kernel".
> 
> Yes, I see.  So now I like the pygrub-nogrub patch more. :)
> As it stands, passing a kernel= option causes pygrub not to bother
> looking for menu options, yes?
Correct.
> > Equally I''d like to split the code up a bit further into
> > "load this file" code and "run interactive pygrub"
code.
> 
> Hmmm.  Let me try and untangle what we require now...
> 
> 1) Receive kernel, args and initrd choices from xm config.
> 2) Find and parse a GRUB menu.lst in a filesystem image.
> 3) Automatically detect a Solaris installation in a filesystem image.
> 4) Let the user choose and modify their boot options.
> 5) Extract a particular file from a filesystem image for booting.
> 
> We should always do all three of 1, 2 and 3: no such thing as too many
> options.
I''m not sure why we''d do 2) if 3) succeeds. What action would
you like
to result?
> I would like to see stage 4 available regardless of which of
> stages 1, 2 and 3 provide boot options (including none).  Letting the
> user choose seems like it should always be the right idea.
If I''ve specified stuff in my .py file it seems at the least intrusive
to then ask me to confirm them.
> There''s also a question of weighting: if we run non-interactively
(or
> time out in interactive mode) what should we do by default?  My
> inclination is to listen to menu.lst first, then autodetected kernels,
> and then the config-file-provided kernel. 
This seems exactly backwards to me. In normal operation, nobody will
provide parameters in the .py file, letting either interactive boot or
the defaults happen. So when they /do/, we should apply it.

I''d also like to avoid having to parse menu.lst if we autodetect: the
parser is rather brittle, and it''s pointless for the only case we
autodetect right now (Solaris).
> > Plus all the problems. It''s trading the 99.9% case on Solaris
("boot the
> > kernel you always boot") for the rare exception ("I totally
screwed up
> > my machine", "I''m a Solaris kernel developer
running a different
> > kernel") which can be handled much better via the .py config
file.
> 
> I have a particular dislike of having to make changes to the xm config
> file; as far as I''m concerned, a major goal of having all this
> bootloader code is that eventually only "hardware" changes will
need to
> be made in dom0 and *all* software configuration will be inside the
> guest filesystem.
Understandable; it''s worth noting that these changes actually /reduce/
the need for this except in development/recovery scenarios. The fact
that there''s no real possibility of sane interaction between
''xm reboot''
and pygrub interactive /without/ having the config override menu.lst is
a pretty strong argument for my approach too. (Edit a line in pygrub and
boot with that: on reboot, you can either choose the default in menu.lst
(your priority order), or re-use what was specified (mine)).

regards
john

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