Re: [PATCH 11/29] vl: extract various command line desugaring snippets to a new function

[Igor Mammedov <imammedo@redhat.com>]

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

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  softmmu/vl.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index c2a5ee61f9..6749109b29 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -126,6 +126,7 @@ static const char *boot_once;
>  static const char *incoming;
>  static const char *loadvm;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;

> +int mem_prealloc; /* force preallocation of physical target memory */
Is there a reason for it not being static?

>  int display_opengl;
>  const char* keyboard_layout = NULL;
>  ram_addr_t ram_size;
> @@ -159,7 +160,7 @@ int fd_bootchk = 1;
>  static int no_reboot;
>  int no_shutdown = 0;
>  int graphic_rotate = 0;
> -const char *watchdog;
> +static const char *watchdog;
>  QEMUOptionRom option_rom[MAX_OPTION_ROMS];
>  int nb_option_roms;
>  int old_param = 0;
> @@ -2910,6 +2911,25 @@ static void qemu_validate_options(void)
>  #endif
>  }
>  
> +static void qemu_process_sugar_options(void)
> +{
> +    if (mem_prealloc) {
> +        char *val;
> +
> +        val = g_strdup_printf("%d",
> +                 (uint32_t) qemu_opt_get_number(qemu_find_opts_singleton("smp-opts"), "cpus", 1));
if -smp isn't present it value used here was mc->default_cpus,
which in most cases is 1 modulo some ARM boards and riscv.

But we probably don't care much how this heuristic is picked up for default_cpus,
is users really care about how many treads QEMU spawns for preallocating RAM,
they should use explicit -object memory-backend-foo,prealloc-threads=X explicitly


> +        object_register_sugar_prop("memory-backend", "prealloc-threads", val);
> +        g_free(val);
> +        object_register_sugar_prop("memory-backend", "prealloc", "on");
> +    }
> +
> +    if (watchdog) {
> +        int i = select_watchdog(watchdog);
> +        if (i > 0)
> +            exit (i == 1 ? 1 : 0);
> +    }
> +}
> +
>  static void qemu_process_early_options(void)
>  {
>      char **dirs;
> @@ -3174,7 +3194,6 @@ static void qemu_machine_creation_done(void)
>  
>  void qemu_init(int argc, char **argv, char **envp)
>  {
> -    int i;
>      int snapshot = 0;
>      QemuOpts *opts, *machine_opts;
>      QemuOpts *icount_opts = NULL, *accel_opts = NULL;
> @@ -3193,7 +3212,6 @@ void qemu_init(int argc, char **argv, char **envp)
>      bool have_custom_ram_size;
>      BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
>      QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list);
> -    int mem_prealloc = 0; /* force preallocation of physical target memory */
>  
>      qemu_add_opts(&qemu_drive_opts);
>      qemu_add_drive_opts(&qemu_legacy_drive_opts);
> @@ -4104,6 +4122,7 @@ void qemu_init(int argc, char **argv, char **envp)
>      loc_set_none();
>  
>      qemu_validate_options();
> +    qemu_process_sugar_options();
>  
>      /* These options affect everything else and should be processed
>       * before daemonizing.
> @@ -4155,15 +4174,6 @@ void qemu_init(int argc, char **argv, char **envp)
>      machine_smp_parse(current_machine,
>          qemu_opts_find(qemu_find_opts("smp-opts"), NULL), &error_fatal);
>  
> -    if (mem_prealloc) {
> -        char *val;
> -
> -        val = g_strdup_printf("%d", current_machine->smp.cpus);
> -        object_register_sugar_prop("memory-backend", "prealloc-threads", val);
> -        g_free(val);
> -        object_register_sugar_prop("memory-backend", "prealloc", "on");
> -    }
> -
>      /*
>       * Get the default machine options from the machine if it is not already
>       * specified either by the configuration file or by the command line.
> @@ -4422,12 +4432,6 @@ void qemu_init(int argc, char **argv, char **envp)
>          select_vgahw(machine_class, vga_model);
>      }
>  
> -    if (watchdog) {
> -        i = select_watchdog(watchdog);
> -        if (i > 0)
> -            exit (i == 1 ? 1 : 0);
> -    }
> -
>      /* 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. */