diff mbox series

[28/29] vl: remove separate preconfig main_loop

Message ID 20201027182144.3315885-29-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show
Series cleanup qemu_init and make sense of command line processing | expand

Commit Message

Paolo Bonzini Oct. 27, 2020, 6:21 p.m. UTC 
Move post-preconfig initialization to the x-exit-preconfig.  If preconfig
is not requested, just exit preconfig mode immediately with the QMP
command.

As a result, the preconfig loop will run with accel_setup_post
and os_setup_post restrictions (xen_restrict, chroot, etc.)
already done.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/runstate.h |  1 -
 monitor/qmp-cmds.c        |  9 ----
 softmmu/vl.c              | 94 +++++++++++++++++----------------------
 3 files changed, 41 insertions(+), 63 deletions(-)

Comments

Igor Mammedov Nov. 20, 2020, 4:26 p.m. UTC | #1 
On Tue, 27 Oct 2020 14:21:43 -0400
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Move post-preconfig initialization to the x-exit-preconfig.  If preconfig
> is not requested, just exit preconfig mode immediately with the QMP
> command.
> 
> As a result, the preconfig loop will run with accel_setup_post
> and os_setup_post restrictions (xen_restrict, chroot, etc.)
> already done.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This one also doesn't apply,

+ one more comment below

> ---
>  include/sysemu/runstate.h |  1 -
>  monitor/qmp-cmds.c        |  9 ----
>  softmmu/vl.c              | 94 +++++++++++++++++----------------------
>  3 files changed, 41 insertions(+), 63 deletions(-)
> 
> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index f760094858..e557f470d4 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -41,7 +41,6 @@ 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/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 7c10b182e4..6680ba6c66 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -102,15 +102,6 @@ void qmp_system_powerdown(Error **errp)
>      qemu_system_powerdown_request();
>  }
>  
> -void qmp_x_exit_preconfig(Error **errp)
> -{
> -    if (qdev_hotplug) {
> -        error_setg(errp, "The command is permitted only before machine initialization");
> -        return;
> -    }
> -    qemu_exit_preconfig_request();
> -}
> -
>  void qmp_cont(Error **errp)
>  {
>      BlockBackend *blk;
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 68acd24d01..98666c0612 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1313,7 +1313,6 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> -static bool preconfig_exit_requested = true;
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>      NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1400,11 +1399,6 @@ static int qemu_debug_requested(void)
>      return r;
>  }
>  
> -void qemu_exit_preconfig_request(void)
> -{
> -    preconfig_exit_requested = true;
> -}
> -
>  /*
>   * Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
>   */
> @@ -1626,10 +1620,6 @@ static bool main_loop_should_exit(void)
>      RunState r;
>      ShutdownCause request;
>  
> -    if (preconfig_exit_requested) {
> -        preconfig_exit_requested = false;
> -        return true;
> -    }
>      if (qemu_debug_requested()) {
>          vm_stop(RUN_STATE_DEBUG);
>      }
> @@ -3523,6 +3513,43 @@ static void qemu_machine_creation_done(void)
>      register_global_state();
>  }
>  
> +void qmp_x_exit_preconfig(Error **errp)
> +{
> +    if (qdev_hotplug) {
> +        error_setg(errp, "The command is permitted only before machine initialization");
> +        return;
> +    }
> +
> +    qemu_finish_machine_init();
> +    qemu_create_cli_devices();
> +    qemu_machine_creation_done();
> +
> +    if (loadvm) {
> +        Error *local_err = NULL;
> +        if (load_snapshot(loadvm, &local_err) < 0) {
> +            error_report_err(local_err);
> +            autostart = 0;
> +            exit(1);
> +        }
> +    }
> +    if (replay_mode != REPLAY_MODE_NONE) {
> +        replay_vmstate_init();
> +    }
> +
> +    if (incoming) {
> +        Error *local_err = NULL;
> +        if (strcmp(incoming, "defer") != 0) {
> +            qmp_migrate_incoming(incoming, &local_err);
> +            if (local_err) {
> +                error_reportf_err(local_err, "-incoming %s: ", incoming);
> +                exit(1);
> +            }
> +        }
> +    } else if (autostart) {
> +        qmp_cont(NULL);
> +    }
> +}
> +
>  void qemu_init(int argc, char **argv, char **envp)
>  {
>      QemuOpts *opts;
> @@ -4092,7 +4119,6 @@ void qemu_init(int argc, char **argv, char **envp)
>                  }
>                  break;
>              case QEMU_OPTION_preconfig:
> -                preconfig_exit_requested = false;
>                  preconfig_requested = true;
>                  break;
>              case QEMU_OPTION_enable_kvm:
> @@ -4515,56 +4541,18 @@ void qemu_init(int argc, char **argv, char **envp)
>      qemu_resolve_machine_memdev();
>      parse_numa_opts(current_machine);
>  
> -    if (preconfig_requested) {
> -        qemu_init_displays();
> -    }

