Lin Ma <l...@suse.com> writes:
> '-object help' prints available user creatable backends.
> '-object $typename,help' prints relevant properties.
>
> Signed-off-by: Lin Ma <l...@suse.com>
> ---
> include/qom/object_interfaces.h | 2 ++
> qemu-options.hx | 7 +++++-
> qom/object_interfaces.c | 55
> +++++++++++++++++++++++++++++++++++++++++
> vl.c | 5 ++++
> 4 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index 8b17f4d..197cd5f 100644
> --- a/include/qom/object_interfaces.h
> +++ b/include/qom/object_interfaces.h
> @@ -165,4 +165,6 @@ int user_creatable_add_opts_foreach(void *opaque,
> */
> void user_creatable_del(const char *id, Error **errp);
>
> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp);
> +
Ugly interface: two arguments that aren't used, one of them void *.
Suggest to follow the device help example: do a non-ugly interface here,
and an ugly, but static wrapper in vl.c
> #endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b1fbdb0..b9573ce 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3761,7 +3761,9 @@ DEF("object", HAS_ARG, QEMU_OPTION_object,
> " create a new object of type TYPENAME setting
> properties\n"
> " in the order they are specified. Note that the 'id'\n"
> " property must be set. These objects are placed in
> the\n"
> - " '/objects' path.\n",
> + " '/objects' path.\n"
> + " Use '-object help' to print available backend types
> and\n"
> + " '-object typename,help' to print relevant
> properties.\n",
The existing text talks about "object of type TYPENAME". The new text
suddenly talks about "backend types". Switching terminology is not a
good idea. Call them "object types".
For comparison, here's the text we use for -device:
use '-device help' to print all possible drivers
use '-device driver,help' to print all possible properties
Let's make this one similar:
use '-object help' to print all possible object types
use '-object driver,help' to print all possible properties
> QEMU_ARCH_ALL)
> STEXI
> @item -object @var{typename}[,@var{prop1}=@var{value1},...]
> @@ -3771,6 +3773,9 @@ in the order they are specified. Note that the 'id'
> property must be set. These objects are placed in the
> '/objects' path.
>
> +Use @code{-object help} to print available backend types and
> +@code{-object @var{typename},help} to print relevant properties.
> +
-device's text:
To get help on possible drivers and properties, use @code{-device
help} and @code{-device @var{driver},help}.
Suggest:
To get help on possible object types and properties, use @code{-object
help} and @code{-object @var{type},help}.
> @table @option
>
> @item -object
> memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index bf59846..da8be39 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -5,6 +5,7 @@
> #include "qapi-visit.h"
> #include "qapi/qmp-output-visitor.h"
> #include "qapi/opts-visitor.h"
> +#include "qemu/help_option.h"
>
> void user_creatable_complete(Object *obj, Error **errp)
> {
> @@ -212,6 +213,60 @@ void user_creatable_del(const char *id, Error **errp)
> object_unparent(obj);
> }
>
> +int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
> +{
> + const char *type = NULL;
> + Object *obj = NULL;
> + ObjectClass *klass;
> + ObjectProperty *prop;
> + ObjectPropertyIterator iter;
> +
> + type = qemu_opt_get(opts, "qom-type");
> + if (type && is_help_option(type)) {
> + GSList *list;
> + printf("Available object backend types:\n");
Wrong when running for an HMP monitor command, e.g. "object_add help".
Use error_printf(). That's only slightly wrong in that it prints to
stderr instead of stdout running for the command line.
> + for (list = object_class_get_list(TYPE_USER_CREATABLE, false); \
> + list; \
> + list = list->next) {
Superfluous line continuations. Simply write
for (list = object_class_get_list(TYPE_USER_CREATABLE, false);
list;
list = list->next) {
Alternatively,
list = object_class_get_list(TYPE_USER_CREATABLE, false);
for (elt = list; elt; elt = elt->next) {
> + const char *name;
> + name = object_class_get_name(OBJECT_CLASS(list->data));
> + if (strcmp(name, TYPE_USER_CREATABLE)) {
> + printf("%s\n", name);
error_printf()
> + }
> + }
> + g_slist_free(list);
Leaks the list, because by now @list is null. Suggest to fix by
adopting the code under "alternatively".
> + goto out;
Suggest return 1, because ...
> + }
> +
> + if (!type || !qemu_opt_has_help_opt(opts)) {
> + return 0;
... you're not going to out here.
No need to initialize @obj then.
> + }
> +
> + klass = object_class_by_name(type);
> + if (!klass) {
> + printf("invalid object type: %s\n", type);
Please use error_report().
> + goto out;
return 1 would do.
> + }
> + if (object_class_is_abstract(klass)) {
> + printf("object type '%s' is abstract\n", type);
Please use error_report().
> + goto out;
return 1 would do.
> + }
I'd put a blank line here ...
> + obj = object_new(type);
> + object_property_iter_init(&iter, obj);
> +
... instead of here.
> + while ((prop = object_property_iter_next(&iter))) {
> + if (prop->description) {
> + printf("%s (%s, %s)\n", prop->name, prop->type,
> prop->description);
> + } else {
> + printf("%s (%s)\n", prop->name, prop->type);
> + }
error_printf()
> + }
There's a very similar loop in machine_help_func(). Can we avoid the
duplication?
> +
> +out:
You don't actually need this label.
> + object_unref(obj);
> + return 1;
> +}
> +
> static void register_types(void)
> {
> static const TypeInfo uc_interface_info = {
> diff --git a/vl.c b/vl.c
> index ebd47af..1456eca 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
> exit(0);
> }
>
> + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
> + NULL, NULL)) {
> + exit(1);
> + }
> +
> if (!trace_init_backends()) {
> exit(1);
> }
Can you explain why you picked this spot in main()?
