Re: [PATCH v2 1/1] util/async-teardown: wire up query-command-line-options

[Thomas Huth]

On 20/03/2023 16.31, Markus Armbruster wrote:
Claudio Imbrenda <imbre...@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 <fiu...@linux.ibm.com>
Fixes: c891c24b1a ("os-posix: asynchronous teardown for shutdown on Linux")
Signed-off-by: Claudio Imbrenda <imbre...@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" ?

 Thomas