All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cleber Rosa <crosa@redhat.com>
To: Yanan Wang <wangyanan55@huawei.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Andrew Jones" <drjones@redhat.com>,
	"Daniel P . Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Pierre Morel" <pmorel@linux.ibm.com>,
	"Pankaj Gupta" <pankaj.gupta.linux@gmail.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	yuzenghui@huawei.com, wanghaibin.wang@huawei.com
Subject: Re: [PATCH for-6.1 v2] machine: Disallow specifying topology parameters as zero
Date: Thu, 22 Jul 2021 18:25:02 -0400	[thread overview]
Message-ID: <874kcm0ywx.fsf@p50.localhost.localdomain> (raw)
In-Reply-To: <20210722154326.1464-2-wangyanan55@huawei.com>


Yanan Wang writes:

> 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.
>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  hw/core/machine.c | 14 ++++++++++++++
>  qapi/machine.json |  6 +++---
>  qemu-options.hx   | 12 +++++++-----
>  3 files changed, 24 insertions(+), 8 deletions(-)

Hi Yanan,

This looks somewhat similar to this very old patch of mine:

   https://mail.gnu.org/archive/html/qemu-devel/2020-10/msg03039.html

I'm putting a reference here because I believe the test can be salvaged
and slightly adapted for this patch of yours.

Let me know if I can help anyhow.

Thanks,
- Cleber.



  parent reply	other threads:[~2021-07-22 22:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 15:43 [PATCH for-6.1 v2 0/1] machine: Disallow specifying topology parameters as zero Yanan Wang
2021-07-22 15:43 ` [PATCH for-6.1 v2] " Yanan Wang
2021-07-22 16:01   ` Daniel P. Berrangé
2021-07-23  8:02     ` Markus Armbruster
2021-07-23  8:40       ` wangyanan (Y)
2021-07-23  8:46         ` Cornelia Huck
2021-07-22 16:15   ` Andrew Jones
2021-07-22 22:25   ` Cleber Rosa [this message]
2021-07-23  1:57     ` wangyanan (Y)
2021-07-23 14:14       ` Cleber Rosa

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=874kcm0ywx.fsf@p50.localhost.localdomain \
    --to=crosa@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=drjones@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=pankaj.gupta.linux@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=pmorel@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=wanghaibin.wang@huawei.com \
    --cc=wangyanan55@huawei.com \
    --cc=yuzenghui@huawei.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.