Re: [Qemu-devel] [PATCH v2 1/3] numa: move numa global variable nb_numa_nodes into MachineState

[Tao Xu]

On 4/18/2019 10:26 PM, Eduardo Habkost wrote:
> On Thu, Apr 18, 2019 at 02:46:00PM +0800, Tao Xu wrote:
> > The aim of this patch is to add struct NumaState in MachineState
> > and move existing numa global nb_numa_nodes into NumaState.
> > And add variable numa_support into MachineClass to decide which
> > submachines support NUMA.
> >
> > Suggested-by: Igor Mammedov <address@hidden>
> > Suggested-by: Eduardo Habkost <address@hidden>
> > Signed-off-by: Tao Xu <address@hidden>
> [...]
> > +    int nb_numa_nodes = ms->numa_state ? ms->numa_state->nb_numa_nodes : 0;
>
> I see 17 occurrences of this same pattern in the patch, and I had
> to double-check the ones that didn't check if ms->numa_state is
> NULL.
>
> I think a
>    int machine_num_numa_nodes(MachineState *ms)
> helper would be useful.
>
> While at it, I suggest naming the nb_numa_nodes field "num_nodes"
> or "nr_nodes", because it's already inside a struct called
> NumaState.
Thank you for your review, I will improve these in the next version of 
patch.