From: Igor Mammedov <imammedo@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: yang.zhong@intel.com, armbru@redhat.com, berrange@redhat.com,
qemu-devel@nongnu.org, ehabkost@redhat.com
Subject: Re: [PATCH 14/14] machine: add smp compound property
Date: Mon, 17 May 2021 16:58:11 +0200 [thread overview]
Message-ID: <20210517165811.20ca1411@redhat.com> (raw)
In-Reply-To: <20210513162901.1310239-15-pbonzini@redhat.com>
On Thu, 13 May 2021 12:29:01 -0400
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Make -smp syntactic sugar for a compound property "-machine
> smp.{cores,threads,cpu,...}". machine_smp_parse is replaced by the
> setter for the property.
>
> numa-test will now cover the new syntax, while other tests
> still use -smp.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/core/machine.c | 108 +++++++++++++++++++++-------------------
> softmmu/vl.c | 33 +++++++++---
> tests/qtest/numa-test.c | 22 ++++----
> 3 files changed, 95 insertions(+), 68 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 55e878fc3e..f33c9ce78c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -19,6 +19,7 @@
> #include "hw/loader.h"
> #include "qapi/error.h"
> #include "qapi/qapi-visit-common.h"
> +#include "qapi/qapi-visit-machine.h"
> #include "qapi/visitor.h"
> #include "hw/sysbus.h"
> #include "sysemu/cpus.h"
> @@ -797,6 +798,57 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> ms->smp.sockets = sockets;
> }
>
> +static void machine_get_smp(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + MachineState *ms = MACHINE(obj);
> + SMPConfiguration *config = &(SMPConfiguration){
> + .has_cores = true, .cores = ms->smp.cores,
> + .has_sockets = true, .sockets = ms->smp.sockets,
> + .has_dies = true, .dies = ms->smp.dies,
> + .has_threads = true, .threads = ms->smp.threads,
> + .has_cpus = true, .cpus = ms->smp.cpus,
> + .has_maxcpus = true, .maxcpus = ms->smp.max_cpus,
> + };
why did you choose to set all has_foo to true?
> + if (!visit_type_SMPConfiguration(v, name, &config, &error_abort)) {
> + return;
> + }
> +}
> +
> +static void machine_set_smp(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + MachineState *ms = MACHINE(obj);
> + SMPConfiguration *config;
> + ERRP_GUARD();
> +
> + if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
> + return;
> + }
> +
> + mc->smp_parse(ms, config, errp);
> + if (errp) {
> + goto out_free;
> + }
> +
> + /* sanity-check smp_cpus and max_cpus against mc */
> + if (ms->smp.cpus < mc->min_cpus) {
> + error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
> + "supported by machine '%s' is %d",
> + ms->smp.cpus,
> + mc->name, mc->min_cpus);
> + } else if (ms->smp.max_cpus > mc->max_cpus) {
> + error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
> + "supported by machine '%s' is %d",
> + current_machine->smp.max_cpus,
> + mc->name, mc->max_cpus);
> + }
> +
> +out_free:
> + qapi_free_SMPConfiguration(config);
> +}
> +
> static void machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -836,6 +888,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
> object_class_property_set_description(oc, "dumpdtb",
> "Dump current dtb to a file and quit");
>
> + object_class_property_add(oc, "smp", "SMPConfiguration",
> + machine_get_smp, machine_set_smp,
> + NULL, NULL);
> + object_class_property_set_description(oc, "smp",
> + "CPU topology");
> +
> object_class_property_add(oc, "phandle-start", "int",
> machine_get_phandle_start, machine_set_phandle_start,
> NULL, NULL);
> @@ -1124,56 +1182,6 @@ MemoryRegion *machine_consume_memdev(MachineState *machine,
> return ret;
> }
>
> -bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
> -{
> - MachineClass *mc = MACHINE_GET_CLASS(ms);
> - ERRP_GUARD();
> -
> - if (opts) {
> - SMPConfiguration config = {
> - .has_cpus = !!qemu_opt_get(opts, "cpus"),
> - .cpus = qemu_opt_get_number(opts, "cpus", 0),
> - .has_sockets = !!qemu_opt_get(opts, "sockets"),
> - .sockets = qemu_opt_get_number(opts, "sockets", 0),
> - .has_dies = !!qemu_opt_get(opts, "dies"),
> - .dies = qemu_opt_get_number(opts, "dies", 0),
> - .has_cores = !!qemu_opt_get(opts, "cores"),
> - .cores = qemu_opt_get_number(opts, "cores", 0),
> - .has_threads = !!qemu_opt_get(opts, "threads"),
> - .threads = qemu_opt_get_number(opts, "threads", 0),
> - .has_maxcpus = !!qemu_opt_get(opts, "maxcpus"),
> - .maxcpus = qemu_opt_get_number(opts, "maxcpus", 0),
> - };
> -
> - mc->smp_parse(ms, &config, errp);
> - if (*errp) {
> - return false;
> - }
> - }
> -
> - /* sanity-check smp_cpus and max_cpus against mc */
> - if (ms->smp.cpus < mc->min_cpus) {
> - error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
> - "supported by machine '%s' is %d",
> - ms->smp.cpus,
> - mc->name, mc->min_cpus);
> - return false;
> - } else if (ms->smp.max_cpus > mc->max_cpus) {
> - error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
> - "supported by machine '%s' is %d",
> - current_machine->smp.max_cpus,
> - mc->name, mc->max_cpus);
> - return false;
> - }
> -
> - if (ms->smp.cpus > 1) {
> - Error *blocker = NULL;
> - error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> - replay_add_blocker(blocker);
> - }
> - return true;
> -}
> -
> void machine_run_board_init(MachineState *machine)
> {
> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index de844f08d7..555385e64d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1504,6 +1504,25 @@ static void machine_help_func(const QDict *qdict)
> }
> }
>
> +static void
> +machine_parse_property_opt(QemuOptsList *opts_list, const char *propname,
> + const char *arg, Error **errp)
> +{
> + QDict *opts, *prop;
> + bool help = false;
> + ERRP_GUARD();
> +
> + prop = keyval_parse(arg, opts_list->implied_opt_name, &help, errp);
> + if (help) {
> + qemu_opts_print_help(opts_list, true);
> + return;
> + }
> + opts = qdict_new();
> + qdict_put(opts, propname, prop);
> + keyval_merge(machine_opts_dict, opts, errp);
> + qobject_unref(opts);
> +}
> +
> static const char *pid_file;
> static Notifier qemu_unlink_pidfile_notifier;
>
> @@ -1796,6 +1815,12 @@ static void qemu_apply_machine_options(QDict *qdict)
> /* fall back to the -kernel/-append */
> semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline);
> }
> +
> + if (current_machine->smp.cpus > 1) {
> + Error *blocker = NULL;
> + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> + replay_add_blocker(blocker);
> + }
> }
>
> static void qemu_create_early_backends(void)
> @@ -2039,9 +2064,6 @@ static void qemu_create_machine(QDict *qdict)
> qemu_set_hw_version(machine_class->hw_version);
> }
>
> - machine_smp_parse(current_machine,
> - qemu_opts_find(qemu_find_opts("smp-opts"), NULL), &error_fatal);
> -
> /*
> * Get the default machine options from the machine if it is not already
> * specified either by the configuration file or by the command line.
> @@ -3325,10 +3347,7 @@ void qemu_init(int argc, char **argv, char **envp)
> }
> break;
> case QEMU_OPTION_smp:
> - if (!qemu_opts_parse_noisily(qemu_find_opts("smp-opts"),
> - optarg, true)) {
> - exit(1);
> - }
> + machine_parse_property_opt(qemu_find_opts("smp-opts"), "smp", optarg, &error_fatal);
> break;
> case QEMU_OPTION_vnc:
> vnc_parse(optarg);
> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
> index dc0ec571ca..c677cd63c4 100644
> --- a/tests/qtest/numa-test.c
> +++ b/tests/qtest/numa-test.c
> @@ -25,7 +25,7 @@ static void test_mon_explicit(const void *data)
> g_autofree char *s = NULL;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 8 -numa node,nodeid=0,memdev=ram,cpus=0-3 "
> + cli = make_cli(data, "-machine smp.cpus=8 -numa node,nodeid=0,memdev=ram,cpus=0-3 "
> "-numa node,nodeid=1,cpus=4-7");
> qts = qtest_init(cli);
>
> @@ -42,7 +42,7 @@ static void test_def_cpu_split(const void *data)
> g_autofree char *s = NULL;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 8 -numa node,memdev=ram -numa node");
> + cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram -numa node");
> qts = qtest_init(cli);
>
> s = qtest_hmp(qts, "info numa");
> @@ -58,7 +58,7 @@ static void test_mon_partial(const void *data)
> g_autofree char *s = NULL;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 8 "
> + cli = make_cli(data, "-machine smp.cpus=8 "
> "-numa node,nodeid=0,memdev=ram,cpus=0-1 "
> "-numa node,nodeid=1,cpus=4-5 ");
> qts = qtest_init(cli);
> @@ -86,7 +86,7 @@ static void test_query_cpus(const void *data)
> QTestState *qts;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 8 -numa node,memdev=ram,cpus=0-3 "
> + cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram,cpus=0-3 "
> "-numa node,cpus=4-7");
> qts = qtest_init(cli);
> cpus = get_cpus(qts, &resp);
> @@ -124,7 +124,7 @@ static void pc_numa_cpu(const void *data)
> QTestState *qts;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-cpu pentium -smp 8,sockets=2,cores=2,threads=2 "
> + cli = make_cli(data, "-cpu pentium -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
> "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> "-numa cpu,node-id=1,socket-id=0 "
> "-numa cpu,node-id=0,socket-id=1,core-id=0 "
> @@ -177,7 +177,7 @@ static void spapr_numa_cpu(const void *data)
> QTestState *qts;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 4,cores=4 "
> + cli = make_cli(data, "-machine smp.cpus=4,smp.cores=4 "
> "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> "-numa cpu,node-id=0,core-id=0 "
> "-numa cpu,node-id=0,core-id=1 "
> @@ -222,7 +222,7 @@ static void aarch64_numa_cpu(const void *data)
> QTestState *qts;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-smp 2 "
> + cli = make_cli(data, "-machine smp.cpus=2 "
> "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
> "-numa cpu,node-id=1,thread-id=0 "
> "-numa cpu,node-id=0,thread-id=1");
> @@ -265,7 +265,7 @@ static void pc_dynamic_cpu_cfg(const void *data)
> QTestState *qs;
> g_autofree char *cli = NULL;
>
> - cli = make_cli(data, "-nodefaults --preconfig -smp 2");
> + cli = make_cli(data, "-nodefaults --preconfig -machine smp.cpus=2");
> qs = qtest_init(cli);
>
> /* create 2 numa nodes */
> @@ -324,7 +324,7 @@ static void pc_hmat_build_cfg(const void *data)
> g_autofree char *cli = NULL;
>
> cli = make_cli(data, "-nodefaults --preconfig -machine hmat=on "
> - "-smp 2,sockets=2 "
> + "-machine smp.cpus=2,smp.sockets=2 "
> "-m 128M,slots=2,maxmem=1G "
> "-object memory-backend-ram,size=64M,id=m0 "
> "-object memory-backend-ram,size=64M,id=m1 "
> @@ -453,7 +453,7 @@ static void pc_hmat_off_cfg(const void *data)
> g_autofree char *cli = NULL;
>
> cli = make_cli(data, "-nodefaults --preconfig "
> - "-smp 2,sockets=2 "
> + "-machine smp.cpus=2,smp.sockets=2 "
> "-m 128M,slots=2,maxmem=1G "
> "-object memory-backend-ram,size=64M,id=m0,prealloc=y "
> "-object memory-backend-ram,size=64M,id=m1 "
> @@ -492,7 +492,7 @@ static void pc_hmat_erange_cfg(const void *data)
> g_autofree char *cli = NULL;
>
> cli = make_cli(data, "-nodefaults --preconfig -machine hmat=on "
> - "-smp 2,sockets=2 "
> + "-machine smp.cpus=2,smp.sockets=2 "
> "-m 128M,slots=2,maxmem=1G "
> "-object memory-backend-ram,size=64M,id=m0 "
> "-object memory-backend-ram,size=64M,id=m1 "
next prev parent reply other threads:[~2021-05-17 15:03 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
2021-05-13 16:28 ` [PATCH 01/14] remove -writeconfig Paolo Bonzini
2021-05-13 16:28 ` [PATCH 02/14] qemu-config: parse configuration files to a QDict Paolo Bonzini
2021-05-13 16:28 ` [PATCH 03/14] vl: plumb keyval-based options into -set and -readconfig Paolo Bonzini
2021-05-13 16:28 ` [PATCH 04/14] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
2021-05-13 16:28 ` [PATCH 05/14] keyval: introduce keyval_parse_into Paolo Bonzini
2021-05-13 16:28 ` [PATCH 06/14] vl: switch -M parsing to keyval Paolo Bonzini
2021-05-13 16:28 ` [PATCH 07/14] qemu-option: remove now-dead code Paolo Bonzini
2021-05-13 16:28 ` [PATCH 08/14] vl: switch -accel parsing to keyval Paolo Bonzini
2021-05-13 16:28 ` [PATCH 09/14] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
2021-05-13 16:28 ` [PATCH 10/14] machine: move common smp_parse code to caller Paolo Bonzini
2021-05-13 16:28 ` [PATCH 11/14] machine: add error propagation to mc->smp_parse Paolo Bonzini
2021-05-13 16:28 ` [PATCH 12/14] machine: pass QAPI struct " Paolo Bonzini
2021-05-13 16:29 ` [PATCH 13/14] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
2021-05-13 16:29 ` [PATCH 14/14] machine: add smp compound property Paolo Bonzini
2021-05-17 14:58 ` Igor Mammedov [this message]
2021-05-13 16:57 ` [PATCH 00/14] vl: compound properties for machines and accelerators no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210517165811.20ca1411@redhat.com \
--to=imammedo@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=yang.zhong@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.