^^^

> -
> -    /* do monitor/qmp handling at preconfig state if requested */
> -    qemu_main_loop();
> -    qemu_finish_machine_init();
> -
> -    qemu_create_cli_devices();
> -
> -    /* initialize displays after all errors have been reported */
> -    if (!preconfig_requested) {
> -        qemu_init_displays();
> -    }

^^^

[...]
>  

1)

> +    if (!preconfig_requested) {
> +        qmp_x_exit_preconfig(&error_fatal);
> +    }
> +    qemu_init_displays();
given that qemu_init_displays() were called in both cases,
shouldn't it be called unconditionally at [1]?

>      accel_setup_post(current_machine);
>      os_setup_post();
> -
> -    return;
>  }
>  
>  void qemu_cleanup(void)
Paolo Bonzini Nov. 20, 2020, 4:39 p.m. UTC | #2 
On 20/11/20 17:26, Igor Mammedov wrote:
>> +    if (!preconfig_requested) {
>> +        qmp_x_exit_preconfig(&error_fatal);
>> +    }
>> +    qemu_init_displays();
> given that qemu_init_displays() were called in both cases,
> shouldn't it be called unconditionally at [1]?

Yes, makes sense.  In fact, it would be nicer to also call

     accel_setup_post(current_machine);
     os_setup_post();

before x_exit_preconfig, but it's left for another day.

Paolo
Paolo Bonzini Nov. 23, 2020, 8:34 a.m. UTC | #3 
On 20/11/20 17:26, Igor Mammedov wrote:
>> +    if (!preconfig_requested) {
>> +        qmp_x_exit_preconfig(&error_fatal);
>> +    }
>> +    qemu_init_displays();
> given that qemu_init_displays() were called in both cases,
> shouldn't it be called unconditionally at [1]?
> 

Yes, correct.

Paolo
diff mbox series

Patch

diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
index f760094858..e557f470d4 100644
--- a/include/sysemu/runstate.h
+++ b/include/sysemu/runstate.h
@@ -41,7 +41,6 @@  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/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 7c10b182e4..6680ba6c66 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -102,15 +102,6 @@  void qmp_system_powerdown(Error **errp)
     qemu_system_powerdown_request();
 }
 
-void qmp_x_exit_preconfig(Error **errp)
-{
-    if (qdev_hotplug) {
-        error_setg(errp, "The command is permitted only before machine initialization");
-        return;
-    }
-    qemu_exit_preconfig_request();
-}
-
 void qmp_cont(Error **errp)
 {
     BlockBackend *blk;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 68acd24d01..98666c0612 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1313,7 +1313,6 @@  static pid_t shutdown_pid;
 static int powerdown_requested;
 static int debug_requested;
 static int suspend_requested;
-static bool preconfig_exit_requested = true;
 static WakeupReason wakeup_reason;
 static NotifierList powerdown_notifiers =
     NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
@@ -1400,11 +1399,6 @@  static int qemu_debug_requested(void)
     return r;
 }
 
