[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/10] vl: list user creatable propeties if '?'
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 10/10] vl: list user creatable propeties if '?' as argument |
Date: |
Thu, 6 Sep 2018 12:05:19 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
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
- [Qemu-devel] [PATCH 04/10] qom/object: register 'type' property as class property, (continued)
- [Qemu-devel] [PATCH 04/10] qom/object: register 'type' property as class property, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 06/10] tests/qom-proplist: check properties are not listed multiple times, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 07/10] tests/qom-proplist: check class properties iterator, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 08/10] vl: handle -object ?, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 05/10] tests/qom-proplist: check duplicate "bv" property registration failed, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 09/10] hostmem: add some properties description, Marc-André Lureau, 2018/09/06
- [Qemu-devel] [PATCH 10/10] vl: list user creatable propeties if '?' as argument, Marc-André Lureau, 2018/09/06