[RFC PATCH] kexec-tools: Fix option/argument parsing

[Michael Neuling]

In message <4BEC8806.70608 at ozlabs.org> you wrote:
> 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 argume
nt
> >>> 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() loo
ks
> >>> 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 argumen
t
> >>> 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 allo
w
> >>> 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 poss
ib
> > le
> >>> options for an architecture build.  The intention is that this set includ
es
> >>> 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 throu
gh
> >>> 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 combine
d
> >>> 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.  :)

Yeah, that's what I'm concerned about.  Users are pretty dumb ;-)

Mikey