Re: [Qemu-devel] [PATCH 05/11] qdev: add "hotplugable" property to Device

[Paolo Bonzini]

Il 11/12/2013 17:57, Igor Mammedov ha scritto:
> On Wed, 11 Dec 2013 17:29:06 +0100
> Paolo Bonzini <address@hidden> wrote:
> 
>> Il 11/12/2013 02:01, Igor Mammedov ha scritto:
>>> Currently it's possible to make PCIDevice not hotplugable by using
>>> no_hotplug field of PCIDeviceClass. However it limits this
>>> only to PCI devices and prevents from generalizing hotplug code.
>>>
>>> So add similar field to DeviceClass so it could be reused with other
>>> Devices and would allow to replace PCI specific hotplug callbacks
>>> with generic implementation.
>>>
>>> In addition expose field as "hotplugable" readonly property, to make
>>> it possible to get it via QOM interface.
>>>
>>> Signed-off-by: Igor Mammedov <address@hidden>
>>> ---
>>>  hw/core/qdev.c         |    8 ++++++++
>>>  include/hw/qdev-core.h |    3 +++
>>>  2 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 25c2d2c..1b1a684 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -719,6 +719,12 @@ static void device_set_realized(Object *obj, bool 
>>> value, Error **err)
>>>      dev->realized = value;
>>>  }
>>>  
>>> +static bool device_get_hotplugable(Object *obj, Error **err)
>>> +{
>>> +    DeviceClass *dc = DEVICE_GET_CLASS(obj);
>>> +    return dc->hotplugable;
>>> +}
>>> +
>>>  static void device_initfn(Object *obj)
>>>  {
>>>      DeviceState *dev = DEVICE(obj);
>>> @@ -736,6 +742,8 @@ static void device_initfn(Object *obj)
>>>  
>>>      object_property_add_bool(obj, "realized",
>>>                               device_get_realized, device_set_realized, 
>>> NULL);
>>> +    object_property_add_bool(obj, "hotplugable",
>>> +                             device_get_hotplugable, NULL, NULL);
>>>  
>>>      class = object_get_class(OBJECT(dev));
>>>      do {
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index 684a5da..5d4a9c8 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -50,6 +50,8 @@ struct VMStateDescription;
>>>   * is changed to %true. Deprecated, new types inheriting directly from
>>>   * TYPE_DEVICE should use @realize instead, new leaf types should consult
>>>   * their respective parent type.
>>> + * @hotplugable: booleean indicating if #DeviceClass is hotplugable, 
>>> available
>>> + * as readonly "hotplugable" property of #DeviceState instance
>>>   *
>>>   * # Realization #
>>>   * Devices are constructed in two stages,
>>> @@ -99,6 +101,7 @@ typedef struct DeviceClass {
>>>      const char *desc;
>>>      Property *props;
>>>      int no_user;
>>> +    bool hotplugable;
>>>  
>>>      /* callbacks */
>>>      void (*reset)(DeviceState *dev);
>>>
>>
>> Does this patch break SCSI and virtio-serial hotplug?
> Ouch,
> this  patch doesn't but 
> "[PATCH 07/11] qdev:pci: refactor PCIDevice to use generic "hotplugable"
> property" does. See 2 hunks in hw/core/qdev.c

If you set dc->hotpluggable = true, those two hunks could also be moved
here.

Paolo

>>
>> Perhaps you could initialize dc->hotpluggable to true, and return
> that's probably the easiest way, since before series all devices were
> considered hotplugable from qdev POV, so it makes sense to make DeviceClass
> hotplugable by default to avoid unexpected regressions.
> 
>> "dc->hotplugable && device->parent_bus->allow_hotplug" from the property
>> getter? 
> sure, since there is no bus-less hotlpug it won't hurt.
> I'll respin amended patch as reply to this thread if it's ok.
> 
>> Or just set the class to true in the common superclasses of all
>> buses that support hotplug, but that's a bit harder.
> perhaps exercise for the later.
> 
>>
>> Paolo
> 
>