Re: [Xen-devel] [PATCH v2] Add HybridISO support for PyGrub2

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-10-28 at 08:46 +0100, Philipp Hahn wrote:
> Hello Ian,
> 
> On Thursday 27 October 2011 12:21:05 Ian Campbell wrote:
> > On Tue, 2011-10-25 at 11:33 +0100, Philipp Hahn wrote:
> > [...snip explanations...]
> >
> > Thanks Philipp, that all seems to make sense.
> >
> > Tim had some suggestions on how/where this functionality could be better
> > implemented though.
> 
> v2: For HybrisISOs use offset 0 in addition instead of replacement.
> 
> Signed-off-by: Philipp Hahn <hahn@xxxxxxxxxxxxx>
> ---
> grub-mkrescue internally uses xorriso, which generates a so-called "Hybrid 
> ISO": The ISO images also contains a DOS partition table, which allows the 
> identical ISO file to be stored on an USB stick for booting from it. This 
> breaks PyGrub, since it (wrongly) detects only the DOS partition table and 
> uses the first partition instead of the complete ISO file.
> 
> Add a check to detect HybridISO files and use offset 0 in addition to 
> partition table parsing.
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -40,15 +40,20 @@ def enable_cursor(ison):
>      except _curses.error:
>          pass
>  
> -def is_disk_image(file):
> +DISK_TYPE_RAW, DISK_TYPE_HYBRIDISO, DISK_TYPE_DOS = range(3)
> +def identify_disk_image(file):
> +    """Detect DOS partition table or HybridISO format."""
>      fd = os.open(file, os.O_RDONLY)
> -    buf = os.read(fd, 512)
> +    buf = os.read(fd, 0x8006)

Can we avoid reading all that just for the 7 bytes we are interested in.
What about (just pseudo python, I didn't actually lookup seek etc):

        buf = os.read(fd, 512)
        if len(buf) >= 512 and \
             not struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
                return ...RAW

        os.seek(0x8000)
        buf = os.read(fd, 6)
        if len... and buf[0x8001...] == 'CD001':
                return ...HYBRID
        else:
                return ...DOS

on the other hand I suppose it's only 8k...

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

Thanks.
Ian
.
>      os.close(fd)
>  
>      if len(buf) >= 512 and \
>             struct.unpack("H", buf[0x1fe: 0x200]) == (0xaa55,):
> -        return True
> -    return False
> +        # HybridISO contains a DOS partition table for booting from USB 
> devices, but really is an ISO image
> +        if len(buf) >= 0x8006 and buf[0x8001:0x8006] == 'CD001':
> +            return DISK_TYPE_HYBRIDISO
> +        return DISK_TYPE_DOS
> +    return DISK_TYPE_RAW
>  
>  SECTOR_SIZE=512
>  DK_LABEL_LOC=1
> @@ -87,12 +92,19 @@ FDISK_PART_SOLARIS_OLD=0x82
>  FDISK_PART_GPT=0xee
>  
>  def get_partition_offsets(file):
> -    if not is_disk_image(file):
> +    image_type = identify_disk_image(file)
> +    if image_type == DISK_TYPE_RAW:
>          # No MBR: assume whole disk filesystem, which is like a 
>          # single partition starting at 0
>          return [0]
> -
> -    part_offs = []
> +    elif image_type == DISK_TYPE_HYBRIDISO:
> +        # A HybridISO contains an ISO filesystem at 0 in addition
> +        # to the DOS partition table
> +        part_offs = [0]
> +    elif image_type == DISK_TYPE_DOS:
> +        part_offs = []
> +    else:
> +        raise ValueError('Unhandled image type returnd by 
> identify_disk_image(): %d' % (image_type,))
>  
>      fd = os.open(file, os.O_RDONLY)
>      buf = os.read(fd, 512)
> 
> Sincerely
> Philipp



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