Re: [Qemu-devel] [PATCH RFC 09/16] hw/i386/pc: don't use smp_cores, smp_threads

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Jun 10, 2016 at 07:40:20PM +0200, Andrew Jones wrote:
[...]
> @@ -1940,9 +1943,10 @@ static void pc_machine_reset(void)
>  
>  static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
>  {
> +    CPUState *cs = first_cpu;

Eww.

>      X86CPUTopoInfo topo;
> -    x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index,
> -                          &topo);
> +
> +    x86_topo_ids_from_idx(cs->nr_cores, cs->nr_threads, cpu_index, &topo);

If first_cpu is already available[1], then current_machine would
also be available. Better to use the new MachineState fields you
added, than the CPUState::nr_{cores,threads} fields that
shouldn't have existed in the first place.

[1] Is it really available? This function is called very early,
    from parse_numa_opts(). I think current_machine is already
    available, though. But we need to ensure parse_numa_opts()
    will be called only after SMP option parsing is done and SMP
    topology data in MachineState is already populated.

    Later, we could also make the NUMA option parsing not depend
    on SMP parsing to be done first. The cpu_index_to_socket_id()
    magic can be moved to numa_post_machine_init(), probably.

-- 
Eduardo