qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Thomas Huth
Subject: Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
Date: Mon, 27 Mar 2023 09:06:33 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

On 24/03/2023 20.10, Claudio Imbrenda wrote:
On Fri, 24 Mar 2023 18:56:06 +0100
Thomas Huth <thuth@redhat.com> wrote:

On 24/03/2023 18.45, Claudio Imbrenda wrote:
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.

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>
---
   os-posix.c            | 14 ++++++++++++++
   qemu-options.hx       | 35 ++++++++++++++++++++++++-----------
   util/async-teardown.c | 21 +++++++++++++++++++++
   3 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 5adc69f560..48acd7acf5 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -36,6 +36,8 @@
   #include "qemu/log.h"
   #include "sysemu/runstate.h"
   #include "qemu/cutils.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
#ifdef CONFIG_LINUX
   #include <sys/prctl.h>
@@ -132,6 +134,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
    */
   int os_parse_cmd_args(int index, const char *optarg)
   {
+    QemuOpts *opts;
+
       switch (index) {
       case QEMU_OPTION_runas:
           user_pwd = getpwnam(optarg);
@@ -155,6 +159,16 @@ int os_parse_cmd_args(int index, const char *optarg)
       case QEMU_OPTION_asyncteardown:
           init_async_teardown();
           break;
+    case QEMU_OPTION_teardown:
+        opts = qemu_opts_parse_noisily(qemu_find_opts("teardown"),
+                                       optarg, false);
+        if (!opts) {
+            return -1;
+        }
+        if (qemu_opt_get_bool(opts, "async", false)) {
+            init_async_teardown();
+        }
+        break;
   #endif
       default:
           return -1;
diff --git a/qemu-options.hx b/qemu-options.hx
index d42f60fb91..8582980b12 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4766,20 +4766,33 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
   DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
       "-async-teardown enable asynchronous teardown\n",
       QEMU_ARCH_ALL)
-#endif
   SRST
   ``-async-teardown``
-    Enable asynchronous teardown. A new process called "cleanup/<QEMU_PID>"
-    will be created at startup sharing the address space with the main qemu
-    process, using clone. It will wait for the main qemu process to
-    terminate completely, and then exit.
-    This allows qemu to terminate very quickly even if the guest was
-    huge, leaving the teardown of the address space to the cleanup
-    process. Since the cleanup process shares the same cgroups as the
-    main qemu process, accounting is performed correctly. This only
-    works if the cleanup process is not forcefully killed with SIGKILL
-    before the main qemu process has terminated completely.
+    Equivalent to -teardown async=on

We should avoid of providing multiple ways of doing the same thing to the
users if there is no real benefit. So I'd vote for either removing the
"-async-teardown" option here directly (since it just has been introduced in
7.2 and there are no known users out there yet), or at least deprecate it
(put an entry in docs/about/deprecated.rst), so we can remove it again in

both are fine for me (although I have a slight preference for removing
it altogether)

If nobody objects, i.e. if we feel certain that nobody is really using the old option yet, I'd also prefer if we'd remove it immediately.

 Thomas





reply via email to

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