Skip to contentSkip to conversation

Fix pygrub --isconfig

PATCH In review John Haxby 2 participants 4 messages Jan 20, 2011 – Jan 21, 2011

lore
Share & tools
lore

John Haxby

author
#0 lore raw

PATCH 1/1 Fix pygrub --isconfig

Recentlly, when trying to get to the bottom of some strange booting problem, I discovered that

    pyrgub --isconfig <image file>

usefully reports the kernel, initrd (ramdisk) and arguments that would be used to boot a kernel. Well, actually, what it reported was an error. This patch makes it work (and jolly useful it is too). The patch also removes a spurious "print >>sys.stderr" that seems to have been left in from some previous debugging.

jch

tools/pygrub/src/pygrub +13 -9
diff -r f47b924f74af tools/pygrub/src/pygrub
@@ -400,7 +400,6 @@
 
         for f,parser in cfg_list:
             if fs.file_exists(f):
-                print >>sys.stderr, "Using %s to parse %s" % (parser,f)
                 self.cf = parser()
                 self.cf.filename = f
                 break
@@ -687,14 +686,6 @@
     else:
         fd = os.open(output, os.O_WRONLY)
 
-    # debug
-    if isconfig:
-        chosencfg = run_grub(file, entry, fs, incfg["args"])
-        print "  kernel: %s" % chosencfg["kernel"]
-        if chosencfg["ramdisk"]:
-            print "  initrd: %s" % chosencfg["ramdisk"]
-        print "  args: %s" % chosencfg["args"]
-        sys.exit(0)
 
     # if boot filesystem is set then pass to fsimage.open
     bootfsargs = '"%s"' % incfg["args"]
@@ -704,6 +695,19 @@
     else:
         fs = fsimage.open(file, get_fs_offset(file))
 
+    # debug
+    if isconfig:
+        chosencfg = run_grub(file, entry, fs, incfg["args"])
+        if chosencfg.has_key("kernel"):
+            print "  kernel: %s" % chosencfg["kernel"]
+        if chosencfg.has_key("ramdisk"):
+            print "  initrd: %s" % chosencfg["ramdisk"]
+        if chosencfg.has_key("args"):
+            print "  args: %s" % chosencfg["args"]
+        if not chosencfg:
+            print "  no config"
+        sys.exit(0)
+
     chosencfg = sniff_solaris(fs, incfg)
 
     if not chosencfg["kernel"]:

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

#1 lore raw

John Haxby writes ("[Xen-devel] [PATCH] Fix pygrub --isconfig"):

Quoted 10 lines

Recentlly, when trying to get to the bottom of some strange booting problem, I discovered that

    pyrgub --isconfig <image file>

usefully reports the kernel, initrd (ramdisk) and arguments that would be used to boot a kernel. Well, actually, what it reported was an error. This patch makes it work (and jolly useful it is too). The patch also removes a spurious "print >>sys.stderr" that seems to have been left in from some previous debugging.

Thanks, but this patch doesn't apply to xen-unstable.hg. xen-unstable has a loop "for offset in part_offs" surrounding the code you seem to be trying to patch.

Is it important to check for "isconfig" before running sniff_solaris and sniff_netware ?

Ian.

author
#2 lore raw
Quoted 6 lines · Ian Jackson

John Haxby writes ("[Xen-devel] [PATCH] Fix pygrub --isconfig"):

Quoted 5 lines

Recentlly, when trying to get to the bottom of some strange booting problem, I discovered that

    pyrgub --isconfig <image file>
Quoted 8 lines
Quoted 5 lines

usefully reports the kernel, initrd (ramdisk) and arguments that would be used to boot a kernel. Well, actually, what it reported was an error. This patch makes it work (and jolly useful it is too). The patch also removes a spurious "print >>sys.stderr" that seems to have been left in from some previous debugging.

Thanks, but this patch doesn't apply to xen-unstable.hg. xen-unstable has a loop "for offset in part_offs" surrounding the code you seem to be trying to patch.

Sorry. This patch was originally from something rather older which I then applied to xen-4.0-testing by mistake.
I have attached an updated patch against xen-unstable.hg.

Quoted 2 lines

Is it important to check for "isconfig" before running sniff_solaris and sniff_netware ?

No, it should really be checked after as (presumably) the intention is to report what would actually be chosen.

Although I have attached the patch to move the isconfig stuff to the point where the chosenconfig is finally known "pygrub --isconfig" is functionally equivalent to "pygrub --not-really --output-format simple --output -" so it might be better just to remove the isconfig code altogether. You could make it a synonym for the longer command, but as isconfig has been unloved for quite a while it probably isn't worth it.

jch

tools/pygrub/src/pygrub +13 -10
diff -r 003acf02d416 tools/pygrub/src/pygrub
@@ -404,7 +404,6 @@
 
         for f,parser in cfg_list:
             if fs.file_exists(f):
-                print >>sys.stderr, "Using %s to parse %s" % (parser,f)
                 self.cf = parser()
                 self.cf.filename = f
                 break
@@ -725,15 +724,6 @@
     else:
         fd = os.open(output, os.O_WRONLY)
 
-    # debug
-    if isconfig:
-        chosencfg = run_grub(file, entry, fs, incfg["args"])
-        print "  kernel: %s" % chosencfg["kernel"]
-        if chosencfg["ramdisk"]:
-            print "  initrd: %s" % chosencfg["ramdisk"]
-        print "  args: %s" % chosencfg["args"]
-        sys.exit(0)
-
     # if boot filesystem is set then pass to fsimage.open
     bootfsargs = '"%s"' % incfg["args"]
     bootfsgroup = re.findall('zfs-bootfs=(.*?)[\s\,\"]', bootfsargs)
@@ -769,6 +759,19 @@
             fs = None
             continue
 
+    # debug
+    if isconfig:
+        if fs:
+            if chosencfg.has_key("kernel"):
+                print "  kernel: %s" % chosencfg["kernel"]
+            if chosencfg.has_key("ramdisk"):
+                print "  initrd: %s" % chosencfg["ramdisk"]
+            if chosencfg.has_key("args"):
+                print "  args: %s" % chosencfg["args"]
+        else:
+            print "  Unable to find partition containing kernel"
+        sys.exit(0)
+
     # Did looping through partitions find us a kernel?
     if not fs:
         raise RuntimeError, "Unable to find partition containing kernel"

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

#3 lore raw

John Haxby writes ("Re: [Xen-devel] [PATCH] Fix pygrub --isconfig"):

Quoted 6 lines

Although I have attached the patch to move the isconfig stuff to the point where the chosenconfig is finally known "pygrub --isconfig" is functionally equivalent to "pygrub --not-really --output-format simple --output -" so it might be better just to remove the isconfig code altogether. You could make it a synonym for the longer command, but as isconfig has been unloved for quite a while it probably isn't worth it.

Thanks for investigating. Under the circumstances I think we probably should just leave things as they are for 4.1 and probably make isconfig an alias for this in 4.2.

Ian.