Re: [Qemu-devel] [RFC 4/6] CLI: add -paused option

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Oct 16, 2017 at 06:22:54PM +0200, Igor Mammedov wrote:

This really needs to have a commit message that provides justification
for why this option is needed when we already have -S that is used
to allow configuration before the guest starts.

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/sysemu/sysemu.h |  1 +
>  qemu-options.hx         | 15 ++++++++++++++
>  qmp.c                   |  5 +++++
>  vl.c                    | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b213696..3feb94f 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -66,6 +66,7 @@ typedef enum WakeupReason {
>      QEMU_WAKEUP_REASON_OTHER,
>  } WakeupReason;
>  
> +void qemu_exit_preconfig_request(void);
>  void qemu_system_reset_request(ShutdownCause reason);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 39225ae..bd44db8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3498,6 +3498,21 @@ STEXI
>  Run the emulation in single step mode.
>  ETEXI
>  
> +DEF("paused", HAS_ARG, QEMU_OPTION_paused, \
> +    "-paused [state=]postconf|preconf\n"
> +    "                postconf: pause QEMU after machine is initialized\n"
> +    "                preconf: pause QEMU before machine is initialized\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -paused
> +@findex -paused
> +if set enabled interactive configuration stages before machine emulation starts.
> +'postconf' option value mimics -S option behaviour where machine is created
> +but emulation isn't started. 'preconf' option value pauses QEMU before machine
> +is created, which allows to query and configure properties affecting machine
> +initialization. Use monitor/QMP command 'cont' to go to exit paused state.
> +ETEXI

To me it feels rather unpleasant to be exposing this kind of detailed knowledge
about the steps QEMU goes through when consttructing the machine and expecting
the mgmt application to synchronize certain monitor actions against this.

> +
>  DEF("S", 0, QEMU_OPTION_S, \
>      "-S              freeze CPU at startup (use 'c' to start execution)\n",
>      QEMU_ARCH_ALL)
> diff --git a/qmp.c b/qmp.c
> index e8c3031..49e9a5c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -167,6 +167,11 @@ void qmp_cont(Error **errp)
>      BlockBackend *blk;
>      Error *local_err = NULL;
>  
> +    if (runstate_check(RUN_STATE_PRELAUNCH)) {
> +        qemu_exit_preconfig_request();
> +        return;
> +    }
> +
>      /* if there is a dump in background, we should wait until the dump
>       * finished */
>      if (dump_in_progress()) {
> diff --git a/vl.c b/vl.c
> index 3fed457..30631fd 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -555,6 +555,20 @@ static QemuOptsList qemu_fw_cfg_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_paused_opts = {
> +    .name = "paused",
> +    .implied_opt_name = "state",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_paused_opts.head),
> +    .desc = {
> +        {
> +            .name = "state",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Pause state of QEMU on startup",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  /**
>   * Get machine options
>   *
> @@ -1689,6 +1703,11 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> +static enum {
> +    PRECONFIG_CONT = 0,
> +    PRECONFIG_PAUSE,
> +    PRECONFIG_SKIP,
> +} preconfig_requested;
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1773,6 +1792,11 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>  
> +void qemu_exit_preconfig_request(void)
> +{
> +    preconfig_requested = PRECONFIG_CONT;
> +}
> +
>  /*
>   * Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
>   */
> @@ -1939,6 +1963,12 @@ static bool main_loop_should_exit(void)
>      RunState r;
>      ShutdownCause request;
>  
> +    if (runstate_check(RUN_STATE_PRELAUNCH)) {
> +        if (preconfig_requested == PRECONFIG_CONT) {
> +            preconfig_requested = PRECONFIG_SKIP;
> +            return true;
> +        }
> +    }
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
> @@ -3177,6 +3207,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_icount_opts);
>      qemu_add_opts(&qemu_semihosting_config_opts);
>      qemu_add_opts(&qemu_fw_cfg_opts);
> +    qemu_add_opts(&qemu_paused_opts);
>      module_call_init(MODULE_INIT_OPTS);
>  
>      runstate_init();
> @@ -3845,6 +3876,26 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>                  break;
> +            case QEMU_OPTION_paused:
> +                {
> +                    const char *value;
> +
> +                    opts = qemu_opts_parse_noisily(qemu_find_opts("paused"),
> +                                                   optarg, true);
> +                    if (opts == NULL) {
> +                        exit(1);
> +                    }
> +                    value = qemu_opt_get(opts, "state");
> +                    if (!strcmp(value, "postconf")) {
> +                        autostart = 0;
> +                    } else if (!strcmp(value, "preconf")) {
> +                        preconfig_requested = PRECONFIG_PAUSE;
> +                    } else {
> +                        error_report("incomplete '-paused' option\n");
> +                        exit(1);
> +                    }
> +                    break;
> +                }
>              case QEMU_OPTION_enable_kvm:
>                  olist = qemu_find_opts("machine");
>                  qemu_opts_parse_noisily(olist, "accel=kvm", false);
> @@ -4731,7 +4782,6 @@ int main(int argc, char **argv, char **envp)
>      current_machine->boot_order = boot_order;
>      current_machine->cpu_model = cpu_model;
>  
> -
>      /* parse features once if machine provides default cpu_type */
>      if (machine_class->default_cpu_type) {
>          current_machine->cpu_type = machine_class->default_cpu_type;
> @@ -4741,6 +4791,8 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +    main_loop(); /* do monitor/qmp handling at preconfig state if requested */
> +
>      machine_run_board_init(current_machine);
>  
>      realtime_init();
> -- 
> 2.7.4
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|