Re: [Qemu-devel] [PATCH 3/9] QMP: First half of the new argument checking code

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Wed, 02 Jun 2010 08:59:11 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Luiz Capitulino <address@hidden> writes:
>
> [...]
>
>> > +
>> > +    type = qobject_to_qstring(obj);
>> > +    assert(type != NULL);
>> > +
>> > +    if (qstring_get_str(type)[0] == 'O') {
>> > +        QemuOptsList *opts_list = qemu_find_opts(cmd_arg_name);
>> > +        assert(opts_list);
>> > +        res->result = check_opts(opts_list, res->qdict);
>> 
>> I doubt this is the right place for calling check_opts.
>
>  Can you suggest the right place?

It's related to check_arg().  I figure it should be dropped along with
it in 5/9.

The new checker needs to cope with 'O':

1. Client must provide all mandatory arguments

   'O' options are optional, so there's nothing to do for
   check_mandatory_args().

2. Each argument provided by the client must be valid
3. Each argument provided by the client must have the type expected
   by the command

   'O' comes in two flavours, like the underlying QemuOptsList: desc
   present (not yet implemented) and empty.

   Empty desc means we accept any option.  check_client_args_type()
   needs to accept unknown arguments.  Is this broken in your patch?

   Non-empty desc probably should be exploded by qdict_from_args_type(),
   i.e. instead of putting (NAME, "O") into the dictionary, put the
   contents of QemuOptsList's desc.  Problem: key is obvious
   (desc->name), but value is not.  Maybe you need to represent
   "expected type" differently, so you can cover both arg_type codes and
   the QemuOptType values.  What its user really needs to know is the
   set of expected types.  Why not put that, in a suitable encoding.

   I recommend to implement only empty desc here for now, and non-empty
   when we implement it.  Would be nice if you could keep it in mind, so
   we don't have to dig up too much of the argument checker for it.