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

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2012/1/3 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 =.

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel