Re: [Xen-devel] [PATCH] pygrub: fix extlinux parsing

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-01-03 at 11:56 +0000, Roger Pau Monnà wrote:
> 2012/1/3 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> > On Mon, 2012-01-02 at 10:49 +0000, Roger Pau Monne wrote:
> >> # HG changeset patch
> >> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> >> # 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@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel