Patchwork vl: convert -smp to qemu_opts_parse()

login
register
mail settings
Submitter Michael Tokarev
Date June 19, 2013, 7:29 p.m.
Message ID <1371670154-32026-1-git-send-email-mjt@msgid.tls.msk.ru>
Download mbox | patch
Permalink /patch/252644/
State New
Headers show

Comments

Michael Tokarev - June 19, 2013, 7:29 p.m.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 vl.c |  122 +++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 69 insertions(+), 53 deletions(-)
Anthony Liguori - June 19, 2013, 7:35 p.m.
Michael Tokarev <mjt@tls.msk.ru> writes:

> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>  vl.c |  122 +++++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 69 insertions(+), 53 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index f94ec9c..530fa8f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1401,48 +1401,79 @@ static void numa_add(const char *optarg)
>      }
>  }
>  
> -static void smp_parse(const char *optarg)
> +static QemuOptsList qemu_smp_opts = {
> +    .name = "smp-opts",
> +    .implied_opt_name = "cpus",
> +    .merge_lists = true,
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_smp_opts.head),
> +    .desc = {
> +        {
> +            .name = "cpus",
> +            .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "sockets",
> +            .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "cores",
> +            .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "threads",
> +            .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "maxcpus",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /*End of list */ }
> +    },
> +};
> +
> +static void smp_parse(QemuOpts *opts)
>  {
> -    int smp, sockets = 0, threads = 0, cores = 0;
> -    char *endptr;
> -    char option[128];
> +    if (opts) {
>  
> -    smp = strtoul(optarg, &endptr, 10);
> -    if (endptr != optarg) {
> -        if (*endptr == ',') {
> -            endptr++;
> -        }
> -    }
> -    if (get_param_value(option, 128, "sockets", endptr) != 0)
> -        sockets = strtoull(option, NULL, 10);
> -    if (get_param_value(option, 128, "cores", endptr) != 0)
> -        cores = strtoull(option, NULL, 10);
> -    if (get_param_value(option, 128, "threads", endptr) != 0)
> -        threads = strtoull(option, NULL, 10);
> -    if (get_param_value(option, 128, "maxcpus", endptr) != 0)
> -        max_cpus = strtoull(option, NULL, 10);
> -
> -    /* compute missing values, prefer sockets over cores over threads */
> -    if (smp == 0 || sockets == 0) {
> -        sockets = sockets > 0 ? sockets : 1;
> -        cores = cores > 0 ? cores : 1;
> -        threads = threads > 0 ? threads : 1;
> -        if (smp == 0) {
> -            smp = cores * threads * sockets;
> -        }
> -    } else {
> -        if (cores == 0) {
> +        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> +        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> +        unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
> +        unsigned threads = qemu_opt_get_number(opts, "threads", 0);
> +
> +        /* compute missing values, prefer sockets over cores over threads */
> +        if (cpus == 0 || sockets == 0) {
> +            sockets = sockets > 0 ? sockets : 1;
> +            cores = cores > 0 ? cores : 1;
>              threads = threads > 0 ? threads : 1;
> -            cores = smp / (sockets * threads);
> +            if (cpus == 0) {
> +                cpus = cores * threads * sockets;
> +            }
>          } else {
> -            threads = smp / (cores * sockets);
> +            if (cores == 0) {
> +                threads = threads > 0 ? threads : 1;
> +                cores = cpus / (sockets * threads);
> +            } else {
> +                threads = cpus / (cores * sockets);
> +            }
>          }
> +
> +        max_cpus = qemu_opt_get_number(opts, "maxcpus", 0);
> +
> +        smp_cpus = cpus;
> +        smp_cores = cores > 0 ? cores : 1;
> +        smp_threads = threads > 0 ? threads : 1;
> +
>      }
> -    smp_cpus = smp;
> -    smp_cores = cores > 0 ? cores : 1;
> -    smp_threads = threads > 0 ? threads : 1;
> -    if (max_cpus == 0)
> +
> +    if (max_cpus == 0) {
>          max_cpus = smp_cpus;
> +    }
> +
> +    if (max_cpus > 255) {
> +        fprintf(stderr, "Unsupported number of maxcpus\n");
> +        exit(1);
> +    }
> +    if (max_cpus < smp_cpus) {
> +        fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
> +        exit(1);
> +    }
> +
>  }
>  
>  static void configure_realtime(QemuOpts *opts)
> @@ -2895,6 +2926,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_trace_opts);
>      qemu_add_opts(&qemu_option_rom_opts);
>      qemu_add_opts(&qemu_machine_opts);
> +    qemu_add_opts(&qemu_smp_opts);
>      qemu_add_opts(&qemu_boot_opts);
>      qemu_add_opts(&qemu_sandbox_opts);
>      qemu_add_opts(&qemu_add_fd_opts);
> @@ -3624,18 +3656,7 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>              case QEMU_OPTION_smp:
> -                smp_parse(optarg);
> -                if (smp_cpus < 1) {
> -                    fprintf(stderr, "Invalid number of CPUs\n");
> -                    exit(1);
> -                }
> -                if (max_cpus < smp_cpus) {
> -                    fprintf(stderr, "maxcpus must be equal to or greater than "
> -                            "smp\n");
> -                    exit(1);
> -                }
> -                if (max_cpus > 255) {
> -                    fprintf(stderr, "Unsupported number of maxcpus\n");
> +                if (!qemu_opts_parse(qemu_find_opts("smp-opts"), optarg, 1)) {
>                      exit(1);
>                  }
>                  break;
> @@ -3959,12 +3980,7 @@ int main(int argc, char **argv, char **envp)
>          data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
>      }
>  
> -    /*
> -     * Default to max_cpus = smp_cpus, in case the user doesn't
> -     * specify a max_cpus value.
> -     */
> -    if (!max_cpus)
> -        max_cpus = smp_cpus;
> +    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
>  
>      machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
>      if (smp_cpus > machine->max_cpus) {
> -- 
> 1.7.10.4
Michael Tokarev - June 28, 2013, 10:39 a.m.
19.06.2013 23:35, Anthony Liguori wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
> 
>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Thank you for the review.  Ok to apply it to the
trivial tree, since the change is quite simple?

Thanks,

/mjt
Peter Maydell - June 28, 2013, 11:27 a.m.
On 28 June 2013 11:39, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 19.06.2013 23:35, Anthony Liguori wrote:
>> Michael Tokarev <mjt@tls.msk.ru> writes:
>>
>>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>>
>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>
> Thank you for the review.  Ok to apply it to the
> trivial tree, since the change is quite simple?

I don't think it's trivial, personally.

thanks
-- PMM
Andreas Färber - June 28, 2013, 11:51 a.m.
Am 28.06.2013 13:27, schrieb Peter Maydell:
> On 28 June 2013 11:39, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 19.06.2013 23:35, Anthony Liguori wrote:
>>> Michael Tokarev <mjt@tls.msk.ru> writes:
>>>
>>>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>>>
>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Thank you for the review.  Ok to apply it to the
>> trivial tree, since the change is quite simple?
> 
> I don't think it's trivial, personally.

In that case as CPU maintainer I'll give it a try - it relates to and/or
conflicts with a proposal from Jason anyway.

A tricky topic - so far I only came up with an easy way to limit CPUs to
-smp, not to -smp plus Nx -device. I'm wondering if it makes more sense
that way - -smp at initialization time and maxcpus for hotplug.

Cheers,
Andreas
Anthony Liguori - July 1, 2013, 1:14 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/vl.c b/vl.c
index f94ec9c..530fa8f 100644
--- a/vl.c
+++ b/vl.c
@@ -1401,48 +1401,79 @@  static void numa_add(const char *optarg)
     }
 }
 
