Re: [PATCH for-6.1 0/1] machine: Disallow specifying topology parameters as zero

[wangyanan (Y)]

On 2021/7/22 14:02, Cornelia Huck wrote:
> On Thu, Jul 22 2021, Yanan Wang <wangyanan55@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 think there may not any users who have already used a configuration
with explicit "parameter=0", instead it should have been just omitted.
Yes, libvirt now rejects "parameter=0" when parsing XML, but we now
still allows "parameter=0" in the direct QEMU cmdlines. If we hope to
disallow this kind of config thoroughly, we'd better also have a sanity
check in QEMU.

Thanks,
Yanan
> > This patch originly comes form [1], and it was suggested that
> > this patch fixing the doc should be sent for 6.1 to avoid a
> > deprecation process in the future.
> >
> > [1] YPWsThPiZa3mF+zp@redhat.com/">https://lore.kernel.org/qemu-devel/YPWsThPiZa3mF+zp@redhat.com/
> >
> > Yanan Wang (1):
> >    machine: Disallow specifying topology parameters as zero
> >
> >   hw/core/machine.c | 30 ++++++++++++++++++++++--------
> >   hw/i386/pc.c      | 33 ++++++++++++++++++++++++---------
> >   qapi/machine.json |  6 +++---
> >   3 files changed; 49 insertions(+); 20 deletions(-)
> .