# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1325501151 -3600
# Node ID bd59a07ed41187bcafc4dd5214f0744c66ef2069
# Parent 3e02aa9670b3265e36bdddbd4760415cd87d047b
pygrub: fix extlinux parsing
pygrub was unable to parse extlinux config files correctly, exactly
the ones like:
LABEL grsec
KERNEL vmlinuz-3.0.10-grsec
APPEND initrd=initramfs-3.0.10-grsec
root=UUID=cfd4a7b4-8c40-4025-b877-8205f1c622ee
modules=sd-mod,usb-storage,ext4 xen quiet
This patch fixes it, adding a new case when parsing the "append" line,
that searches for the initrd image.
Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
diff -r 3e02aa9670b3 -r bd59a07ed411 tools/pygrub/src/ExtLinuxConf.py
--- a/tools/pygrub/src/ExtLinuxConf.py Thu Dec 15 18:55:46 2011 +0100
+++ b/tools/pygrub/src/ExtLinuxConf.py Mon Jan 02 11:45:51 2012 +0100
@@ -60,6 +60,13 @@ class ExtLinuxImage(object):
# Bypass regular self.commands handling
com = None
+ elif arg.find(" "):
+ # find initrd image in append line
+ args = arg.strip().split(" ")
+ for a in args:
+ if a.lower().startswith("initrd"):
+ setattr(self, "initrd",
a.replace("initrd=", ""))
+ arg = arg.replace(a, "")
if com is not None and self.commands.has_key(com):
if self.commands[com] is not None:
@@ -86,10 +93,12 @@ class ExtLinuxImage(object):
self._args = args
def get_kernel(self):
return self._kernel
+ def set_args(self, val):
+ self._args = val
def get_args(self):
return self._args
kernel = property(get_kernel, set_kernel)
- args = property(get_args)
+ args = property(get_args, set_args)
def set_initrd(self, val):
self._initrd = (None,val)
On Mon, 2012-01-02 at 10:49 +0000, Roger Pau Monne wrote:> # HG changeset patch > # User Roger Pau Monne <roger.pau@entel.upc.edu> > # Date 1325501151 -3600 > # Node ID bd59a07ed41187bcafc4dd5214f0744c66ef2069 > # Parent 3e02aa9670b3265e36bdddbd4760415cd87d047b > pygrub: fix extlinux parsing > > pygrub was unable to parse extlinux config files correctly, exactly > the ones like: > > LABEL grsec > KERNEL vmlinuz-3.0.10-grsec > APPEND initrd=initramfs-3.0.10-grsec > root=UUID=cfd4a7b4-8c40-4025-b877-8205f1c622ee > modules=sd-mod,usb-storage,ext4 xen quiet > > This patch fixes it, adding a new case when parsing the "append" line, > that searches for the initrd image. > > Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>Please could you also supply an example file for your platform as a patch to tools/pygrub/examples.> diff -r 3e02aa9670b3 -r bd59a07ed411 tools/pygrub/src/ExtLinuxConf.py > --- a/tools/pygrub/src/ExtLinuxConf.py Thu Dec 15 18:55:46 2011 +0100 > +++ b/tools/pygrub/src/ExtLinuxConf.py Mon Jan 02 11:45:51 2012 +0100 > @@ -60,6 +60,13 @@ class ExtLinuxImage(object): > > # Bypass regular self.commands handling > com = None > + elif arg.find(" "): > + # find initrd image in append line > + args = arg.strip().split(" ") > + for a in args: > + if a.lower().startswith("initrd"):Should check for "initrd=" here? Or perhaps: * check for "=" * split into "k = v" * check that k is precisely "initrd" the first two are probably doable in the same str.split invocation if you handle the exception correctly.> + setattr(self, "initrd", a.replace("initrd=", "")) > + arg = arg.replace(a, "") > > if com is not None and self.commands.has_key(com): > if self.commands[com] is not None: > @@ -86,10 +93,12 @@ class ExtLinuxImage(object): > self._args = args > def get_kernel(self): > return self._kernel > + def set_args(self, val): > + self._args = val > def get_args(self): > return self._args > kernel = property(get_kernel, set_kernel) > - args = property(get_args) > + args = property(get_args, set_args)Are these something required by arg.replace? Ian.
2012/1/3 Ian Campbell <Ian.Campbell@citrix.com>:> On Mon, 2012-01-02 at 10:49 +0000, Roger Pau Monne wrote: >> # HG changeset patch >> # User Roger Pau Monne <roger.pau@entel.upc.edu> >> # Date 1325501151 -3600 >> # Node ID bd59a07ed41187bcafc4dd5214f0744c66ef2069 >> # Parent 3e02aa9670b3265e36bdddbd4760415cd87d047b >> pygrub: fix extlinux parsing >> >> pygrub was unable to parse extlinux config files correctly, exactly >> the ones like: >> >> LABEL grsec >> KERNEL vmlinuz-3.0.10-grsec >> APPEND initrd=initramfs-3.0.10-grsec >> root=UUID=cfd4a7b4-8c40-4025-b877-8205f1c622ee >> modules=sd-mod,usb-storage,ext4 xen quiet >> >> This patch fixes it, adding a new case when parsing the "append" line, >> that searches for the initrd image. >> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > Please could you also supply an example file for your platform as a > patch to tools/pygrub/examples.Done.>> diff -r 3e02aa9670b3 -r bd59a07ed411 tools/pygrub/src/ExtLinuxConf.py >> --- a/tools/pygrub/src/ExtLinuxConf.py Thu Dec 15 18:55:46 2011 +0100 >> +++ b/tools/pygrub/src/ExtLinuxConf.py Mon Jan 02 11:45:51 2012 +0100 >> @@ -60,6 +60,13 @@ class ExtLinuxImage(object): >> >> # Bypass regular self.commands handling >> com = None >> + elif arg.find(" "): >> + # find initrd image in append line >> + args = arg.strip().split(" ") >> + for a in args: >> + if a.lower().startswith("initrd"): > > Should check for "initrd=" here?Yes> Or perhaps: > * check for "=" > * split into "k = v" > * check that k is precisely "initrd" > the first two are probably doable in the same str.split invocation if > you handle the exception correctly.I'm not really sure about splitting '=', some args, like root, are root=UUID=XXXX, which might be difficult to parse in a single split (at least for me). Will look at it later and post an updated patch.>> + setattr(self, "initrd", a.replace("initrd=", "")) >> + arg = arg.replace(a, "") >> >> if com is not None and self.commands.has_key(com): >> if self.commands[com] is not None: >> @@ -86,10 +93,12 @@ class ExtLinuxImage(object): >> self._args = args >> def get_kernel(self): >> return self._kernel >> + def set_args(self, val): >> + self._args = val >> def get_args(self): >> return self._args >> kernel = property(get_kernel, set_kernel) >> - args = property(get_args) >> + args = property(get_args, set_args) > > Are these something required by arg.replace?args was set when setting kernel in previous version, because it assumed the line was: append <kernel> <args> --- <initrd> So args where obtained when the kernel was parsed, but you can also have kernel <kernel> append <args> And <args> contain the initrd path, I remove the "initrd=XXX" from the args and process them normally. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, 2012-01-03 at 11:56 +0000, Roger Pau Monné wrote:> 2012/1/3 Ian Campbell <Ian.Campbell@citrix.com>: > > On Mon, 2012-01-02 at 10:49 +0000, Roger Pau Monne wrote: > >> # HG changeset patch > >> # User Roger Pau Monne <roger.pau@entel.upc.edu> > >> # Date 1325501151 -3600 > >> # Node ID bd59a07ed41187bcafc4dd5214f0744c66ef2069 > >> # Parent 3e02aa9670b3265e36bdddbd4760415cd87d047b > >> pygrub: fix extlinux parsing > >> > >> pygrub was unable to parse extlinux config files correctly, exactly > >> the ones like: > >> > >> LABEL grsec > >> KERNEL vmlinuz-3.0.10-grsec > >> APPEND initrd=initramfs-3.0.10-grsec > >> root=UUID=cfd4a7b4-8c40-4025-b877-8205f1c622ee > >> modules=sd-mod,usb-storage,ext4 xen quiet > >> > >> This patch fixes it, adding a new case when parsing the "append" line, > >> that searches for the initrd image. > >> > >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > > > Please could you also supply an example file for your platform as a > > patch to tools/pygrub/examples. > > Done. > > >> diff -r 3e02aa9670b3 -r bd59a07ed411 tools/pygrub/src/ExtLinuxConf.py > >> --- a/tools/pygrub/src/ExtLinuxConf.py Thu Dec 15 18:55:46 2011 +0100 > >> +++ b/tools/pygrub/src/ExtLinuxConf.py Mon Jan 02 11:45:51 2012 +0100 > >> @@ -60,6 +60,13 @@ class ExtLinuxImage(object): > >> > >> # Bypass regular self.commands handling > >> com = None > >> + elif arg.find(" "): > >> + # find initrd image in append line > >> + args = arg.strip().split(" ") > >> + for a in args: > >> + if a.lower().startswith("initrd"): > > > > Should check for "initrd=" here? > > Yes > > > Or perhaps: > > * check for "=" > > * split into "k = v" > > * check that k is precisely "initrd" > > the first two are probably doable in the same str.split invocation if > > you handle the exception correctly. > > I'm not really sure about splitting '=', some args, like root, are > root=UUID=XXXX, which might be difficult to parse in a single split > (at least for me). Will look at it later and post an updated patch.I think you need to always split on the first =.> > >> + setattr(self, "initrd", a.replace("initrd=", "")) > >> + arg = arg.replace(a, "") > >> > >> if com is not None and self.commands.has_key(com): > >> if self.commands[com] is not None: > >> @@ -86,10 +93,12 @@ class ExtLinuxImage(object): > >> self._args = args > >> def get_kernel(self): > >> return self._kernel > >> + def set_args(self, val): > >> + self._args = val > >> def get_args(self): > >> return self._args > >> kernel = property(get_kernel, set_kernel) > >> - args = property(get_args) > >> + args = property(get_args, set_args) > > > > Are these something required by arg.replace? > > args was set when setting kernel in previous version, because it > assumed the line was: > > append <kernel> <args> --- <initrd>Ah, right. This is the mboot.c32 syntax (i.e. it is always associated with "kernel mboot.c32"). IIRC it is actually append <hypervisor> <args> --- <kernel> <args> --- <initrd> Or more generally "[<thing> <args> ---]+" Perhaps this ought to key off the presence of kernel mboot.c32 specifically?> So args where obtained when the kernel was parsed, but you can also have > > kernel <kernel> > append <args>This is actually the "normal" extlinux syntax.> And <args> contain the initrd path, I remove the "initrd=XXX" from the > args and process them normally.Right. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
2012/1/3 Ian Campbell <Ian.Campbell@citrix.com>:> On Tue, 2012-01-03 at 11:56 +0000, Roger Pau Monné wrote: >> 2012/1/3 Ian Campbell <Ian.Campbell@citrix.com>: >> > On Mon, 2012-01-02 at 10:49 +0000, Roger Pau Monne wrote: >> >> # HG changeset patch >> >> # User Roger Pau Monne <roger.pau@entel.upc.edu> >> >> # Date 1325501151 -3600 >> >> # Node ID bd59a07ed41187bcafc4dd5214f0744c66ef2069 >> >> # Parent 3e02aa9670b3265e36bdddbd4760415cd87d047b >> >> pygrub: fix extlinux parsing >> >> >> >> pygrub was unable to parse extlinux config files correctly, exactly >> >> the ones like: >> >> >> >> LABEL grsec >> >> KERNEL vmlinuz-3.0.10-grsec >> >> APPEND initrd=initramfs-3.0.10-grsec >> >> root=UUID=cfd4a7b4-8c40-4025-b877-8205f1c622ee >> >> modules=sd-mod,usb-storage,ext4 xen quiet >> >> >> >> This patch fixes it, adding a new case when parsing the "append" line, >> >> that searches for the initrd image. >> >> >> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> >> > >> > Please could you also supply an example file for your platform as a >> > patch to tools/pygrub/examples. >> >> Done. >> >> >> diff -r 3e02aa9670b3 -r bd59a07ed411 tools/pygrub/src/ExtLinuxConf.py >> >> --- a/tools/pygrub/src/ExtLinuxConf.py Thu Dec 15 18:55:46 2011 +0100 >> >> +++ b/tools/pygrub/src/ExtLinuxConf.py Mon Jan 02 11:45:51 2012 +0100 >> >> @@ -60,6 +60,13 @@ class ExtLinuxImage(object): >> >> >> >> # Bypass regular self.commands handling >> >> com = None >> >> + elif arg.find(" "): >> >> + # find initrd image in append line >> >> + args = arg.strip().split(" ") >> >> + for a in args: >> >> + if a.lower().startswith("initrd"): >> > >> > Should check for "initrd=" here? >> >> Yes >> >> > Or perhaps: >> > * check for "=" >> > * split into "k = v" >> > * check that k is precisely "initrd" >> > the first two are probably doable in the same str.split invocation if >> > you handle the exception correctly. >> >> I'm not really sure about splitting '=', some args, like root, are >> root=UUID=XXXX, which might be difficult to parse in a single split >> (at least for me). Will look at it later and post an updated patch. > > I think you need to always split on the first =.I was not really sure if args could be like "root=XXX initrd=XXX" or if initrd should be the first one. Anyway, I've sent an updated version that I think is safer.>> >> >> + setattr(self, "initrd", a.replace("initrd=", "")) >> >> + arg = arg.replace(a, "") >> >> >> >> if com is not None and self.commands.has_key(com): >> >> if self.commands[com] is not None: >> >> @@ -86,10 +93,12 @@ class ExtLinuxImage(object): >> >> self._args = args >> >> def get_kernel(self): >> >> return self._kernel >> >> + def set_args(self, val): >> >> + self._args = val >> >> def get_args(self): >> >> return self._args >> >> kernel = property(get_kernel, set_kernel) >> >> - args = property(get_args) >> >> + args = property(get_args, set_args) >> > >> > Are these something required by arg.replace? >> >> args was set when setting kernel in previous version, because it >> assumed the line was: >> >> append <kernel> <args> --- <initrd> > > Ah, right. This is the mboot.c32 syntax (i.e. it is always associated > with "kernel mboot.c32"). IIRC it is actually > append <hypervisor> <args> --- <kernel> <args> --- <initrd> > Or more generally "[<thing> <args> ---]+" > > Perhaps this ought to key off the presence of kernel mboot.c32 > specifically?It is handled in a previous "if".>> So args where obtained when the kernel was parsed, but you can also have >> >> kernel <kernel> >> append <args> > > This is actually the "normal" extlinux syntax. > >> And <args> contain the initrd path, I remove the "initrd=XXX" from the >> args and process them normally. > > Right. > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel