Re: [Qemu-devel] [PATCH 14/30] target-i386: ICC bus: replace BusState.allow_hotplug with hotplug_handler

[Paolo Bonzini <pbonzini@redhat.com>]

Il 24/09/2014 13:48, Igor Mammedov ha scritto:
> it will allow to drop BusState.allow_hotplug field
> and still keep ICC bus hotluggable for CPU/APIC devices.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/cpu/icc_bus.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 7f44c59..cb5ed5c 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -24,18 +24,10 @@
>  
>  /* icc-bridge implementation */
>  
> -static void icc_bus_init(Object *obj)
> -{
> -    BusState *b = BUS(obj);
> -
> -    b->allow_hotplug = true;
> -}
> -
>  static const TypeInfo icc_bus_info = {
>      .name = TYPE_ICC_BUS,
>      .parent = TYPE_BUS,
>      .instance_size = sizeof(ICCBus),
> -    .instance_init = icc_bus_init,
>  };
>  
>  
> @@ -100,11 +92,19 @@ static void icc_bridge_init(Object *obj)
>      s->icc_bus.apic_address_space = &s->apic_container;
>  }
>  
> +static void icc_bridge_realize(DeviceState *dev, Error **errp)
> +{
> +    ICCBridgeState *s = ICC_BRIGDE(dev);
> +
> +    qbus_set_hotplug_handler(BUS(&s->icc_bus), dev, errp);
> +}
> +
>  static void icc_bridge_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>  
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->realize = icc_bridge_realize;
>  }
>  
>  static const TypeInfo icc_bridge_info = {
> @@ -113,6 +113,10 @@ static const TypeInfo icc_bridge_info = {
>      .instance_init  = icc_bridge_init,
>      .instance_size  = sizeof(ICCBridgeState),
>      .class_init = icc_bridge_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_HOTPLUG_HANDLER },
> +        { }
> +    }
>  };
>  
>  
> 

If you do this, aren't you enabling CPU hot-unplug?  Should
hotplug_handler_plug and/or hotplug_handler_unplug return an error if
there is no callback?

Paolo