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>
--
Kjetil T. Homme
Redpill Linpro - Changing the game
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
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 = NoneIf 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)> 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: part_offs = get_partition_offsets(file)> > Ian.
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
On Wed, 2013-06-19 at 09:39 +0100, Wei Liu wrote:> 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.Yes, and the original had this problem too now you mention it.> > > 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? :-)I claim pseudo-code ;-)> > > Wei. > > > part_offs = get_partition_offsets(file) > > > > > > Ian. > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel
On 19/06/13 09:17, 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.Yeah, I think this will have to wait. It does also touch a codepath used by people not using this feature, and so there is ever so slight a chance that there will be breakage. -George
On Wed, Jun 19, 2013 at 09:44:46AM +0100, Ian Campbell wrote:> On Wed, 2013-06-19 at 09:39 +0100, Wei Liu wrote: > > 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: > > > > # 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? :-) > > I claim pseudo-code ;-)Comparisons with None should be done with "is" and "is not". http://www.python.org/dev/peps/pep-0008/ Comparisons to singletons like None should always be done with is or is not, never the equality operators. Matt
Kjetil Torgrim Homme
2013-Jun-20 11:51 UTC
Re: pygrub patch to allow explicit offset to fs
On 19/06/2013 20:10, Matt Wilson wrote:> On Wed, Jun 19, 2013 at 09:44:46AM +0100, Ian Campbell wrote: >> On Wed, 2013-06-19 at 09:39 +0100, Wei Liu wrote: >>> 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: >>>>> # 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:thanks for the feedback, everyone. here''s an updated patch. commit faec4e85318b6769df501d5198b55a607d97c255 Author: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> Date: Thu Jun 20 13:40:21 2013 +0200 updated patch for pygrub --offset after feedback from mailing list Signed-off-by: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> -- Kjetil T. Homme Redpill Linpro - Changing the game _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Thu, Jun 20, 2013 at 01:51:41PM +0200, Kjetil Torgrim Homme wrote:> On 19/06/2013 20:10, Matt Wilson wrote: > >On Wed, Jun 19, 2013 at 09:44:46AM +0100, Ian Campbell wrote: > >>On Wed, 2013-06-19 at 09:39 +0100, Wei Liu wrote: > >>>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: > >>>>> # 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: > > thanks for the feedback, everyone. here''s an updated patch. > > commit faec4e85318b6769df501d5198b55a607d97c255 > Author: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> > Date: Thu Jun 20 13:40:21 2013 +0200 > > updated patch for pygrub --offset after feedback from mailing list > > Signed-off-by: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com>Looks good, thanks! Reviewed-by: Matt Wilson <msw@amazon.com>> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub > index eedfdb2..363fbc7 100644 > --- a/tools/pygrub/src/pygrub > +++ b/tools/pygrub/src/pygrub > @@ -712,7 +712,7 @@ if __name__ == "__main__": > sel = None > > def usage(): > - print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] [-l|--list-entries] [-n|--not-really] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] [--output-directory=] [--output-format=sxp|simple|simple0] <image>" %(sys.argv[0],) > + print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] [-l|--list-entries] [-n|--not-really] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] [--output-directory=] [--output-format=sxp|simple|simple0] [--offset=] <image>" %(sys.argv[0],) > > def copy_from_image(fs, file_to_read, file_type, output_directory, > not_really): > @@ -748,7 +748,7 @@ if __name__ == "__main__": > try: > opts, args = getopt.gnu_getopt(sys.argv[1:], ''qilnh::'', > ["quiet", "interactive", "list-entries", "not-really", "help", > - "output=", "output-format=", "output-directory=", > + "output=", "output-format=", "output-directory=", "offset=", > "entry=", "kernel=", > "ramdisk=", "args=", "isconfig", "debug"]) > except getopt.GetoptError: > @@ -765,6 +765,7 @@ if __name__ == "__main__": > interactive = True > list_entries = False > isconfig = False > + part_offs = None > debug = False > not_really = False > output_format = "sxp" > @@ -797,6 +798,13 @@ if __name__ == "__main__": > incfg["ramdisk"] = a > elif o in ("--args",): > incfg["args"] = a > + elif o in ("--offset",): > + try: > + part_offs = [ int(a) ] > + except ValueError: > + print "offset value must be an integer" > + usage() > + sys.exit(1) > elif o in ("--entry",): > entry = a > # specifying the entry to boot implies non-interactive > @@ -807,7 +815,7 @@ if __name__ == "__main__": > debug = True > elif o in ("--output-format",): > if a not in ["sxp", "simple", "simple0"]: > - print "unkonwn output format %s" % a > + print "unknown output format %s" % a > usage() > sys.exit(1) > output_format = a > @@ -840,7 +848,8 @@ if __name__ == "__main__": > bootfsoptions = "" > > # get list of offsets into file which start partitions > - part_offs = get_partition_offsets(file) > + if part_offs is None: > + part_offs = get_partition_offsets(file) > > for offset in part_offs: > try:
On Mon, 2013-06-24 at 08:17 -0700, Matt Wilson wrote:> On Thu, Jun 20, 2013 at 01:51:41PM +0200, Kjetil Torgrim Homme wrote: > > On 19/06/2013 20:10, Matt Wilson wrote: > > >On Wed, Jun 19, 2013 at 09:44:46AM +0100, Ian Campbell wrote: > > >>On Wed, 2013-06-19 at 09:39 +0100, Wei Liu wrote: > > >>>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: > > >>>>> # 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: > > > > thanks for the feedback, everyone. here''s an updated patch. > > > > commit faec4e85318b6769df501d5198b55a607d97c255 > > Author: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> > > Date: Thu Jun 20 13:40:21 2013 +0200 > > > > updated patch for pygrub --offset after feedback from mailing list > > > > Signed-off-by: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> > > Looks good, thanks! > > Reviewed-by: Matt Wilson <msw@amazon.com>I reformatted the commit message and applied, thanks. pygrub: allow user to specify an explicit offset to fs This new option overrides partition table parsing Signed-off-by: Kjetil Torgrim Homme <kjetil.homme@redpill-linpro.com> Reviewed-by: Matt Wilson <msw@amazon.com>