On Wed, May 27, 2020 at 11:12:22AM +0530, Anup Patel wrote: > When "sockets" sub-option of "-smp" option is not specified, the > smp_parse() function will assume one CPU per-socket and set the > number of sockets equal to number of CPUs. > > This is counter-intuitive and we should allow machine emulation to > decide default number of sockets when "sockets" sub-option is not > specified.
I don't agree with this. Having the semantics of the -smp option be the same across all targets/machines *is* intuitive. Changing semantics of -smp per-machine will create a worse experiance for people configuring QEMU as the configuration will mean different things depending on the machine choce. > To achieve this, we add boolean flag sockets_specified > in struct CpuTopology which tells machine emulation whether the > "sockets" sub-option was specified in command-line. > > Signed-off-by: Anup Patel <anup.pa...@wdc.com> > --- > hw/core/machine.c | 2 ++ > include/hw/boards.h | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index bb3a7b18b1..fd5ef5a4bb 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -706,6 +706,8 @@ static void smp_parse(MachineState *ms, QemuOpts *opts) > unsigned cores = qemu_opt_get_number(opts, "cores", 0); > unsigned threads = qemu_opt_get_number(opts, "threads", 0); > > + ms->smp.sockets_specified = (sockets == 0) ? false : true; > + > /* compute missing values, prefer sockets over cores over threads */ > if (cpus == 0 || sockets == 0) { > cores = cores > 0 ? cores : 1; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 18815d9be2..59b28ada65 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -244,6 +244,7 @@ typedef struct DeviceMemoryState { > * @cores: the number of cores in one package > * @threads: the number of threads in one core > * @sockets: the number of sockets on the machine > + * @sockets_specified: the number of sockets were specified for the machine > * @max_cpus: the maximum number of logical processors on the machine > */ > typedef struct CpuTopology { > @@ -251,6 +252,7 @@ typedef struct CpuTopology { > unsigned int cores; > unsigned int threads; > unsigned int sockets; > + bool sockets_specified; > unsigned int max_cpus; > } CpuTopology; > > -- > 2.25.1 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|