-static void smp_parse(const char *optarg)
+static QemuOptsList qemu_smp_opts = {
+    .name = "smp-opts",
+    .implied_opt_name = "cpus",
+    .merge_lists = true,
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_smp_opts.head),
+    .desc = {
+        {
+            .name = "cpus",
+            .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "sockets",
+            .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "cores",
+            .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "threads",
+            .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "maxcpus",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /*End of list */ }
+    },
+};
+
+static void smp_parse(QemuOpts *opts)
 {
-    int smp, sockets = 0, threads = 0, cores = 0;
-    char *endptr;
-    char option[128];
+    if (opts) {
 
-    smp = strtoul(optarg, &endptr, 10);
-    if (endptr != optarg) {
-        if (*endptr == ',') {
-            endptr++;
-        }
-    }
-    if (get_param_value(option, 128, "sockets", endptr) != 0)
-        sockets = strtoull(option, NULL, 10);
-    if (get_param_value(option, 128, "cores", endptr) != 0)
-        cores = strtoull(option, NULL, 10);
-    if (get_param_value(option, 128, "threads", endptr) != 0)
-        threads = strtoull(option, NULL, 10);
-    if (get_param_value(option, 128, "maxcpus", endptr) != 0)
-        max_cpus = strtoull(option, NULL, 10);
-
-    /* compute missing values, prefer sockets over cores over threads */
-    if (smp == 0 || sockets == 0) {
-        sockets = sockets > 0 ? sockets : 1;
-        cores = cores > 0 ? cores : 1;
-        threads = threads > 0 ? threads : 1;
-        if (smp == 0) {
-            smp = cores * threads * sockets;
-        }
-    } else {
-        if (cores == 0) {
+        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+        unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
+        unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+
+        /* compute missing values, prefer sockets over cores over threads */
+        if (cpus == 0 || sockets == 0) {
+            sockets = sockets > 0 ? sockets : 1;
+            cores = cores > 0 ? cores : 1;
             threads = threads > 0 ? threads : 1;
-            cores = smp / (sockets * threads);
+            if (cpus == 0) {
+                cpus = cores * threads * sockets;
+            }
         } else {
-            threads = smp / (cores * sockets);
+            if (cores == 0) {
+                threads = threads > 0 ? threads : 1;
+                cores = cpus / (sockets * threads);
+            } else {
+                threads = cpus / (cores * sockets);
+            }
         }
+
+        max_cpus = qemu_opt_get_number(opts, "maxcpus", 0);
+
+        smp_cpus = cpus;
+        smp_cores = cores > 0 ? cores : 1;
+        smp_threads = threads > 0 ? threads : 1;
+
     }
-    smp_cpus = smp;
-    smp_cores = cores > 0 ? cores : 1;
-    smp_threads = threads > 0 ? threads : 1;
-    if (max_cpus == 0)
+
+    if (max_cpus == 0) {
         max_cpus = smp_cpus;
+    }
+
+    if (max_cpus > 255) {
+        fprintf(stderr, "Unsupported number of maxcpus\n");
+        exit(1);
+    }
+    if (max_cpus < smp_cpus) {
+        fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
+        exit(1);
+    }
+
 }
 
 static void configure_realtime(QemuOpts *opts)
@@ -2895,6 +2926,7 @@  int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_trace_opts);
     qemu_add_opts(&qemu_option_rom_opts);
     qemu_add_opts(&qemu_machine_opts);
+    qemu_add_opts(&qemu_smp_opts);
     qemu_add_opts(&qemu_boot_opts);
     qemu_add_opts(&qemu_sandbox_opts);
     qemu_add_opts(&qemu_add_fd_opts);
@@ -3624,18 +3656,7 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_smp:
-                smp_parse(optarg);
-                if (smp_cpus < 1) {
-                    fprintf(stderr, "Invalid number of CPUs\n");
-                    exit(1);
-                }
-                if (max_cpus < smp_cpus) {
-                    fprintf(stderr, "maxcpus must be equal to or greater than "
-                            "smp\n");
-                    exit(1);
-                }
-                if (max_cpus > 255) {
-                    fprintf(stderr, "Unsupported number of maxcpus\n");
+                if (!qemu_opts_parse(qemu_find_opts("smp-opts"), optarg, 1)) {
                     exit(1);
                 }
                 break;
@@ -3959,12 +3980,7 @@  int main(int argc, char **argv, char **envp)
         data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
     }
 
-    /*
-     * Default to max_cpus = smp_cpus, in case the user doesn't
-     * specify a max_cpus value.
-     */
-    if (!max_cpus)
-        max_cpus = smp_cpus;
+    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
     machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
     if (smp_cpus > machine->max_cpus) {