pygrub patch to allow explicit offset to fs

pygrub patch to allow explicit offset to fs

[Kjetil Torgrim Homme]

[-- 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

Re: pygrub patch to allow explicit offset to fs

[Ian Campbell]

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.

Re: pygrub patch to allow explicit offset to fs

[Wei Liu]

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

Re: pygrub patch to allow explicit offset to fs

[Ian Campbell]

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

Re: pygrub patch to allow explicit offset to fs

[George Dunlap]

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

Re: pygrub patch to allow explicit offset to fs

[Matt Wilson]

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

Re: pygrub patch to allow explicit offset to fs

[Kjetil Torgrim Homme]

[-- 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

Re: pygrub patch to allow explicit offset to fs

[Matt Wilson]

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:

Re: pygrub patch to allow explicit offset to fs

[Ian Campbell]

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>