Re: [PATCH 08/29] vl: move prelaunch part of qemu_init to new functions

[Igor Mammedov]

On Tue, 27 Oct 2020 14:21:23 -0400
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The final part of qemu_init, starting with the completion of
> board init, is already relatively clean.  Split it out of
> qemu_init so that qemu_init keeps only the messy parts.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  softmmu/vl.c | 250 +++++++++++++++++++++++++++------------------------
>  1 file changed, 135 insertions(+), 115 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index a086cd7066..f9bae50c27 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -120,6 +120,9 @@
>  static const char *cpu_option;
>  static const char *data_dir[16];
>  static int data_dir_idx;
> +static const char *mem_path;
> +static const char *boot_order;
> +static const char *boot_once;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
>  int display_opengl;
>  const char* keyboard_layout = NULL;
> @@ -3000,6 +3003,135 @@ static void qemu_init_subsystems(void)
>      socket_init();
>  }
>  
> +/*
> + * Called after leaving preconfig state.  From here on runstate is
> + * RUN_STATE_PRELAUNCH or RUN_STATE_INMIGRATE.
> + */
> +static void qemu_finish_machine_init(void)
> +{
> +    MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
> +
> +    if (machine_class->default_ram_id && current_machine->ram_size &&
> +        numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
> +        create_default_memdev(current_machine, mem_path);
> +    }
> +
> +    /* from here on runstate is RUN_STATE_PRELAUNCH */
> +    machine_run_board_init(current_machine);
> +
> +    /*
> +     * TODO To drop support for deprecated bogus if=..., move
> +     * drive_check_orphaned() here, replacing this call.  Also drop
> +     * its deprecation warning, along with DriveInfo member
> +     * @claimed_by_board.
> +     */
> +    drive_mark_claimed_by_board();
> +
> +    realtime_init();
> +
> +    if (hax_enabled()) {
> +        /* FIXME: why isn't cpu_synchronize_all_post_init enough? */
> +        hax_sync_vcpus();
> +    }
> +}
> +
> +static void qemu_create_cli_devices(void)
> +{
> +    soundhw_init();
> +
> +    qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> +                      parse_fw_cfg, fw_cfg_find(), &error_fatal);
> +
> +    /* init USB devices */
> +    if (machine_usb(current_machine)) {
> +        if (foreach_device_config(DEV_USB, usb_parse) < 0)
> +            exit(1);
> +    }
> +
> +    /* init generic devices */
> +    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
> +    qemu_opts_foreach(qemu_find_opts("device"),
> +                      device_init_func, NULL, &error_fatal);
> +    rom_reset_order_override();
> +}
> +
> +static void qemu_machine_creation_done(void)
> +{
> +    DisplayState *ds;
> +
> +    cpu_synchronize_all_post_init();
> +
> +    /* Did we create any drives that we failed to create a device for? */
> +    drive_check_orphaned();
> +
> +    /* Don't warn about the default network setup that you get if
> +     * no command line -net or -netdev options are specified. There
> +     * are two cases that we would otherwise complain about:
> +     * (1) board doesn't support a NIC but the implicit "-net nic"
> +     * requested one
> +     * (2) CONFIG_SLIRP not set, in which case the implicit "-net nic"
> +     * sets up a nic that isn't connected to anything.
> +     */
> +    if (!default_net && (!qtest_enabled() || has_defaults)) {
> +        net_check_clients();
> +    }
> +
> +    if (boot_once) {
> +        qemu_boot_set(boot_once, &error_fatal);
> +        qemu_register_reset(restore_boot_order, g_strdup(boot_order));
> +    }
> +
> +    /* init local displays */
> +    ds = init_displaystate();
> +    qemu_display_init(ds, &dpy);
> +
> +    /* must be after terminal init, SDL library changes signal handlers */
> +    os_setup_signal_handling();
> +
> +    /* init remote displays */
> +#ifdef CONFIG_VNC
> +    qemu_opts_foreach(qemu_find_opts("vnc"),
> +                      vnc_init_func, NULL, &error_fatal);
> +#endif
> +
> +    if (using_spice) {
> +        qemu_spice.display_init();
> +    }
> +
> +    if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
> +        exit(1);
> +    }
> +
> +    qdev_machine_creation_done();
> +
> +    /* TODO: once all bus devices are qdevified, this should be done
> +     * when bus is created by qdev.c */
> +    /*
> +     * TODO: If we had a main 'reset container' that the whole system
> +     * lived in, we could reset that using the multi-phase reset
> +     * APIs. For the moment, we just reset the sysbus, which will cause
> +     * all devices hanging off it (and all their child buses, recursively)
> +     * to be reset. Note that this will *not* reset any Device objects
> +     * which are not attached to some part of the qbus tree!
> +     */
> +    qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default());
> +    qemu_run_machine_init_done_notifiers();
> +
> +    if (rom_check_and_register_reset() != 0) {
> +        error_report("rom check and register reset failed");
> +        exit(1);
> +    }
> +
> +    replay_start();
> +
> +    /* This checkpoint is required by replay to separate prior clock
> +       reading from the other reads, because timer polling functions query
> +       clock values from the log. */
> +    replay_checkpoint(CHECKPOINT_RESET);
> +    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
> +    register_global_state();
> +}
> +
>  void qemu_init(int argc, char **argv, char **envp)
>  {
>      int i;
> @@ -3007,9 +3139,6 @@ void qemu_init(int argc, char **argv, char **envp)
>      int linux_boot;
>      const char *initrd_filename;
>      const char *kernel_filename, *kernel_cmdline;
> -    const char *boot_order = NULL;
> -    const char *boot_once = NULL;
> -    DisplayState *ds;
>      QemuOpts *opts, *machine_opts;
>      QemuOpts *icount_opts = NULL, *accel_opts = NULL;
>      QemuOptsList *olist;
> @@ -3026,7 +3155,6 @@ void qemu_init(int argc, char **argv, char **envp)
>      uint64_t ram_slots = 0;
>      FILE *vmstate_dump_file = NULL;
>      Error *err = NULL;
> -    const char *mem_path = NULL;
>      bool have_custom_ram_size;
>      BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
>      QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list);
> @@ -4346,117 +4474,10 @@ void qemu_init(int argc, char **argv, char **envp)
>      /* do monitor/qmp handling at preconfig state if requested */
>      qemu_main_loop();
>  
> -    if (machine_class->default_ram_id && current_machine->ram_size &&
> -        numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
> -        create_default_memdev(current_machine, mem_path);
> -    }
> -
> -    /* from here on runstate is RUN_STATE_PRELAUNCH */
> -    machine_run_board_init(current_machine);
> -
> -    /*
> -     * TODO To drop support for deprecated bogus if=..., move
> -     * drive_check_orphaned() here, replacing this call.  Also drop
> -     * its deprecation warning, along with DriveInfo member
> -     * @claimed_by_board.
> -     */
> -    drive_mark_claimed_by_board();
> -
> -    realtime_init();
> +    qemu_finish_machine_init();
> +    qemu_create_cli_devices();
> +    qemu_machine_creation_done();
>  
> -    soundhw_init();
> -
> -    if (hax_enabled()) {
> -        hax_sync_vcpus();
> -    }
> -
> -    qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> -                      parse_fw_cfg, fw_cfg_find(), &error_fatal);
> -
> -    /* init USB devices */
> -    if (machine_usb(current_machine)) {
> -        if (foreach_device_config(DEV_USB, usb_parse) < 0)
> -            exit(1);
> -    }
> -
> -    /* init generic devices */
> -    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
> -    qemu_opts_foreach(qemu_find_opts("device"),
> -                      device_init_func, NULL, &error_fatal);
> -
> -    cpu_synchronize_all_post_init();
> -
> -    rom_reset_order_override();
> -
> -    /* Did we create any drives that we failed to create a device for? */
> -    drive_check_orphaned();
> -
> -    /* Don't warn about the default network setup that you get if
> -     * no command line -net or -netdev options are specified. There
> -     * are two cases that we would otherwise complain about:
> -     * (1) board doesn't support a NIC but the implicit "-net nic"
> -     * requested one
> -     * (2) CONFIG_SLIRP not set, in which case the implicit "-net nic"
> -     * sets up a nic that isn't connected to anything.
> -     */
> -    if (!default_net && (!qtest_enabled() || has_defaults)) {
> -        net_check_clients();
> -    }
> -
> -    if (boot_once) {
> -        qemu_boot_set(boot_once, &error_fatal);
> -        qemu_register_reset(restore_boot_order, g_strdup(boot_order));
> -    }
> -
> -    /* init local displays */
> -    ds = init_displaystate();
> -    qemu_display_init(ds, &dpy);
> -
> -    /* must be after terminal init, SDL library changes signal handlers */
> -    os_setup_signal_handling();
> -
> -    /* init remote displays */
> -#ifdef CONFIG_VNC
> -    qemu_opts_foreach(qemu_find_opts("vnc"),
> -                      vnc_init_func, NULL, &error_fatal);
> -#endif
> -
> -    if (using_spice) {
> -        qemu_spice.display_init();
> -    }
> -
> -    if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
> -        exit(1);
> -    }
> -
> -    qdev_machine_creation_done();
> -
> -    /* TODO: once all bus devices are qdevified, this should be done
> -     * when bus is created by qdev.c */
> -    /*
> -     * TODO: If we had a main 'reset container' that the whole system
> -     * lived in, we could reset that using the multi-phase reset
> -     * APIs. For the moment, we just reset the sysbus, which will cause
> -     * all devices hanging off it (and all their child buses, recursively)
> -     * to be reset. Note that this will *not* reset any Device objects
> -     * which are not attached to some part of the qbus tree!
> -     */
> -    qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default());
> -    qemu_run_machine_init_done_notifiers();
> -
> -    if (rom_check_and_register_reset() != 0) {
> -        error_report("rom check and register reset failed");
> -        exit(1);
> -    }
> -
> -    replay_start();
> -
> -    /* This checkpoint is required by replay to separate prior clock
> -       reading from the other reads, because timer polling functions query
> -       clock values from the log. */
> -    replay_checkpoint(CHECKPOINT_RESET);
> -    qemu_system_reset(SHUTDOWN_CAUSE_NONE);
> -    register_global_state();
>      if (loadvm) {
>          Error *local_err = NULL;
>          if (load_snapshot(loadvm, &local_err) < 0) {
> @@ -4475,7 +4496,6 @@ void qemu_init(int argc, char **argv, char **envp)
>          dump_vmstate_json_to_file(vmstate_dump_file);
>          exit(0);
>      }
> -
>      if (incoming) {
>          Error *local_err = NULL;
>          qemu_start_incoming_migration(incoming, &local_err);