Re: [Qemu-devel] [PATCH 10/10] vl: list user creatable propeties if '?' as argument

[Eric Blake]

On 09/06/2018 10:12 AM, Marc-André Lureau wrote:

Subject has typo and awkward grammar; I'd suggest:

vl: list user creatable properties when 'help' is argument

> Iterate over the writable class properties, sort and print them out
> with the description if available.
>
> Ex: qemu -object memory-backend-memfd,?

As elsewhere in the series, s/\?/help/

> memory-backend-memfd.dump=bool (Set to 'off' to exclude from core dump)
> memory-backend-memfd.host-nodes=int (Binds memory to the list of NUMA host 
> nodes)
> memory-backend-memfd.hugetlb=bool (Use huge pages)
> memory-backend-memfd.hugetlbsize=int (Huge pages size (ex: 2M, 1G))
> memory-backend-memfd.merge=bool (Mark memory as mergeable)
> memory-backend-memfd.policy=HostMemPolicy (Set the NUMA policy)
> memory-backend-memfd.prealloc=bool (Preallocate memory)
> memory-backend-memfd.seal=bool (Seal growing & shrinking)
> memory-backend-memfd.share=bool (Mark the memory as private to QEMU or shared)
> memory-backend-memfd.size=int (Size of the memory region (ex: 500M))

Why "name=type (text)" here, but "name=type - text" in 2/10? 
Consistency would be nice.

> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>   qom/object_interfaces.c |  6 +++---
>   vl.c                    | 45 ++++++++++++++++++++++++++++++++++++++---
>   2 files changed, 45 insertions(+), 6 deletions(-)

> +++ b/vl.c
> @@ -2721,6 +2721,11 @@ static int machine_set_property(void *opaque,
>       return 0;
>   }
>   
> +static gint
> +pstrcmp(const char **a, const char **b)
> +{
> +    return g_strcmp0(*a, *b);
> +}

This is the second time your series has added this static helper. Should 
it be a common helper instead?


> +        g_ptr_array_sort(array, (GCompareFunc)pstrcmp);
> +        for (i = 0; i < array->len; i++) {
> +            error_printf("%s\n", (char *)array->pdata[i]);
> +        }
> +        g_ptr_array_set_free_func(array, g_free);
> +        g_ptr_array_free(array, true);
> +        exit(0);

Again, printing to stderr then exiting with status 0 is awkward.  Print 
to stdout when successfully offering help text.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org