pygrub fails with NameError: name 'fs' is not defined

pygrub fails with NameError: name 'fs' is not defined

[Sven Köhler]

Hi,

I get this rather puzzling error:

Traceback (most recent call last):
  File "/usr/lib64/xen/bin/pygrub", line 885, in <module>
    if not fs:
NameError: name 'fs' is not defined

This is pygrub from xen-tools 4.4.0.
I found a rather old thread about the same issue:
http://old-list-archives.xenproject.org/xen-devel/2011-01/msg02076.html

But it seems like the problem has already been fixed and indeed there
are some similarities, namely that /dev/md4 (which is xvda1 in the domU)
does not contain a partition table.

Now the issue here seems to be that boot sector of /dev/md4 is a little
screwed up. Namely, fdisk -l /dev/md4 seems to report an empty partition
table. This is not the case for other domU I have. Maybe, the
boot-sector is to MBR like after running grub-install /dev/xvda1 inside
the domU. Or maybe after trying LILO. I don't know.

Anyhow, the error above seems like a programming mistake (variable fs is
not defined? Sorry, I don't know python). So I'm hereby reporting it.

Maybe I will switch to a partitioned /dev/xvda instead of providing
/dev/xvda1 directly to the domU.

Has there been any improvement regarding pygrub for the case of
unpartitioned disks? It seems like it would be easy to support that case
more reliably if xl would pass some more information to pygrub. However,
in the thread from the archive, Ian made it sound like that would be
complicated. (In fact, if pygrub would know that the device name was
xvda1 and not xvda it could stop looking for a partition table).


Regards,
  Sven

Re: pygrub fails with NameError: name 'fs' is not defined

[Sven Köhler]

Am 10.05.2014 03:12, schrieb Sven Köhler:
> Now the issue here seems to be that boot sector of /dev/md4 is a little
> screwed up. Namely, fdisk -l /dev/md4 seems to report an empty partition
> table. This is not the case for other domU I have. Maybe, the
> boot-sector is to MBR like after running grub-install /dev/xvda1 inside
> the domU. Or maybe after trying LILO. I don't know.

Yes, it was GRUB, it seems. The first sector of md4 contained GRUB code
and also the MBR signature (0x55 0xAA at offset 510) and so pygrub must
have thought that he's dealing with an MBR partition table. I should
point out, that the partition table was empty (all zeros) and this must
have lead to the undefined fs variable. The case of an empty partition
table seems to lead to this bug.


Regards,
  Sven

[PATCH] tools/pygrub: Fix error handling if no valid partitions are found

[Andrew Cooper]

If no partitions at all are found, pygrub never creates the name 'fs',
resulting in a NameError indicating the lack of fs, rather than a
RuntimeError explaining that no partitions were found.

Set fs to None right at the start, and use the pythonic idiom "if fs is None:"
to protect against otherwise valid values for fs which compare equal to
0/False.

Reported-by: Sven Köhler <sven.koehler@gmail.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>

---
---
 tools/pygrub/src/pygrub |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 54fecee..4c35f9d 100644
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -747,7 +747,7 @@ if __name__ == "__main__":
         usage()
         sys.exit(1)
     file = args[0]
-        
+    fs = None
     output = None
     entry = None
     interactive = True
@@ -869,7 +869,7 @@ if __name__ == "__main__":
         sys.exit(0)
 
     # Did looping through partitions find us a kernel?
-    if not fs:
+    if fs is None:
         raise RuntimeError, "Unable to find partition containing kernel"
 
     bootcfg["kernel"] = copy_from_image(fs, chosencfg["kernel"], "kernel",
-- 
1.7.10.4


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

Re: [PATCH] tools/pygrub: Fix error handling if no valid partitions are found

[Ian Jackson]

Andrew Cooper writes ("[PATCH] tools/pygrub: Fix error handling if no valid partitions are found"):
> If no partitions at all are found, pygrub never creates the name 'fs',
> resulting in a NameError indicating the lack of fs, rather than a
> RuntimeError explaining that no partitions were found.
> 
> Set fs to None right at the start, and use the pythonic idiom "if fs is None:"
> to protect against otherwise valid values for fs which compare equal to
> 0/False.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks,
Ian.

Re: [PATCH] tools/pygrub: Fix error handling if no valid partitions are found

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH] tools/pygrub: Fix error handling if no valid partitions are found"):
> Andrew Cooper writes ("[PATCH] tools/pygrub: Fix error handling if no valid partitions are found"):
> > If no partitions at all are found, pygrub never creates the name 'fs',
> > resulting in a NameError indicating the lack of fs, rather than a
> > RuntimeError explaining that no partitions were found.
> > 
> > Set fs to None right at the start, and use the pythonic idiom "if fs is None:"
> > to protect against otherwise valid values for fs which compare equal to
> > 0/False.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Backported to 4.4, 4.3, 4.2.  Will push shortly.

Ian.