[Top][All Lists]

[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)

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]