Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

[Markus Armbruster <armbru@redhat.com>]

David Hildenbrand <david@redhat.com> writes:

> On 20.07.23 12:12, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>> This is a continuation of the v5 of the patch series located here:
>> https://lore.kernel.org/qemu-devel/cover.1686577753.git.maciej.szmigiero@oracle.com/
>> 
>
> We're now in QEMU soft-freeze, which means the memslot series might take a bit to land. I'm going to follow-up on that soonish.
>
>> Changes from v5:
>> * Incorporate David's rework of the driver on top of his virtio-mem-memslots
>> patches (specifically, commit 6769107d1a4f), making use of a memory region
>> container created upfront to avoid calling memory_device{,_pre}_plug()
>> functions from the driver and introducing a driver-specific MemoryDeviceInfo
>> sub-type.
>> * Include two additional David's memory-device patches necessary for the
>> aforementioned conversion in this patch set.
>> * Use multiple memslots to cover the hot-add memory backend in order to
>> reduce metadata size for the not-yet-hot-added part of the memory backend.
>> * Add David's "Co-developed-by:" to patches where he contributed some changes.
>> * Use OBJECT_DEFINE_TYPE_WITH_INTERFACES() and OBJECT_DECLARE_SIMPLE_TYPE()
>> macros instead of open-coding the equivalent functionality.
>> * Drop no longer necessary patch adding g_autoptr() cleanup function for the
>> Error type.
>> David Hildenbrand (2):
>>    memory-device: Support empty memory devices
>>    memory-device: Drop size alignment check
>> Maciej S. Szmigiero (4):
>>    Add Hyper-V Dynamic Memory Protocol definitions
>>    qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo
>>    qapi: Add HV_BALLOON_STATUS_REPORT event
>>    Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon)
>
> That is still a gigantic patch. Is there any way to split that into reasonable chunks? For example, move the whole hotplug/memslot part into
> a dedicated patch?
>
> See below on splitting off the PC changes.
>
>>   Kconfig.host                     |    3 +
>>   hw/core/machine-hmp-cmds.c       |   15 +
>>   hw/hyperv/Kconfig                |    5 +
>>   hw/hyperv/hv-balloon.c           | 2246 ++++++++++++++++++++++++++++++
>>   hw/hyperv/meson.build            |    1 +
>>   hw/hyperv/trace-events           |   18 +
>>   hw/i386/pc.c                     |   22 +
>>   hw/mem/memory-device.c           |   45 +-
>>   include/hw/hyperv/dynmem-proto.h |  423 ++++++
>>   include/hw/hyperv/hv-balloon.h   |   18 +
>>   include/hw/mem/memory-device.h   |    7 +-
>>   meson.build                      |   28 +-
>>   meson_options.txt                |    2 +
>>   qapi/machine.json                |   64 +-
>>   scripts/meson-buildoptions.sh    |    3 +
>
> It's probably best to separate the actual device implementation from wiring up the machine. That is, have a HV_BALLOON_SUPPORTED kconfig
> (like VIRTIO_MEM_SUPPORTED), and activate that in a single commit for
> PC, where you also modify hw/i386/pc.c.
>
> That commit would be called something like "pc: Support hv-balloon".

Furthermore, try to factor out stuff related to QMP:

0. The driver with QMP stuff omitted / stubbed out

1. Enable query-memory-devices

2. Add HV_BALLOON_STATUS_REPORT event