On Mon, Feb 13, 2023 at 01:41:19PM +0000, Daniel P. Berrangé wrote:
> Date: Mon, 13 Feb 2023 13:41:19 +0000
> From: "Daniel P. Berrangé" <berra...@redhat.com>
> Subject: Re: [RFC 04/52] i386/WHPX: Fix error message when fail to set
> ProcessorCount
>
> On Mon, Feb 13, 2023 at 05:49:47PM +0800, Zhao Liu wrote:
> > From: Zhao Liu <zhao1....@intel.com>
> >
> > 003f230 (machine: Tweak the order of topology members in struct
> > CpuTopology) changes the meaning of MachineState.smp.cores from "the
> > number of cores in one package" to "the number of cores in one die"
> > and doesn't fix other uses of MachineState.smp.cores. And because of
> > the introduction of cluster, now smp.cores just means "the number of
> > cores in one cluster". This clearly does not fit the semantics here.
> >
> > And before this error message, WHvSetPartitionProperty() is called to
> > set prop.ProcessorCount.
> >
> > So the error message should show the prop.ProcessorCount other than
> > "cores per cluster" or "cores per package".
> >
> > Cc: Sunil Muthuswamy <sunil...@microsoft.com>
> > Signed-off-by: Zhao Liu <zhao1....@intel.com>
> > ---
> > target/i386/whpx/whpx-all.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> This patch and the 3 patches before it all look like basic bug
> fixes to current code, not really part of the new hybrid topology
> feature work.
>
> As such I'd suggest sending these first four patches as a separate
> series, so the bug fixes can be merged fairly quickly. I expect the
> rest of the hybrid topology series is going to take a long time to
> get agreement on, so no need to delay the easy bug fixes.
Yes, I'll split out the first four. Thanks!
Zhao
>
> >
> > diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
> > index e738d83e8191..fc349f887e47 100644
> > --- a/target/i386/whpx/whpx-all.c
> > +++ b/target/i386/whpx/whpx-all.c
> > @@ -2613,8 +2613,8 @@ static int whpx_accel_init(MachineState *ms)
> > sizeof(WHV_PARTITION_PROPERTY));
> >
> > if (FAILED(hr)) {
> > - error_report("WHPX: Failed to set partition core count to %d,"
> > - " hr=%08lx", ms->smp.cores, hr);
> > + error_report("WHPX: Failed to set partition processor count to %d,"
> > + " hr=%08lx", prop.ProcessorCount, hr);
> > ret = -EINVAL;
> > goto error;
> > }
> > --
> > 2.34.1
> >
> >
>
> With 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 :|
>
