Re: [Qemu-devel] [PATCH v4 1/1] numa: Print warning if no node is assigned to a CPU

[Michael S. Tsirkin]

On Wed, Mar 04, 2015 at 10:35:51AM -0300, Eduardo Habkost wrote:
> On Wed, Mar 04, 2015 at 01:30:35PM +0100, Michael S. Tsirkin wrote:
> > On Tue, Feb 24, 2015 at 03:57:23PM -0300, Eduardo Habkost wrote:
> > > Instead of silently assigning CPU to node 0 when it is omitted from the
> > > command-line, check if all CPUs up to max_cpus are present in the NUMA
> > > configuration.
> > 
> > Could you please describe the problematic configuration
> > in more detail in commit log?
> > Also, how does this interact with cpu hotplug?
> 
> I will try to reword the message to make it clearer. Suggestions are
> welcome.


Just include the command line that reproduces the problem,
and describe the guest behaviour that results.

> Basically, the problem is:
> 
> * Currently all possible CPUs (including hotplug ones) need to be
>   present in the SRAT when QEMU is started[1]. QEMU already does that.
> * When a CPU is ommitted from the command-line, QEMU is silently
>   assigning it to node 0. This is not a very useful default and
>   just confuse users.

It's reasonable if user doesn't want to play with numa, isn't it?

> 
> > > 
> > > I am making this a warning and not a fatal error, to allow management
> > > software to be updated if necessary.
> > >
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > > v1 -> v2: (no changes)
> > > 
> > > v2 -> v3:
> > >  * Use enumerate_cpus() and error_report() for error message
> > >  * Simplify logic using bitmap_full()
> > > 
> > > v3 -> v4:
> > >  * Clarify error message, mention that all CPUs up to
> > >    maxcpus need to be described in NUMA config
> > > ---
> > >  numa.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/numa.c b/numa.c
> > > index 9a3fc15..d8021b9 100644
> > > --- a/numa.c
> > > +++ b/numa.c
> > > @@ -201,6 +201,16 @@ static void validate_numa_cpus(void)
> > >          bitmap_or(seen_cpus, seen_cpus,
> > >                    numa_info[i].node_cpu, MAX_CPUMASK_BITS);
> > >      }
> > > +
> > > +    if (!bitmap_full(seen_cpus, max_cpus)) {
> > > +        char *msg;
> > > +        bitmap_complement(seen_cpus, seen_cpus, max_cpus);
> > > +        msg = enumerate_cpus(seen_cpus, max_cpus);
> > > +        error_report("warning: CPU(s) not present in any NUMA nodes: 
> > > %s", msg);
> > > +        error_report("warning: All CPU(s) up to maxcpus should be 
> > > described "
> > > +                     "in NUMA config");
> > 
> > What happens with e.g. windows guests when this warning is emitted?
> > do they blue-screen?
> 
> As described on the commit log above, the CPU is silently assigned to
> node 0 when it is omitted from the command-line. In other words, this
> won't make guests confused, but can make users confused.
> 
> All possible VCPUs (including the hotplug ones, i.e. up to max_cpus) are
> expected to be present in the SRAT, and they are already present. The
> only problem is that instead of complaining about missing CPUs, QEMU is
> assigning node 0 to them.
> 
> -- 
> Eduardo