Re: [Qemu-devel] [PATCH] cpu: don't allow negative core id

[Laurent Vivier <lvivier@redhat.com>]

On 02/08/2017 15:42, Philippe Mathieu-Daudé wrote:
> Hi Laurent,
> 
> On Wed, Aug 2, 2017 at 7:32 AM, Laurent Vivier <lvivier@redhat.com> wrote:
>> With pseries machine type a negative core-id is not managed properly:
>> -1 gives an inaccurate error message ("core -1 already populated"),
>> -2 crashes QEMU (core dump)
>>
>> As it seems a negative value is invalid for any architecture,
>> instead of checking this in spapr_core_pre_plug() I think it's better
>> to check this in the generic part, core_prop_set_core_id()
> 
> Why is this property signed? If there is not reason to use it negative,
> is it possible to use object_property_add(.."uint"..)?

You should be right:

{ 'struct': 'NumaNodeOptions',
  'data': {
   '*nodeid': 'uint16',
   '*cpus':   ['uint16'],
   '*mem':    'size',
   '*memdev': 'str' }}

but

{ 'struct': 'CpuInstanceProperties',
  'data': { '*node-id': 'int',
            '*socket-id': 'int',
            '*core-id': 'int',
            '*thread-id': 'int'
  }
}

But I'm not sure it's a good idea to change the API now.

> Also what about core_prop_set_nr_threads()? It might coredump the
> same way.

In pseries case, there is another test in the spapr part that prevents that.

Thanks,
Laurent