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

[Eric Blake <eblake@redhat.com>]

On 09/06/2018 11:27 AM, Marc-André Lureau wrote:

> We use name=type (text) for devices properties, ex:
> 
> qemu-system-x86_64 -device tpm-tis,?
> tpm-tis.tpmdev=str (ID of a tpm to use as a backend)
> tpm-tis.irq=uint32
> tpm-tis.tpm-tis-mmio[0]=child<qemu:memory-region>
> 
> But
> qemu-img create -f qcow2 -o ?
> 
>      size             Virtual disk size
>      compat           Compatibility level (0.10 or 1.1)
>      backing_file     File name of a base image
> 
> I think I like more "name=type - text" form I introduced in "improve
> qemu_opts_print_help() output". I guess I should change device
> properties help for consistency then.

I don't have a strong preference for one form over the other, so much as 
consistency in the various applications using the form.


>>> +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?
> 
> as qemu_pstrcmp in cutils? inline in the header?

Does glib not already have such a helper? cutils sounds as good a place 
as any, although inline may be at odds with typically using it as a 
callback function (I don't know how well the compiler and linker handle 
such a situation).

> 
>>
>>
>>> +        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.
> 
> We use error_printf() for qdev list (qdev_device_help), which
> redirects to monitor or stderr.
> 
> Should I also change ir for consistency? hopefully nobody relies on
> the output going to stderr...

I don't worry too much about that. We've already fixed other places 
where qemu binaries were antisocial, such as commit ac1307ab fixing 
'qemu-img --help' to give status 0 instead of 1.

In general, when a user asks for help, the help text should go to 
stdout, and the exit status should be 0 (unless you go to great lengths 
to also detect write failures such as ENOSPC or EPIPE, in which case you 
should ALSO attempt to write to stderr prior to exiting with nonzero 
status that you couldn't output the help text - but since FILE* I/O can 
cache things, detecting all possible write errors is not reliable unless 
you check the result of fclose(stdout), which in turn has to be deferred 
to the end of your program execution, generally via atexit().  The extra 
complications and code maintenance for checking for --help output 
failures is something that I'm personally okay with skipping).

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