-void qemu_exit_preconfig_request(void)
-{
-    preconfig_exit_requested = true;
-}
-
 /*
  * Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
  */
@@ -1626,10 +1620,6 @@  static bool main_loop_should_exit(void)
     RunState r;
     ShutdownCause request;
 
-    if (preconfig_exit_requested) {
-        preconfig_exit_requested = false;
-        return true;
-    }
     if (qemu_debug_requested()) {
         vm_stop(RUN_STATE_DEBUG);
     }
@@ -3523,6 +3513,43 @@  static void qemu_machine_creation_done(void)
     register_global_state();
 }
 
+void qmp_x_exit_preconfig(Error **errp)
+{
+    if (qdev_hotplug) {
+        error_setg(errp, "The command is permitted only before machine initialization");
+        return;
+    }
+
+    qemu_finish_machine_init();
+    qemu_create_cli_devices();
+    qemu_machine_creation_done();
+
+    if (loadvm) {
+        Error *local_err = NULL;
+        if (load_snapshot(loadvm, &local_err) < 0) {
+            error_report_err(local_err);
+            autostart = 0;
+            exit(1);
+        }
+    }
+    if (replay_mode != REPLAY_MODE_NONE) {
+        replay_vmstate_init();
+    }
+
+    if (incoming) {
+        Error *local_err = NULL;
+        if (strcmp(incoming, "defer") != 0) {
+            qmp_migrate_incoming(incoming, &local_err);
+            if (local_err) {
+                error_reportf_err(local_err, "-incoming %s: ", incoming);
+                exit(1);
+            }
+        }
+    } else if (autostart) {
+        qmp_cont(NULL);
+    }
+}
+
 void qemu_init(int argc, char **argv, char **envp)
 {
     QemuOpts *opts;
@@ -4092,7 +4119,6 @@  void qemu_init(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_preconfig:
-                preconfig_exit_requested = false;
                 preconfig_requested = true;
                 break;
             case QEMU_OPTION_enable_kvm:
@@ -4515,56 +4541,18 @@  void qemu_init(int argc, char **argv, char **envp)
     qemu_resolve_machine_memdev();
     parse_numa_opts(current_machine);
 
-    if (preconfig_requested) {
-        qemu_init_displays();
-    }
-
-    /* do monitor/qmp handling at preconfig state if requested */
-    qemu_main_loop();
-    qemu_finish_machine_init();
-
-    qemu_create_cli_devices();
-
-    /* initialize displays after all errors have been reported */
-    if (!preconfig_requested) {
-        qemu_init_displays();
-    }
-    qemu_machine_creation_done();
-
-    if (loadvm) {
-        Error *local_err = NULL;
-        if (load_snapshot(loadvm, &local_err) < 0) {
-            error_report_err(local_err);
-            autostart = 0;
-            exit(1);
-        }
-    }
-    if (replay_mode != REPLAY_MODE_NONE) {
-        replay_vmstate_init();
-    }
-
     if (vmstate_dump_file) {
         /* dump and exit */
         dump_vmstate_json_to_file(vmstate_dump_file);
         exit(0);
     }
-    if (incoming) {
-        Error *local_err = NULL;
-        if (strcmp(incoming, "defer") != 0) {
-            qmp_migrate_incoming(incoming, &local_err);
-            if (local_err) {
-                error_reportf_err(local_err, "-incoming %s: ", incoming);
-                exit(1);
-            }
-        }
-    } else if (autostart) {
-        qmp_cont(NULL);
-    }
 
+    if (!preconfig_requested) {
+        qmp_x_exit_preconfig(&error_fatal);
+    }
+    qemu_init_displays();
     accel_setup_post(current_machine);
     os_setup_post();
-
-    return;
 }
 
 void qemu_cleanup(void)