[RFC PATCH] kexec-tools: Fix option/argument parsing
matt at ozlabs.org
Thu May 13 19:15:18 EDT 2010
Michael Neuling wrote:
> In message <20100513144549.GB10534 at verge.net.au> you wrote:
>> On Thu, May 13, 2010 at 06:14:32PM +1000, Matt Evans wrote:
>>> In playing with kexec-tools I've noticed various problems with the argument
>>> passing, meaning one has to be careful to use certain forms of arguments
>>> and present them in a certain order.
>>> The arguments end up being parsed three times, each getting more specific
>>> than the last. main() looks for general args, arch_process_options() looks
>>> for options common to all loaders for the arch, and then finally many
>>> file_type load() methods check for options specific to that filetype.
>>> As GNU getopt works, it re-orders the argv pushing any args it doesn't
>>> recognise to the end. This includes arguments to valid options which
>>> are simply not recognised the first time around.
>>> For example, this does not work:
>>> # ./kexec -l --append "init=/bin/foo" /boot/vmlinux
>>> Cannot open `init=/bin/foo': No such file or directory
>>> but this does:
>>> # ./kexec -l --append="init=/bin/foo" /boot/vmlinux
>>> Using the --opt<space>arg variant results in the first non-option argument
>>> in main() being "init=/bin/foo" which is then used as the kernel filename,
>>> failing. Also, the order affects things in unintuitive ways:
>>> # ./kexec -l /boot/vmlinux --append "init=/bin/foo"
>>> <appears to load correctly, but command line appended with "/boot/vmlinux"!
>>> This behaviour is avoided by using the --opt= forms, but getopt does allow
>>> both and hence the user can have a fairly frustrating experience. (Doing
>>> something unexpected (ex. 3) is more annoying than an error exit (ex. 1)
>>> in many cases.)
>>> The fix presented here is to create a new #define to encapsulate all possib
>>> options for an architecture build. The intention is that this set includes
>>> all possible loaders' all possible options.
>>> main() walks through the options and silently ignores any non-general
>>> options (BUT respects that "--arg foo" should remain together, as
>>> getopt_long() does now recognise it). arch_process_options() walks through
>>> them again and responds to any arch-specific options, again ignoring but
>>> respecting any non-arch options. Finally the loader may look for its
>>> options, and find them in-order and present. Any outside of this combined
>>> set are complained-about as usual.
>>> So, comments please. Is this a reasonable way to do it or is there an
>>> obvious better way I've missed? :-) So far I have been able to test on
>>> x86(32,64) and ppc(32,64) but none of the others. :(
>> This seems reasonable to me.
>> I've compiled tested the code on all architectures except cris (I don't
>> have my build environment at the moment). I found a minor problem on arm
>> which I have noted below.
> I suspect it'll break someones kexec scripts, so maybe we take this
> patch (or something like it) but bump up the release revision to 2.1?
It /should/ not, as I haven't changed existing working behaviour -- unless someone with a script is relying on existing broken behaviour? I guess we've all been burned at least once by saying, "but SURELY nobody would be doing that", but hey. :)
More information about the kexec