[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-optio
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options |
Date: |
Mon, 20 Mar 2023 16:31:00 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Claudio Imbrenda <imbrenda@linux.ibm.com> writes:
> The recently introduced -async-teardown commandline option was not
> wired up properly and did not show up in the output of the QMP command
> query-command-line-options. This means that libvirt will have no way to
> discover whether the feature is supported.
There was nothing improper in its wiring. The issue is that
query-command-line-options is junk. See my recent post
Subject: query-command-line-options (was: [PATCH 1/7] qemu: capabilities:
Introduce QEMU_CAPS_MACHINE_ACPI)
Date: Tue, 07 Mar 2023 10:40:23 +0100
Message-ID: <87jzzsc320.fsf_-_@pond.sub.org>
> This patch fixes the issue by correctly wiring up the commandline
> option so that it appears in the output of query-command-line-options.
>
> Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Fixes: c891c24b1a ("os-posix: asynchronous teardown for shutdown on Linux")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> util/async-teardown.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/util/async-teardown.c b/util/async-teardown.c
> index 62cdeb0f20..c9b9a3cdb2 100644
> --- a/util/async-teardown.c
> +++ b/util/async-teardown.c
> @@ -12,6 +12,9 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/config-file.h"
> +#include "qemu/option.h"
> +#include "qemu/module.h"
> #include <dirent.h>
> #include <sys/prctl.h>
> #include <sched.h>
> @@ -144,3 +147,17 @@ void init_async_teardown(void)
> clone(async_teardown_fn, new_stack_for_clone(), CLONE_VM, NULL);
> sigprocmask(SIG_SETMASK, &old_signals, NULL);
> }
> +
> +static QemuOptsList qemu_async_teardown_opts = {
> + .name = "async-teardown",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_async_teardown_opts.head),
> + .desc = {
> + { /* end of list */ }
> + },
> +};
> +
> +static void register_async_teardown(void)
> +{
> + qemu_add_opts(&qemu_async_teardown_opts);
> +}
> +opts_init(register_async_teardown);
Now it *is* improperly wired up :)
You're defining new QemuOpts config group "async-teardown" with
arbitrary option parameters, but don't actually use it for parsing or
recording the option. I figure because you can't: there is no option
argument to parse and record, which is what QemuOpts is designed to do.
If you need the feature to be visible in query-command-line-options, you
should make it an option parameter (a KEY, not a GROUP), preferably of
an existing group / option.