vl: convert -smp to qemu_opts_parse()

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 vl.c |  122 +++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 69 insertions(+), 53 deletions(-)

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

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

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

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

Applied.  Thanks.

Regards,

Anthony Liguori