On Thu, Jul 22 2021, Andrew Jones <drjo...@redhat.com> wrote: > On Thu, Jul 22, 2021 at 08:02:16AM +0200, Cornelia Huck wrote: >> On Thu, Jul 22 2021, Yanan Wang <wangyana...@huawei.com> wrote: >> >> > In the SMP configuration, we should either specify a topology >> > parameter with a reasonable value (equal to or greater than 1) >> > or just leave it omitted and QEMU will calculate its value. >> > Configurations which explicitly specify the topology parameters >> > as zero like "sockets=0" are meaningless, so disallow them. >> > >> > However; the commit 1e63fe685804d >> > (machine: pass QAPI struct to mc->smp_parse) has documented that >> > '0' has the same semantics as omitting a parameter in the qapi >> > comment for SMPConfiguration. So this patch fixes the doc and >> > also adds the corresponding sanity check in the smp parsers. >> >> Are we expecting any real users to have used that 'parameter=0' >> behaviour? I expect that libvirt and other management software already >> did the right thing; unfortunately, sometimes weird configuration lines >> tend to persist in search results. > > I understand this concern. I think the only documentation we had prior to > commit 1e63fe685804 was > > DEF("smp", HAS_ARG, QEMU_OPTION_smp, > "-smp > [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n" > " set the number of CPUs to 'n' [default=1]\n" > " maxcpus= maximum number of total cpus, including\n" > " offline CPUs for hotplug, etc\n" > " cores= number of CPU cores on one socket (for PC, it's > on one die)\n" > " threads= number of threads on one CPU core\n" > " dies= number of CPU dies on one socket (for PC only)\n" > " sockets= number of discrete sockets in the system\n", > QEMU_ARCH_ALL) > SRST > ``-smp > [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]`` > Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs > are supported. On Sparc32 target, Linux limits the number of usable > CPUs to 4. For the PC target, the number of cores per die, the > number of threads per cores, the number of dies per packages and the > total number of sockets can be specified. Missing values will be > computed. If any on the three values is given, the total number of > CPUs n can be omitted. maxcpus specifies the maximum number of > hotpluggable CPUs. > ERST > > This doesn't mention zero inputs and even implies non-zero inputs.
Yes, hopefully that kept people away from using 0 magic, unless they read the code. > > I'm not sure if we need to worry about the odd command line that used zero > for some parameters. What do you think? I did a cursory search for bad examples, and nothing popped up. So this should be reasonably painless.