This patch is part of a fix I applied to Fedora 16 to get pygrub to boot a Fedora 16 guest (see https://bugzilla.redhat.com/show_bug.cgi?id=745335 ). By default Fedora 16 installs a Bios boot partition as the first GPT partition to contain grub2 boot code, and the grub2 configuration files are in the GPT second partition. Pygrub currently only checks the first partition, so the attached patch tells it to check all the GPT partitions for grub configuration. Michael Young _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Oct-17 17:03 UTC
Re: [Xen-devel] [PATCH] Improve GPT support in pygrub
On Sun, Oct 16, 2011 at 09:45:00PM +0100, M A Young wrote:> This patch is part of a fix I applied to Fedora 16 to get pygrub to > boot a Fedora 16 guest (see > https://bugzilla.redhat.com/show_bug.cgi?id=745335 ). By default > Fedora 16 installs a Bios boot partition as the first GPT partition > to contain grub2 boot code, and the grub2 configuration files are in > the GPT second partition. Pygrub currently only checks the first > partition, so the attached patch tells it to check all the GPT > partitions for grub configuration. > > Michael Young> Check all GPT partitions for grub configuration, not just the first > Signed-off-by: Michael Young <m.a.young@durham.ac.uk>Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>> > --- a/tools/pygrub/src/pygrub 2011-10-16 20:58:02.000000000 +0100 > +++ b/tools/pygrub/src/pygrub 2011-10-16 20:59:52.000000000 +0100 > @@ -78,9 +78,17 @@ > def get_fs_offset_gpt(file): > fd = os.open(file, os.O_RDONLY) > # assume the first partition is an EFI system partition. > - os.lseek(fd, SECTOR_SIZE * 2, 0) > + os.lseek(fd, SECTOR_SIZE, 0) > buf = os.read(fd, 512) > - return struct.unpack("<Q", buf[32:40])[0] * SECTOR_SIZE > + partcount = struct.unpack("<L", buf[80:84])[0] > + partsize = struct.unpack("<L", buf[84:88])[0] > + i = partcount > + offsets = [] > + while i>0: > + buf = os.read(fd, partsize) > + offsets.append(struct.unpack("<Q", buf[32:40])[0] * SECTOR_SIZE) > + i -= 1 > + return offsets > > FDISK_PART_SOLARIS=0xbf > FDISK_PART_SOLARIS_OLD=0x82 > @@ -114,7 +122,9 @@ > continue # no solaris magic at that offset, ignore partition > > if type == FDISK_PART_GPT: > - offset = get_fs_offset_gpt(file) > + for offset in get_fs_offset_gpt(file): > + part_offs.append(offset) > + break > > # Active partition has 0x80 as the first byte. > # If active, prepend to front of list, otherwise append to back.> _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 10/17/2011 07:03 PM, Konrad Rzeszutek Wilk wrote:>> > Check all GPT partitions for grub configuration, not just the first >> > Signed-off-by: Michael Young<m.a.young@durham.ac.uk> > Tested-by: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com>You may need more to handle correctly this line: set default="${saved_entry}" but this can be done in a separate patch. Paolo _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, 19 Oct 2011, Paolo Bonzini wrote:> On 10/17/2011 07:03 PM, Konrad Rzeszutek Wilk wrote: >>> > Check all GPT partitions for grub configuration, not just the first >>> > Signed-off-by: Michael Young<m.a.young@durham.ac.uk> >> Tested-by: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com> > > You may need more to handle correctly this line: > > set default="${saved_entry}" > > but this can be done in a separate patch.Yes, there is this and 3 other issues that I was intending to submit as separate patches for. Briefly the other issues are * Fedora 16 uses /boot/grub2 not /boot/grub * the Fedora 16 grub2 configurations can have partitions references like (hd0,gpt2) * Fedora 16 can have submenus Michael Young _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, 2011-10-19 at 08:31 +0100, M A Young wrote:> On Wed, 19 Oct 2011, Paolo Bonzini wrote: > > > On 10/17/2011 07:03 PM, Konrad Rzeszutek Wilk wrote: > >>> > Check all GPT partitions for grub configuration, not just the first > >>> > Signed-off-by: Michael Young<m.a.young@durham.ac.uk> > >> Tested-by: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com> > > > > You may need more to handle correctly this line: > > > > set default="${saved_entry}" > > > > but this can be done in a separate patch. > > Yes, there is this and 3 other issues that I was intending to submit as > separate patches for. Briefly the other issues are > * Fedora 16 uses /boot/grub2 not /boot/grub > * the Fedora 16 grub2 configurations can have partitions references like > (hd0,gpt2) > * Fedora 16 can have submenusI think it would be useful to have an archive of the different syntaxes which we need to support. We should encourage folks who are fixing pygrub to work on a particular distro to also submit an example of a standardish configuration file as a patch to e.g. tools/pygrub/examples/<distro>.grub (or .grub2 if that''s appropriate). IOW please include an example f16 config in your series ;-) If someone is feeling ultra keen they could even write a little regression tester framework to iterate over the examples and ensure they produce something sane... Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel