Re: [PATCH v2 3/4] qom: Add user_creatable_print_help_from_qdict()

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Kevin Wolf <kwolf@redhat.com> writes:
>
>> This adds a function that, given a QDict of non-help options, prints
>> help for user creatable objects.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  include/qom/object_interfaces.h | 9 +++++++++
>>  qom/object_interfaces.c         | 9 +++++++++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/include/qom/object_interfaces.h 
>> b/include/qom/object_interfaces.h
>> index f118fb516b..53b114b11a 100644
>> --- a/include/qom/object_interfaces.h
>> +++ b/include/qom/object_interfaces.h
>> @@ -161,6 +161,15 @@ int user_creatable_add_opts_foreach(void *opaque,
>>   */
>>  bool user_creatable_print_help(const char *type, QemuOpts *opts);
>>  
>> +/**
>> + * user_creatable_print_help_from_qdict:
>> + * @args: options to create
>> + *
>> + * Prints help considering the other options given in @args (if "qom-type" 
>> is
>> + * given and valid, print properties for the type, otherwise print valid 
>> types)
>> + */
>> +void user_creatable_print_help_from_qdict(QDict *args);
>> +
>>  /**
>>   * user_creatable_del:
>>   * @id: the unique ID for the object
>> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
>> index 3fd1da157e..ed896fe764 100644
>> --- a/qom/object_interfaces.c
>> +++ b/qom/object_interfaces.c
>> @@ -279,6 +279,15 @@ bool user_creatable_print_help(const char *type, 
>> QemuOpts *opts)
>>      return false;
>>  }
>>  
>> +void user_creatable_print_help_from_qdict(QDict *args)
>> +{
>> +    const char *type = qdict_get_try_str(args, "qom-type");
>> +
>> +    if (!type || !user_creatable_print_type_properites(type)) {
>> +        user_creatable_print_types();
>> +    }
>
> Existing user_creatable_print_help():
>
> 1. "qom-type=help,..." and its sugared forms, in particular "help"
>
>    List QOM types and succeed.
>
> 2. "qom-type=T,help,..." 
>
> 2a. If T names a QOM type
>
>     List T's properties and succeed.
>
> 2b. If T does not name a QOM type
>
>     Fail.  Callers typically interpret this as "no help requested",
>     proceed, then choke on invalid qom-type=T.

And of course

  3. Else

     No help requested; fail.

> New user_creatable_print_help() treats case 2b like case 1.
>
> Intentional?

The next patch relies on this, so I figure the answer is yes.

The difference to user_creatable_print_help() is subtle.  Perhaps the
contract should point it out explicitly.  Up to you.

By the way, the contract of user_creatable_print_help() is inaccurate:

 * Prints help if requested in @opts.
 *
 * Returns: true if @opts contained a help option and help was printed, false
 * if no help option was found.

One, it prints help when either @type or @opts request it.

Two, "if no help option was found" is misleading: case 2b.

Not this patch's problem.
>
>> +}
>> +
>>  bool user_creatable_del(const char *id, Error **errp)
>>  {
>>      Object *container;

Reviewed-by: Markus Armbruster <armbru@redhat.com>