[PATCH] Improve GPT support in pygrub

[PATCH] Improve GPT support in pygrub

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 483 bytes --]

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

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1386 bytes --]

Check all GPT partitions for grub configuration, not just the first
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- 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.

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Improve GPT support in pygrub

[Konrad Rzeszutek Wilk]

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

Re: [PATCH] Improve GPT support in pygrub

[Paolo Bonzini]

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

Re: [PATCH] Improve GPT support in pygrub

[M A Young]

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

Re: [PATCH] Improve GPT support in pygrub

[Ian Campbell]

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 submenus

I 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.