Re: [PATCH V3 4/9] vl: helper to request exec

[Peter Xu <peterx@redhat.com>]

On Thu, Aug 14, 2025 at 10:17:18AM -0700, Steve Sistare wrote:
> Add a qemu_system_exec_request() hook that causes the main loop to exit and
> exec a command using the specified arguments.  This will be used during CPR
> to exec a new version of QEMU.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  include/system/runstate.h |  3 +++
>  system/runstate.c         | 29 +++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/system/runstate.h b/include/system/runstate.h
> index 929379a..c005f49 100644
> --- a/include/system/runstate.h
> +++ b/include/system/runstate.h
> @@ -128,6 +128,8 @@ typedef enum WakeupReason {
>      QEMU_WAKEUP_REASON_OTHER,
>  } WakeupReason;
>  
> +typedef void (*qemu_exec_func)(char **exec_argv);
> +
>  void qemu_system_reset_request(ShutdownCause reason);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
> @@ -139,6 +141,7 @@ void qemu_register_wakeup_support(void);
>  void qemu_system_shutdown_request_with_code(ShutdownCause reason,
>                                              int exit_code);
>  void qemu_system_shutdown_request(ShutdownCause reason);
> +void qemu_system_exec_request(qemu_exec_func func, const strList *args);
>  void qemu_system_powerdown_request(void);
>  void qemu_register_powerdown_notifier(Notifier *notifier);
>  void qemu_register_shutdown_notifier(Notifier *notifier);
> diff --git a/system/runstate.c b/system/runstate.c
> index 6178b00..b4980ff 100644
> --- a/system/runstate.c
> +++ b/system/runstate.c
> @@ -41,6 +41,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-run-state.h"
>  #include "qapi/qapi-events-run-state.h"
> +#include "qapi/type-helpers.h"
>  #include "qemu/accel.h"
>  #include "qemu/error-report.h"
>  #include "qemu/job.h"
> @@ -422,6 +423,8 @@ static NotifierList wakeup_notifiers =
>  static NotifierList shutdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(shutdown_notifiers);
>  static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);
> +qemu_exec_func exec_func;
> +static char **exec_argv;
>  
>  ShutdownCause qemu_shutdown_requested_get(void)
>  {
> @@ -443,6 +446,11 @@ static int qemu_shutdown_requested(void)
>      return qatomic_xchg(&shutdown_requested, SHUTDOWN_CAUSE_NONE);
>  }
>  
> +static int qemu_exec_requested(void)
> +{
> +    return exec_argv != NULL;
> +}
> +
>  static void qemu_kill_report(void)
>  {
>      if (!qtest_driver() && shutdown_signal) {
> @@ -803,6 +811,23 @@ void qemu_system_shutdown_request(ShutdownCause reason)
>      qemu_notify_event();
>  }
>  
> +static void qemu_system_exec(void)
> +{
> +    exec_func(exec_argv);
> +
> +    /* exec failed */
> +    g_strfreev(exec_argv);
> +    exec_argv = NULL;
> +    exec_func = NULL;

Would this really happen?

If so, do we at least want to dump something?

> +}
> +
> +void qemu_system_exec_request(qemu_exec_func func, const strList *args)
> +{
> +    exec_func = func;
> +    exec_argv = strv_from_str_list(args);
> +    qemu_notify_event();
> +}
> +
>  static void qemu_system_powerdown(void)
>  {
>      qapi_event_send_powerdown();
> @@ -849,6 +874,10 @@ static bool main_loop_should_exit(int *status)
>      if (qemu_suspend_requested()) {
>          qemu_system_suspend();
>      }
> +    if (qemu_exec_requested()) {
> +        qemu_system_exec();
> +        return false;
> +    }

Some explanation of why it needs to be done explicitly here would be
helpful.  E.g., can we do exec inside a BH scheduled for the main thread?
What if we exec() directly in another thread (rather than the main loop
thread)?

>      request = qemu_shutdown_requested();
>      if (request) {
>          qemu_kill_report();
> -- 
> 1.8.3.1
> 

-- 
Peter Xu