From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: pygrub patch to allow explicit offset to fs
Date: Wed, 19 Jun 2013 09:39:54 +0100
Message-ID: <20130619083954.GC2847@zion.uk.xensource.com>
References: <51C0EFE5.10109@redpill-linpro.com>
	<1371629850.22783.58.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1371629850.22783.58.camel@zakaz.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com>, George Dunlap <george.dunlap@eu.citrix.com>, xen-devel@lists.xensource.com, wei.liu2@citrix.com
List-Id: xen-devel@lists.xenproject.org

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