[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: |
Claudio Imbrenda |
Subject: |
Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options |
Date: |
Mon, 20 Mar 2023 17:21:17 +0100 |
On Mon, 20 Mar 2023 17:05:07 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
> > On 20/03/2023 16.31, Markus Armbruster wrote:
> >> 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.
> >
> > Would it make sense to add it e.g. to "-action" instead, i.e. something
> > like
> > "-action teardown=async" ?
>
> I believe the new parameter "teardown" would be visible in
> query-command-line-options.
>
> How well does it fit -action?
I guess it can be shoehorned in. generally action is about stuff that
happens in/to the guest, while in this case it's about how qemu will
perform the teardown of its address space once it terminates.
the important parts are: this is an OS-specific option (Linux), and it
needs to be parsed and enabled before sandboxing (otherwise clone(2)
might not work)
- [PATCH v2 0/1] util/async-teardown: wire up query-command-line-options, Claudio Imbrenda, 2023/03/20
- [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Claudio Imbrenda, 2023/03/20
- Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Markus Armbruster, 2023/03/20
- Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Paolo Bonzini, 2023/03/21
- Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Daniel P . Berrangé, 2023/03/22
- Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Thomas Huth, 2023/03/22
- Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options, Markus Armbruster, 2023/03/22