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

[Andreas Färber]

Am 01.08.2013 16:12, schrieb Jason J. Herne:
> From: "Jason J. Herne" <address@hidden>
> 
> 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 <address@hidden>
> ---
>  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.

Regards,
Andreas

> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 877eac7..62eb810 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -377,7 +377,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
> *cpu, int type,
>  
>  uint8_t *s390_get_storage_keys(void);
>  void s390_alloc_storage_keys(ram_addr_t ram_size);
> -
> +void s390_cpu_set_ipistate(uint16_t cpu_addr, S390CPU *state);
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
>  void s390_add_running_cpu(S390CPU *cpu);
>  unsigned s390_del_running_cpu(S390CPU *cpu);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg