qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/1] util/async-teardown: wire up query-command-line-options
@ 2023-03-24 17:45 Claudio Imbrenda
  2023-03-24 17:45 ` [PATCH v3 1/1] " Claudio Imbrenda
  0 siblings, 1 reply; 6+ messages in thread
From: Claudio Imbrenda @ 2023-03-24 17:45 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, david, thuth, borntraeger, frankja, fiuczy, pasic,
	nsg, berrange, alex.bennee, armbru

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 adding a new -teardown commandline option
with an async=on|off parameter, correctly wired up so that it appears
in the output of query-command-line-options.

v2->v3
* add a new teardown option with an async parameter [Markus]
* reworded documentation of existing -async-teardown option so that it
  points to the new teardown option

v1->v2
* remove the unneeded .implied_opt_name initializer [Thomas]

Claudio Imbrenda (1):
  util/async-teardown: wire up query-command-line-options

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

-- 
2.39.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
  2023-03-24 17:45 [PATCH v3 0/1] util/async-teardown: wire up query-command-line-options Claudio Imbrenda
@ 2023-03-24 17:45 ` Claudio Imbrenda
  2023-03-24 17:56   ` Thomas Huth
  2023-03-27  7:20   ` Thomas Huth
  0 siblings, 2 replies; 6+ messages in thread
From: Claudio Imbrenda @ 2023-03-24 17:45 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, david, thuth, borntraeger, frankja, fiuczy, pasic,
	nsg, berrange, alex.bennee, armbru

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
+ERST
+
+DEF("teardown", HAS_ARG, QEMU_OPTION_teardown,
+    "-teardown async[=on|off]\n"
+    "                process teardown options\n"
+    "                async=on enables asynchronous teardown\n"
+   ,
+    QEMU_ARCH_ALL)
+SRST
+``-teardown``
+    Set process teardown options.
+
+    ``async=on`` enables 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.
 ERST
+#endif
 
 DEF("msg", HAS_ARG, QEMU_OPTION_msg,
     "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 62cdeb0f20..4a5dbce958 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,21 @@ 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_teardown_opts = {
+    .name = "teardown",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_teardown_opts.head),
+    .desc = {
+        {
+            .name = "async",
+            .type = QEMU_OPT_BOOL,
+        },
+        { /* end of list */ }
+    },
+};
+
+static void register_teardown(void)
+{
+    qemu_add_opts(&qemu_teardown_opts);
+}
+opts_init(register_teardown);
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
  2023-03-24 17:45 ` [PATCH v3 1/1] " Claudio Imbrenda
@ 2023-03-24 17:56   ` Thomas Huth
  2023-03-24 19:10     ` Claudio Imbrenda
  2023-03-27  7:20   ` Thomas Huth
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2023-03-24 17:56 UTC (permalink / raw)
  To: Claudio Imbrenda, pbonzini
  Cc: qemu-devel, david, borntraeger, frankja, fiuczy, pasic, nsg,
	berrange, alex.bennee, armbru

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 
two releases.

  Thomas



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
  2023-03-24 17:56   ` Thomas Huth
@ 2023-03-24 19:10     ` Claudio Imbrenda
  2023-03-27  7:06       ` Thomas Huth
  0 siblings, 1 reply; 6+ messages in thread
From: Claudio Imbrenda @ 2023-03-24 19:10 UTC (permalink / raw)
  To: Thomas Huth
  Cc: pbonzini, qemu-devel, david, borntraeger, frankja, fiuczy, pasic,
	nsg, berrange, alex.bennee, armbru

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)

> two releases.
> 
>   Thomas
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
  2023-03-24 19:10     ` Claudio Imbrenda
@ 2023-03-27  7:06       ` Thomas Huth
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2023-03-27  7:06 UTC (permalink / raw)
  To: Claudio Imbrenda
  Cc: pbonzini, qemu-devel, david, borntraeger, frankja, fiuczy, pasic,
	nsg, berrange, alex.bennee, armbru

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
  2023-03-24 17:45 ` [PATCH v3 1/1] " Claudio Imbrenda
  2023-03-24 17:56   ` Thomas Huth
@ 2023-03-27  7:20   ` Thomas Huth
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2023-03-27  7:20 UTC (permalink / raw)
  To: Claudio Imbrenda, pbonzini
  Cc: qemu-devel, david, borntraeger, frankja, fiuczy, pasic, nsg,
	berrange, alex.bennee, armbru

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;

Maybe it's better to use exit(1) here (like it is done in the -runas part), 
otherwise you get a somewhat weird second error message:

$ ./qemu-system-s390x -teardown aysnc=on
qemu-system-s390x: -teardown aysnc=on: Invalid parameter 'aysnc'
qemu-system-s390x: -teardown aysnc=on: Option not supported in this build

> +        }
> +        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
> +ERST
> +
> +DEF("teardown", HAS_ARG, QEMU_OPTION_teardown,
> +    "-teardown async[=on|off]\n"
> +    "                process teardown options\n"
> +    "                async=on enables asynchronous teardown\n"
> +   ,
> +    QEMU_ARCH_ALL)
> +SRST
> +``-teardown``
> +    Set process teardown options.
> +
> +    ``async=on`` enables 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

While you're at it, we officially spell QEMU with capital letters, so I'd 
maybe do a s/qemu/QEMU/g here.

> +    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.
>   ERST
> +#endif
>   
>   DEF("msg", HAS_ARG, QEMU_OPTION_msg,
>       "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
> diff --git a/util/async-teardown.c b/util/async-teardown.c
> index 62cdeb0f20..4a5dbce958 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,21 @@ 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_teardown_opts = {
> +    .name = "teardown",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_teardown_opts.head),
> +    .desc = {
> +        {
> +            .name = "async",
> +            .type = QEMU_OPT_BOOL,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
> +static void register_teardown(void)
> +{
> +    qemu_add_opts(&qemu_teardown_opts);
> +}
> +opts_init(register_teardown);

  Thomas



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-27  7:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 17:45 [PATCH v3 0/1] util/async-teardown: wire up query-command-line-options Claudio Imbrenda
2023-03-24 17:45 ` [PATCH v3 1/1] " Claudio Imbrenda
2023-03-24 17:56   ` Thomas Huth
2023-03-24 19:10     ` Claudio Imbrenda
2023-03-27  7:06       ` Thomas Huth
2023-03-27  7:20   ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).