diff mbox series

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

Message ID 20200930124557.51835-4-kwolf@redhat.com
State New, archived
Headers show
Series qemu-storage-daemon: Remove QemuOpts from --object parser | expand

Commit Message

Kevin Wolf Sept. 30, 2020, 12:45 p.m. UTC
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(+)

Comments

Eric Blake Sept. 30, 2020, 1:48 p.m. UTC | #1
On 9/30/20 7:45 AM, Kevin Wolf wrote:
> 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(+)

As with v1,
Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Oct. 2, 2020, 12:25 p.m. UTC | #2
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.

New user_creatable_print_help() treats case 2b like case 1.

Intentional?

> +}
> +
>  bool user_creatable_del(const char *id, Error **errp)
>  {
>      Object *container;
Markus Armbruster Oct. 2, 2020, 12:36 p.m. UTC | #3
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>
diff mbox series

Patch

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();
+    }
+}
+
 bool user_creatable_del(const char *id, Error **errp)
 {
     Object *container;