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: >>> Hi, >>> >>> >>> 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 >>> <joy> >>> >>> 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 > le >>> 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. :) Cheers, Matt
