Re: pygrub patch to allow explicit offset to fs

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jun 19, 2013 at 09:17:30AM +0100, Ian Campbell wrote:
> On Wed, 2013-06-19 at 01:40 +0200, Kjetil Torgrim Homme wrote:
> > I recently needed an old VM to work even though it was created on a SAN 
> > LUN with no partition table, just LVM straight onto the raw device.
> > 
> > pygrub didn't like this, so I added a simple hack to allow the user to 
> > override pygrub's probing when necessary.  please consider applying this 
> > patch.  btw, I think most LVM will have first filesystem at offset 196608.
> > 
> > 
> > commit 80a3f7b48da235695f8560deb41c19b23e7799e3
> > Author: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com>
> > Date:   Wed Jun 19 00:54:43 2013 +0200
> > 
> >      allow user to specify offset parameter which overrides partition 
> > table parsing
> > 
> >      Signed-off-by: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com>
> 
> Looks good to me, thanks.
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> although I've suggested a possible simplification below.
> 
> I think with the freeze being in force this, as a new feature, will have
> to wait for the 4.4 dev cycle to be applied, although I will defer to
> George's judgement.
> 
> > diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> > index eedfdb2..d46ee8c 100644
> > --- a/tools/pygrub/src/pygrub
> > +++ b/tools/pygrub/src/pygrub
> > [...]
> > @@ -765,6 +765,7 @@ if __name__ == "__main__":
> >      interactive = True
> >      list_entries = False
> >      isconfig = False
> > +    user_provided_offset = None
> 
> If you make this "part_offs = None"...
> 
> >      debug = False
> >      not_really = False
> >      output_format = "sxp"
> > @@ -797,6 +798,8 @@ if __name__ == "__main__":
> >              incfg["ramdisk"] = a
> >          elif o in ("--args",):
> >              incfg["args"] = a
> > +        elif o in ("--offset",):
> > +            user_provided_offset = a
> 
> ... and this "part_offs = int(a)" ... (or [int(a)] if that's correct)

Need to take care of exception in this conversion.

> 
> >          elif o in ("--entry",):
> >              entry = a
> >              # specifying the entry to boot implies non-interactive
> > @@ -840,7 +843,10 @@ if __name__ == "__main__":
> >          bootfsoptions = ""
> >  
> >      # get list of offsets into file which start partitions
> > -    part_offs = get_partition_offsets(file)
> > +    if user_provided_offset is None:
> > +        part_offs = get_partition_offsets(file)
> > +    else:
> > +        part_offs = [ int(user_provided_offset) ]
> 
> Then this can become just:
>      if part_offs = None:

You do know you missed a "=" here, right? :-)


Wei.

>          part_offs = get_partition_offsets(file)
> > 
> > Ian.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel