Re: [RFC 04/52] i386/WHPX: Fix error message when fail to set ProcessorCount

[Zhao Liu]

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é" <berrange@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.liu@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 <sunilmut@microsoft.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@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 :|
>