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 Accepted
Commit 4d2c17b0ef760881485a3d31f941117d9fc71bd8
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
<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Il mer 11 nov 2020, 20:57 Igor Mammedov &lt;<a href="mailto:imammedo@redhat.com">imammedo@redhat.com</a>&gt; ha scritto:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, 27 Oct 2020 14:21:26 -0400<br>
Paolo Bonzini &lt;<a href="mailto:pbonzini@redhat.com" target="_blank" rel="noreferrer">pbonzini@redhat.com</a>&gt; wrote:<br>
<br>
&gt; Signed-off-by: Paolo Bonzini &lt;<a href="mailto:pbonzini@redhat.com" target="_blank" rel="noreferrer">pbonzini@redhat.com</a>&gt;<br>
&gt; ---<br>
&gt;  softmmu/vl.c | 40 ++++++++++++++++++++++------------------<br>
&gt;  1 file changed, 22 insertions(+), 18 deletions(-)<br>
&gt; <br>
&gt; diff --git a/softmmu/vl.c b/softmmu/vl.c<br>
&gt; index c2a5ee61f9..6749109b29 100644<br>
&gt; --- a/softmmu/vl.c<br>
&gt; +++ b/softmmu/vl.c<br>
&gt; @@ -126,6 +126,7 @@ static const char *boot_once;<br>
&gt;  static const char *incoming;<br>
&gt;  static const char *loadvm;<br>
&gt;  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;<br>
<br>
&gt; +int mem_prealloc; /* force preallocation of physical target memory */<br>
Is there a reason for it not being static?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I will check if I am using it later in the series, but I don&#39;t think so.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
&gt;  int display_opengl;<br>
&gt;  const char* keyboard_layout = NULL;<br>
&gt;  ram_addr_t ram_size;<br>
&gt; @@ -159,7 +160,7 @@ int fd_bootchk = 1;<br>
&gt;  static int no_reboot;<br>
&gt;  int no_shutdown = 0;<br>
&gt;  int graphic_rotate = 0;<br>
&gt; -const char *watchdog;<br>
&gt; +static const char *watchdog;<br>
&gt;  QEMUOptionRom option_rom[MAX_OPTION_ROMS];<br>
&gt;  int nb_option_roms;<br>
&gt;  int old_param = 0;<br>
&gt; @@ -2910,6 +2911,25 @@ static void qemu_validate_options(void)<br>
&gt;  #endif<br>
&gt;  }<br>
&gt;  <br>
&gt; +static void qemu_process_sugar_options(void)<br>
&gt; +{<br>
&gt; +    if (mem_prealloc) {<br>
&gt; +        char *val;<br>
&gt; +<br>
&gt; +        val = g_strdup_printf(&quot;%d&quot;,<br>
&gt; +                 (uint32_t) qemu_opt_get_number(qemu_find_opts_singleton(&quot;smp-opts&quot;), &quot;cpus&quot;, 1));<br>
if -smp isn&#39;t present it value used here was mc-&gt;default_cpus,<br>
which in most cases is 1 modulo some ARM boards and riscv.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Yes, I remember noticing that but decided I would not care. I should have added it to the commit message, though.</div><div dir="auto"><br></div><div dir="auto">Paolo</div></div>
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. */