Re: [Xen-devel] pygrub fails to read single partition with grub bootsector in 4.1.0-rc

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

Thanks Michael.

On Sun, 2011-01-30 at 19:57 +0000, M A Young wrote:
> I have a single partition (actually under lvm) which starts with a grub 
> boot sector.

Do you mean an unpartitioned disk (but with an MBR) as opposed to a disk
with a single partition?

>  pygrub in 4.0.1 coped with this successfully, but under 
> 4.1.0-rc2 I get the error
> Traceback (most recent call last):
>    File "pygrub.orig", line 773, in <module>
>      if not fs:
> NameError: name 'fs' is not defined
> 
> The problem is when you install grub on a partition (it can be on the MBR 
> or on the boot sector of a partition), it installs an MBR-like boot 
> sector, in particular ending with 0xaa55 in bytes 510 and 511.
> 
> In 4.1.0 pygrub sees this and decides in get_partition_offsets() that it 
> is looking at an MBR, however when it checks the offsets it finds they are 
> all zero so returns an empty list of offsets to try, resulting in the 
> error above (in 4.0.1 the default is to return the offset in the first 
> partition, which is 0 so it worked). The attached patch aims to detect 
> this situation an return an offset of zero in this case, though perhaps it 
> makes sense to default to an offset of 0 rather than a blank list if no 
> appropriate offsets are detected.

Hrm, it's not clear to me that this situation is a valid one per what a
native BIOS would find acceptable.

AIUI (although I'm no expert here) the valid choices for disk contents
are either a completely raw disk (i.e. no MBR at all) or a disk with an
MBR and a partition table (possibly only containing a single large
partition) but not something in between as you have here. Strictly
speaking I suspect this is a bug in whatever created the MBR though.

That said I'm inclined to think that we should be reasonably tolerant in
what we accept as input and that there is little harm in trying to treat
things as a whole disk image if we would have otherwise failed anyway
(worst case is we simply fail a bit later on). pygrub is read-only so
its probably not going to eat anything.

So I think it is acceptable that when get_partition_offsets() detects
this specific situation it behaves as if the is_disk_image test had
succeeded, i.e. returns [0] (which I presume is what part_offs.append(0)
you add effectively means since part_offs ==[] at this point). So:

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

However we need a signed-off-by from you too before we can consider
applying the patch.

Ian.


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