Re: [Qemu-devel] [PATCH v3 1/6] qemu-option: has_help_option() and is_valid_option_list()

[Kevin Wolf <kwolf@redhat.com>]

Am 24.02.2014 um 09:42 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 21.02.2014 um 21:22 hat Eric Blake geschrieben:
> >> On 02/21/2014 08:24 AM, Kevin Wolf wrote:
> >> > has_help_option() checks if any help option ('help' or '?') occurs
> >> > anywhere in an option string, so that things like 'cluster_size=4k,help'
> >> > are recognised.
> >> > 
> >> > is_valid_option_list() ensures that the option list doesn't have options
> >> > with leading commas or trailing unescaped commas.
> 
> Care to explain why these are bad?  I dimly remember Eric and you
> discussing it last week, but I sure won't next time I look at this
> code...

Because with them you can trick qemu-img into producing escaped commas
when it tries to concatenate option strings ('-o backing_file=foo, -o
help' results in '-o backing_file=foo,,help').

> >> Rejects empty options, but also rejects values beginning with a comma.
> >> But we have legacy users that accept implicitly named first options (see
> >> opts_do_parse()).  For example, this is a valid command line (albeit one
> >> that prints a list of valid machines):
> >> 
> >> qemu-kvm -machine ,,blah
> >> 
> >> as shorthand for
> >> 
> >> qemu-kvm -machine type=,,blah
> >> 
> >> and where *buf would indeed be validly ','.
> >
> > Right, but I can't allow this without allowing '-o ,,' which breaks the
> > real use case. So the lesson is that you can concatenate option strings
> > and use implicit option names at the same time.
> 
> Another wart on the freakishly complex QemuOpts.

Rather what you get for operating on strings instead of directly going
to QemuOpts. We're parsing the option strings in the wrong place and
qemu-img shouldn't be concatenating option strings. But it takes a
larger cleanup until we can have that and I didn't want to block this
improvement on infrastructure work.

Kevin