diff mbox series

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

Message ID 20201027182144.3315885-12-pbonzini@redhat.com
State New
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 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

Comments

Igor Mammedov Nov. 11, 2020, 7:57 p.m. UTC | #1 
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. */
Paolo Bonzini Nov. 11, 2020, 8:04 p.m. UTC | #2 
Il mer 11 nov 2020, 20:57 Igor Mammedov <imammedo@redhat.com> ha scritto:

> 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?
>

I will check if I am using it later in the series, but I don't think so.


> >  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.
>

Yes, I remember noticing that but decided I would not care. I should have
added it to the commit message, though.

Paolo
Igor Mammedov Nov. 18, 2020, 4:55 p.m. UTC | #3 
On Wed, 11 Nov 2020 21:04:53 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il mer 11 nov 2020, 20:57 Igor Mammedov <imammedo@redhat.com> ha scritto:
> 
> > 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?
> >  
> 
> I will check if I am using it later in the series, but I don't think so.
with it fixed to static

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

> 
> 
> > >  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.
> >  
> 
> Yes, I remember noticing that but decided I would not care. I should have
> added it to the commit message, though.
> 
> Paolo
diff mbox series

Patch

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 */
 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));
+        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. */