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

[David Hildenbrand <david@redhat.com>]

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".

-- 
Cheers,

David / dhildenb