Re: [Qemu-devel] [qemu-s390x] [PATCH v4 00/14] MemoryDevice: use multi stage hotplug handlers

[Paolo Bonzini]

On 31/05/2018 13:47, Igor Mammedov wrote:
> On Wed, 30 May 2018 16:03:29 +0200
> Paolo Bonzini <address@hidden> wrote:
> 
>> On 25/05/2018 14:43, David Hildenbrand wrote:
>>> On 17.05.2018 10:15, David Hildenbrand wrote:  
>>>> We can have devices that need certain other resources that are e.g.
>>>> system resources managed by the machine. We need a clean way to assign
>>>> these resources (without violating layers as brought up by Igor).
>>>>
>>>> One example is virtio-mem/virtio-pmem. Both device types need to be
>>>> assigned some region in guest physical address space. This device memory
>>>> belongs to the machine and is managed by it. However, virito devices are
>>>> hotplugged using the hotplug handler their proxy device implements. So we
>>>> could trigger e.g. a PCI hotplug handler for virtio-pci or a CSS/CCW
>>>> hotplug handler for virtio-ccw. But definetly not the machine.
>>>>
>>>> Now, we can route other devices through the machine hotplug handler, to
>>>> properly assign/unassign resources - like a portion in guest physical
>>>> address space.
>>>>
>>>> v3 -> v4:
>>>> - Removed the s390x bits, will send that out separately (was just a proof
>>>>   that it works just fine with s390x)
>>>> - Fixed a typo and reworded a comment
>>>>
>>>> v2 -> v3:
>>>> - Added "memory-device: introduce separate config option"
>>>> - Dropped "parent_bus" check from hotplug handler lookup functions
>>>> - "Handly" -> "Handle" in patch description.
>>>>
>>>> v1 -> v2:
>>>> - Use multi stage hotplug handler instead of resource handler
>>>> - MemoryDevices only compiled if necessary (CONFIG_MEM_HOTPLUG)
>>>> - Prepare PC/SPAPR machines properly for multi stage hotplug handlers
>>>> - Route SPAPR unplug code via the hotunplug handler
>>>> - Directly include s390x support. But there are no usable memory devices
>>>>   yet (well, only my virtio-mem prototype)
>>>> - Included "memory-device: drop assert related to align and start of 
>>>> address
>>>>   space"
>>>>
>>>> David Hildenbrand (13):
>>>>   memory-device: drop assert related to align and start of address space
>>>>   memory-device: introduce separate config option
>>>>   pc: prepare for multi stage hotplug handlers
>>>>   pc: route all memory devices through the machine hotplug handler
>>>>   spapr: prepare for multi stage hotplug handlers
>>>>   spapr: route all memory devices through the machine hotplug handler
>>>>   spapr: handle pc-dimm unplug via hotplug handler chain
>>>>   spapr: handle cpu core unplug via hotplug handler chain
>>>>   memory-device: new functions to handle plug/unplug
>>>>   pc-dimm: implement new memory device functions
>>>>   memory-device: factor out pre-plug into hotplug handler
>>>>   memory-device: factor out unplug into hotplug handler
>>>>   memory-device: factor out plug into hotplug handler
>>>>
>>>> Igor Mammedov (1):
>>>>   qdev: let machine hotplug handler to override bus hotplug handler
>>>>
>>>>  default-configs/i386-softmmu.mak   |   3 +-
>>>>  default-configs/ppc64-softmmu.mak  |   3 +-
>>>>  default-configs/x86_64-softmmu.mak |   3 +-
>>>>  hw/Makefile.objs                   |   2 +-
>>>>  hw/core/qdev.c                     |   6 +-
>>>>  hw/i386/pc.c                       | 102 ++++++++++++++++++++++-------
>>>>  hw/mem/Makefile.objs               |   4 +-
>>>>  hw/mem/memory-device.c             | 129 
>>>> +++++++++++++++++++++++--------------
>>>>  hw/mem/pc-dimm.c                   |  48 ++++++--------
>>>>  hw/mem/trace-events                |   4 +-
>>>>  hw/ppc/spapr.c                     | 129 
>>>> +++++++++++++++++++++++++++++++------
>>>>  include/hw/mem/memory-device.h     |  21 ++++--
>>>>  include/hw/mem/pc-dimm.h           |   3 +-
>>>>  include/hw/qdev-core.h             |  11 ++++
>>>>  qapi/misc.json                     |   2 +-
>>>>  15 files changed, 330 insertions(+), 140 deletions(-)
>>>>  
>>>
>>> As there was no negative feedback so far, I will go ahead and assume
>>> that this approach is the right thing to do.  
>>
>> Ok, I'll queue this.
> I think it's a bit premature.
> Series would need a respin and it should also include
> for completness at leas one actual user (virtio-mem) to see
> how new interfaces/wrappers would be used and if they actually needed.

Yeah, I noticed that a respin was needed after sending this.  Thanks,

Paolo