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

[M A Young <m.a.young@xxxxxxxxxxxx>]

On Mon, 31 Jan 2011, Ian Campbell wrote:

> 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?
Yes. It is what you get if have grub installed on a partition rather than 
in an MBR, and then tell xen to map that partition to a partition in the 
guest.
> >  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.
I don't know whether it is actually needed for a bootable partition or 
not, but it does seem to be common practice to stick aa55 at the end of 
the first 512 byte sector of a bootable partition (I have just checked a 
Windows XP partition and it does the same though with text in what would 
be the partition table so 4.0.1 pygrub probably does something weird with 
it).
> 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.
The patch in my previous email is
Signed-off-by: Michael Young <m.a.young@xxxxxxxxxxxx>

        Michael Young

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