[Qemu-devel] help parsing qemu options

[Qemu-devel] help parsing qemu options

[Gabriel L. Somlo]

Hi,

Assuming I'm trying to add a new command line option to qemu in vl.c:

...

static QemuOptsList qemu_foo_opts = {
    .name = "foo",
    .implied_opt_name = "name",
    .head = QTAILQ_HEAD_INITIALIZER(qemu_foo_opts.head),
    .desc = {
        {
            .name = "name",
            .type = QEMU_OPT_STRING,
        }, {
            .name = "file",
            .type = QEMU_OPT_STRING,
        },
        { /* end of list */ }
    },
};

...

main() {

    ...
    qemu_add_opts(&qemu_foo_opts);

    ...

    /* second pass of option parsing */
    ...
    for(;;) {
        ...
        switch(popt->index) {
            ...

        case QEMU_OPTION_foo:
            opts = qemu_opts_parse(qemu_find_opts("foo"), optarg, 0);
            if (!foo_option_add(qemu_opt_get(opts, "name"),
                                qemu_opt_get(opts, "file"))) {
                fprintf(stderr, "invalid foo entry config %s\n", optarg);
                exit(1);
            }
            break;

    ...
}


Somewhere else in the source I have

bool foo_option_add(const char *name, const char *file) {

    if (/* everything OK */) {
        return true;
    }

    return false;
}    


Assuming the above is correct (and that the appropriate glue is added
to qemu-options.hx to tie "-foo name=abc,file=xyz" to QEMU_OPTION_foo),
I'm wondering about preventing "name" and "file" from being turned
into Booleans should their arguments be omitted on the command line.

To clarify:

-foo abcxyz   results in a parse error generated by qemu_opts_parse()
              which is as it should be

-foo file=abc,name=xyz   results in a call to foo_option_add("xyz", "abc")
                         which is the desired behavior

-foo file=,name=   results in a call to foo_option_add("", "")
                   which is also OK, as I can sanity-check my
                   arguments from within foo_option_add()

However,

-foo file,name    results in a call to foo_option_add("on", "on")
                  i.e., in the absence of string values, both
                  "file" and "name" are converted into Booleans
                  and given the string value "on" by qemu_opts_parse()
                  which is NOT what I want, and I'm wondering if
                  that behavior can somehow be turned off for
                  any given QemuOptsList ?

I guess I could be looking for a file named "on" in the current
directory, and attempt to use the value "on" to insert the object
from the given file (and fail if no file named "on" could be found,
but this is not as clean as I would like it to be, and I'm wondering
if there's a better way).

Thanks much,
--Gabriel

Re: [Qemu-devel] help parsing qemu options

[Markus Armbruster]

"Gabriel L. Somlo" <gsomlo@gmail.com> writes:

> Hi,
>
> Assuming I'm trying to add a new command line option to qemu in vl.c:
>
> ...
>
> static QemuOptsList qemu_foo_opts = {
>     .name = "foo",
>     .implied_opt_name = "name",
>     .head = QTAILQ_HEAD_INITIALIZER(qemu_foo_opts.head),
>     .desc = {
>         {
>             .name = "name",
>             .type = QEMU_OPT_STRING,
>         }, {
>             .name = "file",
>             .type = QEMU_OPT_STRING,
>         },
>         { /* end of list */ }
>     },
> };
>
> ...
>
> main() {
>
>     ...
>     qemu_add_opts(&qemu_foo_opts);
>
>     ...
>
>     /* second pass of option parsing */
>     ...
>     for(;;) {
>         ...
>         switch(popt->index) {
>             ...
>
>         case QEMU_OPTION_foo:
>             opts = qemu_opts_parse(qemu_find_opts("foo"), optarg, 0);
>             if (!foo_option_add(qemu_opt_get(opts, "name"),
>                                 qemu_opt_get(opts, "file"))) {
>                 fprintf(stderr, "invalid foo entry config %s\n", optarg);
>                 exit(1);
>             }
>             break;
>
>     ...
> }
>
>
> Somewhere else in the source I have
>
> bool foo_option_add(const char *name, const char *file) {
>
>     if (/* everything OK */) {
>         return true;
>     }
>
>     return false;
> }    

Looks sane so far.

> Assuming the above is correct (and that the appropriate glue is added
> to qemu-options.hx to tie "-foo name=abc,file=xyz" to QEMU_OPTION_foo),
> I'm wondering about preventing "name" and "file" from being turned
> into Booleans should their arguments be omitted on the command line.
>
> To clarify:
>
> -foo abcxyz   results in a parse error generated by qemu_opts_parse()
>               which is as it should be
>
> -foo file=abc,name=xyz   results in a call to foo_option_add("xyz", "abc")
>                          which is the desired behavior
>
> -foo file=,name=   results in a call to foo_option_add("", "")
>                    which is also OK, as I can sanity-check my
>                    arguments from within foo_option_add()
>
> However,
>
> -foo file,name    results in a call to foo_option_add("on", "on")
>                   i.e., in the absence of string values, both
>                   "file" and "name" are converted into Booleans
>                   and given the string value "on" by qemu_opts_parse()
>                   which is NOT what I want, and I'm wondering if
>                   that behavior can somehow be turned off for
>                   any given QemuOptsList ?
>
> I guess I could be looking for a file named "on" in the current
> directory, and attempt to use the value "on" to insert the object
> from the given file (and fail if no file named "on" could be found,
> but this is not as clean as I would like it to be, and I'm wondering
> if there's a better way).

Reproduced:

    $ upstream-qemu -nodefaults -S -display vnc=:0 -monitor stdio -name process=foo,guest
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) info name
    on

QemuOpts is baroque.

No, you can't switch it off.  I'm afraid adding such a switch is not a
good idea, because it would make QemuOpts even more baroque.

Perhaps we can limit the convenience syntax "omitted value means =on" to
boolean options.  Could be hairy, because .desc can be empty, which
defers part of the checking until later.

Re: [Qemu-devel] help parsing qemu options

[Gabriel L. Somlo]

On Tue, Mar 10, 2015 at 09:40:09AM +0100, Markus Armbruster wrote:
> "Gabriel L. Somlo" <gsomlo@gmail.com> writes:
> > Assuming the above is correct (and that the appropriate glue is added
> > to qemu-options.hx to tie "-foo name=abc,file=xyz" to QEMU_OPTION_foo),
> > I'm wondering about preventing "name" and "file" from being turned
> > into Booleans should their arguments be omitted on the command line.
> >
> > To clarify:
> >
> > -foo abcxyz   results in a parse error generated by qemu_opts_parse()
> >               which is as it should be
> >
> > -foo file=abc,name=xyz   results in a call to foo_option_add("xyz", "abc")
> >                          which is the desired behavior
> >
> > -foo file=,name=   results in a call to foo_option_add("", "")
> >                    which is also OK, as I can sanity-check my
> >                    arguments from within foo_option_add()
> >
> > However,
> >
> > -foo file,name    results in a call to foo_option_add("on", "on")
> >                   i.e., in the absence of string values, both
> >                   "file" and "name" are converted into Booleans
> >                   and given the string value "on" by qemu_opts_parse()
> >                   which is NOT what I want, and I'm wondering if
> >                   that behavior can somehow be turned off for
> >                   any given QemuOptsList ?
> >
> > I guess I could be looking for a file named "on" in the current
> > directory, and attempt to use the value "on" to insert the object
> > from the given file (and fail if no file named "on" could be found,
> > but this is not as clean as I would like it to be, and I'm wondering
> > if there's a better way).
> 
> Reproduced:
> 
>     $ upstream-qemu -nodefaults -S -display vnc=:0 -monitor stdio -name process=foo,guest
>     QEMU 2.2.50 monitor - type 'help' for more information
>     (qemu) info name
>     on
> 
> QemuOpts is baroque.
> 
> No, you can't switch it off.  I'm afraid adding such a switch is not a
> good idea, because it would make QemuOpts even more baroque.
> 
> Perhaps we can limit the convenience syntax "omitted value means =on" to
> boolean options.  Could be hairy, because .desc can be empty, which
> defers part of the checking until later.

Maybe adding a .default field to .desc, so if the parameter is defined
but no value is assigned, the .default value (which could be NULL) is
used instead of always defaulting to "on" ?

No idea if that'd be worth it though, I was just wondering if there's
already a mechanism controlling whether fallback-to-bool is a (per
QemuOptsList struct) tunable option, which I now know it is not :)

Thanks much,
--Gabriel

Re: [Qemu-devel] help parsing qemu options

[Markus Armbruster]

"Gabriel L. Somlo" <gsomlo@gmail.com> writes:

> On Tue, Mar 10, 2015 at 09:40:09AM +0100, Markus Armbruster wrote:
>> "Gabriel L. Somlo" <gsomlo@gmail.com> writes:
>> > Assuming the above is correct (and that the appropriate glue is added
>> > to qemu-options.hx to tie "-foo name=abc,file=xyz" to QEMU_OPTION_foo),
>> > I'm wondering about preventing "name" and "file" from being turned
>> > into Booleans should their arguments be omitted on the command line.
>> >
>> > To clarify:
>> >
>> > -foo abcxyz   results in a parse error generated by qemu_opts_parse()
>> >               which is as it should be
>> >
>> > -foo file=abc,name=xyz   results in a call to foo_option_add("xyz", "abc")
>> >                          which is the desired behavior
>> >
>> > -foo file=,name=   results in a call to foo_option_add("", "")
>> >                    which is also OK, as I can sanity-check my
>> >                    arguments from within foo_option_add()
>> >
>> > However,
>> >
>> > -foo file,name    results in a call to foo_option_add("on", "on")
>> >                   i.e., in the absence of string values, both
>> >                   "file" and "name" are converted into Booleans
>> >                   and given the string value "on" by qemu_opts_parse()
>> >                   which is NOT what I want, and I'm wondering if
>> >                   that behavior can somehow be turned off for
>> >                   any given QemuOptsList ?
>> >
>> > I guess I could be looking for a file named "on" in the current
>> > directory, and attempt to use the value "on" to insert the object
>> > from the given file (and fail if no file named "on" could be found,
>> > but this is not as clean as I would like it to be, and I'm wondering
>> > if there's a better way).
>> 
>> Reproduced:
>> 
>>     $ upstream-qemu -nodefaults -S -display vnc=:0 -monitor stdio
>> -name process=foo,guest
>>     QEMU 2.2.50 monitor - type 'help' for more information
>>     (qemu) info name
>>     on
>> 
>> QemuOpts is baroque.
>> 
>> No, you can't switch it off.  I'm afraid adding such a switch is not a
>> good idea, because it would make QemuOpts even more baroque.
>> 
>> Perhaps we can limit the convenience syntax "omitted value means =on" to
>> boolean options.  Could be hairy, because .desc can be empty, which
>> defers part of the checking until later.
>
> Maybe adding a .default field to .desc, so if the parameter is defined
> but no value is assigned, the .default value (which could be NULL) is
> used instead of always defaulting to "on" ?

Introduces a second way to do defaults.  The existing way is to provide
them separately for each use, like this:

   int port = qdict_get_try_int(qdict, "port", -1);

or this:

    const char *device = qdict_get_try_str(qdict, "device");
    if (!device)
        device = "tcp::" DEFAULT_GDBSTUB_PORT;

Adding another way begs the question what to do when *both* ways provide
a default, and they differ.

Getting the semantics consistent could be hairy, because .desc can be
empty, which defers part of the checking until later.

Prior mention of the idea:
https://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg03813.html
which elaborates on
https://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg04598.html

> No idea if that'd be worth it though, I was just wondering if there's
> already a mechanism controlling whether fallback-to-bool is a (per
> QemuOptsList struct) tunable option, which I now know it is not :)
>
> Thanks much,

You're welcome :)

Re: [Qemu-devel] help parsing qemu options

[Kevin Wolf]

Am 11.03.2015 um 08:52 hat Markus Armbruster geschrieben:
> "Gabriel L. Somlo" <gsomlo@gmail.com> writes:
> 
> > On Tue, Mar 10, 2015 at 09:40:09AM +0100, Markus Armbruster wrote:
> >> "Gabriel L. Somlo" <gsomlo@gmail.com> writes:
> >> > Assuming the above is correct (and that the appropriate glue is added
> >> > to qemu-options.hx to tie "-foo name=abc,file=xyz" to QEMU_OPTION_foo),
> >> > I'm wondering about preventing "name" and "file" from being turned
> >> > into Booleans should their arguments be omitted on the command line.
> >> >
> >> > To clarify:
> >> >
> >> > -foo abcxyz   results in a parse error generated by qemu_opts_parse()
> >> >               which is as it should be
> >> >
> >> > -foo file=abc,name=xyz   results in a call to foo_option_add("xyz", "abc")
> >> >                          which is the desired behavior
> >> >
> >> > -foo file=,name=   results in a call to foo_option_add("", "")
> >> >                    which is also OK, as I can sanity-check my
> >> >                    arguments from within foo_option_add()
> >> >
> >> > However,
> >> >
> >> > -foo file,name    results in a call to foo_option_add("on", "on")
> >> >                   i.e., in the absence of string values, both
> >> >                   "file" and "name" are converted into Booleans
> >> >                   and given the string value "on" by qemu_opts_parse()
> >> >                   which is NOT what I want, and I'm wondering if
> >> >                   that behavior can somehow be turned off for
> >> >                   any given QemuOptsList ?
> >> >
> >> > I guess I could be looking for a file named "on" in the current
> >> > directory, and attempt to use the value "on" to insert the object
> >> > from the given file (and fail if no file named "on" could be found,
> >> > but this is not as clean as I would like it to be, and I'm wondering
> >> > if there's a better way).
> >> 
> >> Reproduced:
> >> 
> >>     $ upstream-qemu -nodefaults -S -display vnc=:0 -monitor stdio
> >> -name process=foo,guest
> >>     QEMU 2.2.50 monitor - type 'help' for more information
> >>     (qemu) info name
> >>     on
> >> 
> >> QemuOpts is baroque.
> >> 
> >> No, you can't switch it off.  I'm afraid adding such a switch is not a
> >> good idea, because it would make QemuOpts even more baroque.
> >> 
> >> Perhaps we can limit the convenience syntax "omitted value means =on" to
> >> boolean options.  Could be hairy, because .desc can be empty, which
> >> defers part of the checking until later.
> >
> > Maybe adding a .default field to .desc, so if the parameter is defined
> > but no value is assigned, the .default value (which could be NULL) is
> > used instead of always defaulting to "on" ?
> 
> Introduces a second way to do defaults.  The existing way is to provide
> them separately for each use, like this:
> 
>    int port = qdict_get_try_int(qdict, "port", -1);
> 
> or this:
> 
>     const char *device = qdict_get_try_str(qdict, "device");
>     if (!device)
>         device = "tcp::" DEFAULT_GDBSTUB_PORT;
> 
> Adding another way begs the question what to do when *both* ways provide
> a default, and they differ.
> 
> Getting the semantics consistent could be hairy, because .desc can be
> empty, which defers part of the checking until later.
> 
> Prior mention of the idea:
> https://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg03813.html
> which elaborates on
> https://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg04598.html

def_value_str exists in the current codebase, and it seems to take
precedence when a different default is specified by the caller.

Kevin

Re: [Qemu-devel] help parsing qemu options

[Gabriel L. Somlo]

On Wed, Mar 11, 2015 at 12:59:40PM +0100, Kevin Wolf wrote:
> 
> def_value_str exists in the current codebase, and it seems to take
> precedence when a different default is specified by the caller.

So I've now added .def_value_str fields:

static QemuOptsList qemu_foo_opts = {
    .name = "foo",
    .head = QTAILQ_HEAD_INITIALIZER(qemu_foo_opts.head),
    .desc = {
        {
            .name = "name",
            .type = QEMU_OPT_STRING,
            .def_value_str = "abc",
        }, {
            .name = "file",
            .type = QEMU_OPT_STRING,
            .def_value_str = "xyz",
        },
        { /* end of list */ }
    },
};

but "-foo name,file" still results in qemu_opt_get(opts, "name")
returning "on", rather than "abc". Is that a bug, or am I missing
something, or doing something wrong ?

Thanks,
--Gabriel

Re: [Qemu-devel] help parsing qemu options

[Gabriel L. Somlo]

On Wed, Mar 11, 2015 at 08:40:46AM -0400, Gabriel L. Somlo wrote:
> On Wed, Mar 11, 2015 at 12:59:40PM +0100, Kevin Wolf wrote:
> > 
> > def_value_str exists in the current codebase, and it seems to take
> > precedence when a different default is specified by the caller.
> 
> So I've now added .def_value_str fields:
> 
> static QemuOptsList qemu_foo_opts = {
>     .name = "foo",
>     .head = QTAILQ_HEAD_INITIALIZER(qemu_foo_opts.head),
>     .desc = {
>         {
>             .name = "name",
>             .type = QEMU_OPT_STRING,
>             .def_value_str = "abc",
>         }, {
>             .name = "file",
>             .type = QEMU_OPT_STRING,
>             .def_value_str = "xyz",
>         },
>         { /* end of list */ }
>     },
> };
> 
> but "-foo name,file" still results in qemu_opt_get(opts, "name")
> returning "on", rather than "abc". Is that a bug, or am I missing
> something, or doing something wrong ?

Specifically, in qemu_opt_get() (in util/qemu-option.c:311)
desc->def_value_str is only used if qemu_opt_find() returns NULL,
which in my case does NOT happen ("on" is returned instead).

That, in turn, probably happens because opts_do_parse()
(in util/qemu-options.c:776) assumes that "option without value"
is "probably a flag" :)

Not sure we'd want to mess with that assumption, but that most likely
means I don't get to use .def_value_str the way I thought I could :)

Thanks,
--Gabriel

Re: [Qemu-devel] help parsing qemu options

[Kevin Wolf]

Am 11.03.2015 um 13:53 hat Gabriel L. Somlo geschrieben:
> On Wed, Mar 11, 2015 at 08:40:46AM -0400, Gabriel L. Somlo wrote:
> > On Wed, Mar 11, 2015 at 12:59:40PM +0100, Kevin Wolf wrote:
> > > 
> > > def_value_str exists in the current codebase, and it seems to take
> > > precedence when a different default is specified by the caller.
> > 
> > So I've now added .def_value_str fields:
> > 
> > static QemuOptsList qemu_foo_opts = {
> >     .name = "foo",
> >     .head = QTAILQ_HEAD_INITIALIZER(qemu_foo_opts.head),
> >     .desc = {
> >         {
> >             .name = "name",
> >             .type = QEMU_OPT_STRING,
> >             .def_value_str = "abc",
> >         }, {
> >             .name = "file",
> >             .type = QEMU_OPT_STRING,
> >             .def_value_str = "xyz",
> >         },
> >         { /* end of list */ }
> >     },
> > };
> > 
> > but "-foo name,file" still results in qemu_opt_get(opts, "name")
> > returning "on", rather than "abc". Is that a bug, or am I missing
> > something, or doing something wrong ?
> 
> Specifically, in qemu_opt_get() (in util/qemu-option.c:311)
> desc->def_value_str is only used if qemu_opt_find() returns NULL,
> which in my case does NOT happen ("on" is returned instead).
> 
> That, in turn, probably happens because opts_do_parse()
> (in util/qemu-options.c:776) assumes that "option without value"
> is "probably a flag" :)
> 
> Not sure we'd want to mess with that assumption, but that most likely
> means I don't get to use .def_value_str the way I thought I could :)

Indeed, setting a default value alone won't solve your problem. It might
help you with implementing the solution, though, which would involve
something like changing the meaning of valueless options to taking the
default and using "on" as the default for boolean values.

I haven't thought thoroughly about QemuOptsLists yet which accept any
option. They may still be a reason for the approach outlined above to
fail.

Kevin