Philippe Mathieu-Daudé <phi...@redhat.com> writes: > On 9/28/21 05:57, Yanan Wang wrote: >> Currently the only difference between smp_parse and pc_smp_parse >> is the support of dies parameter and the related error reporting. >> With some arch compat variables like "bool dies_supported", we can >> make smp_parse generic enough for all arches and the PC specific >> one can be removed. >> >> Making smp_parse() generic enough can reduce code duplication and >> ease the code maintenance, and also allows extending the topology >> with more arch specific members (e.g., clusters) in the future. >> >> Suggested-by: Andrew Jones <drjo...@redhat.com> >> Suggested-by: Daniel P. Berrange <berra...@redhat.com> >> Signed-off-by: Yanan Wang <wangyana...@huawei.com> >> --- >> hw/core/machine.c | 91 +++++++++++++++++++++++++++++++++++---------- >> hw/i386/pc.c | 84 +---------------------------------------- >> include/hw/boards.h | 9 +++++ >> 3 files changed, 81 insertions(+), 103 deletions(-) > >> +/* >> + * smp_parse - Generic function used to parse the given SMP configuration >> + * >> + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be >> + * automatically computed based on the provided ones. >> + * >> + * In the calculation of omitted sockets/cores/threads: we prefer sockets >> + * over cores over threads before 6.2, while preferring cores over sockets >> + * over threads since 6.2. >> + * >> + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are >> omitted, >> + * maxcpus will be computed from the given parameters and cpus will be set >> + * equal to maxcpus. When only one of maxcpus and cpus is given then the >> + * omitted one will be set to its given counterpart's value. Both maxcpus >> and >> + * cpus may be specified, but maxcpus must be equal to or greater than cpus. >> + * >> + * For compatibility, apart from the parameters that will be computed, newly >> + * introduced topology members which are likely to be target specific should >> + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). >> + */ >> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error >> **errp) > > Can we have it return a boolean instead?
Yes, please. From error.h's big comment: * = Rules = [...] * - Whenever practical, also return a value that indicates success / * failure. This can make the error checking more concise, and can * avoid useless error object creation and destruction. Note that * we still have many functions returning void. We recommend * • bool-valued functions return true on success / false on failure, * • pointer-valued functions return non-null / null pointer, and * • integer-valued functions return non-negative / negative.
