* pygrub patch to allow explicit offset to fs @ 2013-06-18 23:40 Kjetil Torgrim Homme 2013-06-19 8:17 ` Ian Campbell 0 siblings, 1 reply; 9+ messages in thread From: Kjetil Torgrim Homme @ 2013-06-18 23:40 UTC (permalink / raw) To: xen-devel; +Cc: Ian.Campbell [-- Attachment #1: Type: text/plain, Size: 735 bytes --] 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 [-- Attachment #2: pygrub-offset.patch --] [-- Type: text/x-patch, Size: 2387 bytes --] 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 @@ -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 + user_provided_offset = 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 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) ] for offset in part_offs: try: [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-18 23:40 pygrub patch to allow explicit offset to fs Kjetil Torgrim Homme @ 2013-06-19 8:17 ` Ian Campbell 2013-06-19 8:39 ` Wei Liu 2013-06-19 10:06 ` George Dunlap 0 siblings, 2 replies; 9+ messages in thread From: Ian Campbell @ 2013-06-19 8:17 UTC (permalink / raw) To: Kjetil Torgrim Homme; +Cc: George Dunlap, 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 = 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) > 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-19 8:17 ` Ian Campbell @ 2013-06-19 8:39 ` Wei Liu 2013-06-19 8:44 ` Ian Campbell 2013-06-19 10:06 ` George Dunlap 1 sibling, 1 reply; 9+ messages in thread From: Wei Liu @ 2013-06-19 8:39 UTC (permalink / raw) To: Ian Campbell; +Cc: Kjetil Torgrim Homme, George Dunlap, xen-devel, wei.liu2 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-19 8:39 ` Wei Liu @ 2013-06-19 8:44 ` Ian Campbell 2013-06-19 18:10 ` Matt Wilson 0 siblings, 1 reply; 9+ messages in thread From: Ian Campbell @ 2013-06-19 8:44 UTC (permalink / raw) To: Wei Liu; +Cc: Kjetil Torgrim Homme, George Dunlap, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-19 8:44 ` Ian Campbell @ 2013-06-19 18:10 ` Matt Wilson 2013-06-20 11:51 ` Kjetil Torgrim Homme 0 siblings, 1 reply; 9+ messages in thread From: Matt Wilson @ 2013-06-19 18:10 UTC (permalink / raw) To: Ian Campbell; +Cc: Kjetil Torgrim Homme, George Dunlap, xen-devel, Wei Liu 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-19 18:10 ` Matt Wilson @ 2013-06-20 11:51 ` Kjetil Torgrim Homme 2013-06-24 15:17 ` Matt Wilson 0 siblings, 1 reply; 9+ messages in thread From: Kjetil Torgrim Homme @ 2013-06-20 11:51 UTC (permalink / raw) To: Matt Wilson; +Cc: George Dunlap, xen-devel, Wei Liu, Ian Campbell [-- Attachment #1: Type: text/plain, Size: 1065 bytes --] 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 [-- Attachment #2: pygrub-offset-2.patch --] [-- Type: text/x-patch, Size: 2828 bytes --] 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: [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-20 11:51 ` Kjetil Torgrim Homme @ 2013-06-24 15:17 ` Matt Wilson 2013-07-17 10:37 ` Ian Campbell 0 siblings, 1 reply; 9+ messages in thread From: Matt Wilson @ 2013-06-24 15:17 UTC (permalink / raw) To: Kjetil Torgrim Homme; +Cc: George Dunlap, xen-devel, Wei Liu, Ian Campbell 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: ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-24 15:17 ` Matt Wilson @ 2013-07-17 10:37 ` Ian Campbell 0 siblings, 0 replies; 9+ messages in thread From: Ian Campbell @ 2013-07-17 10:37 UTC (permalink / raw) To: Matt Wilson; +Cc: Kjetil Torgrim Homme, George Dunlap, xen-devel, Wei Liu 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> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pygrub patch to allow explicit offset to fs 2013-06-19 8:17 ` Ian Campbell 2013-06-19 8:39 ` Wei Liu @ 2013-06-19 10:06 ` George Dunlap 1 sibling, 0 replies; 9+ messages in thread From: George Dunlap @ 2013-06-19 10:06 UTC (permalink / raw) To: Ian Campbell; +Cc: Kjetil Torgrim Homme, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-17 10:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-18 23:40 pygrub patch to allow explicit offset to fs Kjetil Torgrim Homme 2013-06-19 8:17 ` Ian Campbell 2013-06-19 8:39 ` Wei Liu 2013-06-19 8:44 ` Ian Campbell 2013-06-19 18:10 ` Matt Wilson 2013-06-20 11:51 ` Kjetil Torgrim Homme 2013-06-24 15:17 ` Matt Wilson 2013-07-17 10:37 ` Ian Campbell 2013-06-19 10:06 ` George Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).