Re: [Qemu-devel] [PATCH 5/8] [PATCH RFC v3] s390-qemu: cpu hotplug - ipi_states enhancements

["Jason J. Herne" <jjherne@linux.vnet.ibm.com>]

On 09/05/2013 08:01 AM, Andreas Färber wrote:
> Am 01.08.2013 16:12, schrieb Jason J. Herne:
>> From: "Jason J. Herne" <jjherne@us.ibm.com>
>>
>> Modify s390_cpu_addr2state to allow fetching state information for cpu addresses
>> above smp_cpus.  Hotplug requires this capability.
>>
>> Also add s390_cpu_set_state function to allow modification of ipi_state entries
>> during hotplug.
>>
>> Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
>> ---
>>   hw/s390x/s390-virtio.c |    9 +++++----
>>   target-s390x/cpu.h     |    2 +-
>>   2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
>> index 21e9124..5ad9cf3 100644
>> --- a/hw/s390x/s390-virtio.c
>> +++ b/hw/s390x/s390-virtio.c
>> @@ -54,12 +54,13 @@
>>   static VirtIOS390Bus *s390_bus;
>>   static S390CPU **ipi_states;
>>
>> -S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>> +void s390_cpu_set_ipistate(uint16_t cpu_addr, S390CPU *state)
>>   {
>> -    if (cpu_addr >= smp_cpus) {
>> -        return NULL;
>> -    }
>> +    ipi_states[cpu_addr] = state;
>> +}
>>
>> +S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>> +{
>>       return ipi_states[cpu_addr];
>>   }
>>
>
> This is what got us into the link<> discussion last time. If we do
>
> for (i = 0; i < ARRAY_SIZE(ipi_states); i++) {
>      name = g_strdup_printf("cpu[%i]", i);
>      object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
>                               &ipi_states[i], &err);
> }
>
> then we get said /machine/cpu[n] link<> properties, at a QMP level
> either returning nothing or the canonical path to the CPU object.
>
> On IRC I didn't get an answer of whether it was being done the above way
> because there is infrastructure missing, and a look at object.h now
> confirms that suspicion. CC'ing Anthony and Paolo.
>
> Since object_property_add_link() uses a NULL opaque, my idea would be to
> add a single setter hook argument passed through as opaque to
> object_set_link_property(), which would call it with the old and the new
> value.
>
> The purpose would be to avoid growing our own internal setter API, which
> is disjoint from the QMP qom-set we are targetting at.
>

Ok, you lost me :).  I must admit my understanding of QOM is still 
limited. Sorry for not keeping up.  Why do we need this new hook?  The 
link would contain a pointer to the correct ipi_states entry right? 
What are we using opaque for?

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)