[PATCH] pygrub: do not overload RuntimeError for "no menu.lst found"

[PATCH] pygrub: do not overload RuntimeError for "no menu.lst found"

[Paolo Bonzini]

From: pbonzini@redhat.com

# HG changeset patch
# User Paolo Bonzini <pbonzini@redhat.com>
# Date 1319096986 -7200
# Node ID bd1f7361d3d7f4c767af21317fb4ec7ea1372f42
# Parent  1b110e895e285f43f14532e14c77597e54a0bcd2
pygrub will still try the next partition if run_grub exits with a
"real" error, thus hiding the root cause from the trace.  Defining
a separate exception for "no bootloader config file found" avoids
this.

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -29,6 +29,9 @@ import grub.ExtLinuxConf
 
 PYGRUB_VER = 0.6
 
+class NotFoundError(RuntimeError):
+    pass
+
 def enable_cursor(ison):
     if ison:
         val = 2
@@ -412,7 +412,7 @@ class Grub:
                 self.cf.filename = f
                 break
         if self.__dict__.get('cf', None) is None:
-            raise RuntimeError, "couldn't find bootloader config file in the image provided."
+            raise NotFoundError, "couldn't find bootloader config file in the image provided."
         f = fs.open_file(self.cf.filename)
         buf = f.read()
         del f
@@ -763,9 +766,9 @@ if __name__ == "__main__":
                 break
             fs = None
 
-        except:
+        except (IOError, NotFoundError):
             # IOErrors raised by fsimage.open
-            # RuntimeErrors raised by run_grub if no menu.lst present
+            # NotFoundError raised by run_grub if no menu.lst present
             fs = None
             continue

Re: [PATCH] pygrub: do not overload RuntimeError for "no menu.lst found"

[Ian Campbell]

On Thu, 2011-10-20 at 09:10 +0100, Paolo Bonzini wrote:
> From: pbonzini@redhat.com
> 
> # HG changeset patch
> # User Paolo Bonzini <pbonzini@redhat.com>
> # Date 1319096986 -7200
> # Node ID bd1f7361d3d7f4c767af21317fb4ec7ea1372f42
> # Parent  1b110e895e285f43f14532e14c77597e54a0bcd2
> pygrub will still try the next partition if run_grub exits with a
> "real" error, thus hiding the root cause from the trace.  Defining
> a separate exception for "no bootloader config file found" avoids
> this.

Are all the other RuntimeError's ok or should we be defining a bunch of
more specific exceptions?

We need a signed-off-by for this change, per DCO:
http://wiki.xen.org/xenwiki/SubmittingXenPatches

Thanks,
Ian.

> 
> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -29,6 +29,9 @@ import grub.ExtLinuxConf
>  
>  PYGRUB_VER = 0.6
>  
> +class NotFoundError(RuntimeError):
> +    pass
> +
>  def enable_cursor(ison):
>      if ison:
>          val = 2
> @@ -412,7 +412,7 @@ class Grub:
>                  self.cf.filename = f
>                  break
>          if self.__dict__.get('cf', None) is None:
> -            raise RuntimeError, "couldn't find bootloader config file in the image provided."
> +            raise NotFoundError, "couldn't find bootloader config file in the image provided."
>          f = fs.open_file(self.cf.filename)
>          buf = f.read()
>          del f
> @@ -763,9 +766,9 @@ if __name__ == "__main__":
>                  break
>              fs = None
>  
> -        except:
> +        except (IOError, NotFoundError):
>              # IOErrors raised by fsimage.open
> -            # RuntimeErrors raised by run_grub if no menu.lst present
> +            # NotFoundError raised by run_grub if no menu.lst present
>              fs = None
>              continue
>  
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] pygrub: do not overload RuntimeError for "no menu.lst found"

[Paolo Bonzini]

On 10/20/2011 10:32 AM, Ian Campbell wrote:
> >  From:pbonzini@redhat.com
> >
> >  # HG changeset patch
> >  # User Paolo Bonzini<pbonzini@redhat.com>
> >  # Date 1319096986 -7200
> >  # Node ID bd1f7361d3d7f4c767af21317fb4ec7ea1372f42
> >  # Parent  1b110e895e285f43f14532e14c77597e54a0bcd2
> >  pygrub will still try the next partition if run_grub exits with a
> >  "real" error, thus hiding the root cause from the trace.  Defining
> >  a separate exception for "no bootloader config file found" avoids
> >  this.
>
> Are all the other RuntimeError's ok or should we be defining a bunch of
> more specific exceptions?

get_solaris_slice triggers RuntimeErrors which are eaten by the caller. 
  That's not too nice and could also be replaced by NotFoundError, but 
it does not hide as many possible errors as run_grub.

read_config triggers a RuntimeError if it cannot find the file image 
itself.  It is called via run_grub (which calls the Grub constructor), 
so it will become a hard failure after this patch.  This is intended; 
not finding the file image is very wrong.

Grub2ConfigFile triggers RuntimeErrors if it fails to parse the grub.cfg 
file (wrong menuentry nesting, missing braces, unexpected braces).  This 
is also called via run_grub and will become a hard failure.  The new 
behavior is better than looking for another boot partition, since the 
actual error is hidden and anyway it's very unlikely that there will be 
two boot partitions.

Other occurrences are only for the case of __name__ == "__main__", so 
they are unaffected by the patch.

> We need a signed-off-by for this change, per DCO:
> http://wiki.xen.org/xenwiki/SubmittingXenPatches

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Shall I resend, or can you add it while applying?

Paolo