[PATCH 00/10] Introduce virtio-mem <memory/> model

[Daniel Henrique Barboza]

On 1/22/21 9:50 AM, Michal Privoznik wrote:
> Technically, this is another version of:
> 
> https://www.redhat.com/archives/libvir-list/2020-December/msg00199.html
> 
> But since virtio-pmem part is pushed now, I've reworked virtio-mem a bit
> and sending it as a new series.
> 
> For curious ones, David summarized behaviour well when implementing
> virtio-mem support in kernel:
> 
> https://lwn.net/Articles/755423/
> 
> For less curious ones:
> 
>    # virsh update-memory $dom --requested-size 4G
> 
> adds additional 4GiB of RAM to guest;
> 
>    # virsh update-memory $dom --requested-size 0
> 
> removes those 4GiB added earlier.
> 
> Patches are also available on my GitLab:
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/tree/virtio_mem_v3

Code LGTM:

Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>


A few questions about the overall design:

- it is mentioned that 'requested-size' should respect the granularity
of the block unit, but later on the 'actual' attribute is added to track
the size that the device was expanded/shrunk. What happens if we forfeit
the granularity check of the memory increments? Will QEMU error out because
we're requesting an invalid value or it will silently size the device to a
plausible size?


- Reading the lwn article I understood that David implemented this support
for s390x as well. If that's the case, then I believe you should double
check later on what's the THP size that Z uses to be sure that it's the
same 2MiB value you're considering in patch 03.


- in patch 03 it is mentioned that:

"If it wants to give more memory to the guest it changes 'requested-size' to
a bigger value, and if it wants to shrink guest memory it changes the
'requested-size' to a smaller value. Note, value of zero means that guest
should release all memory offered by the device."

Does size zero implicates the virtio-mem device unplug? Will the device still
exist in the guest even with zeroed memory, acting as a sort of 'deflated
virtio-balloon'?



Thanks,


DHB



> 
> 
> Michal Prívozník (10):
>    virhostmem: Introduce virHostMemGetTHPSize()
>    qemu_capabilities: Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI
>    conf: Introduce virtio-mem <memory/> model
>    qemu: Build command line for virtio-mem
>    qemu: Wire up <memory/> live update
>    qemu: Wire up MEMORY_DEVICE_SIZE_CHANGE event
>    qemu: Refresh the actual size of virtio-mem on monitor reconnect
>    qemu: Recalculate balloon on MEMORY_DEVICE_SIZE_CHANGE event and
>      reconnect
>    virsh: Introduce update-memory command
>    news: document recent virtio memory addition
> 
>   NEWS.rst                                      |   7 +
>   docs/formatdomain.rst                         |  42 +++-
>   docs/manpages/virsh.rst                       |  31 +++
>   docs/schemas/domaincommon.rng                 |  16 ++
>   src/conf/domain_conf.c                        | 100 ++++++++-
>   src/conf/domain_conf.h                        |  13 ++
>   src/conf/domain_validate.c                    |  39 ++++
>   src/libvirt_private.syms                      |   3 +
>   src/qemu/qemu_alias.c                         |  10 +-
>   src/qemu/qemu_capabilities.c                  |   2 +
>   src/qemu/qemu_capabilities.h                  |   1 +
>   src/qemu/qemu_command.c                       |  13 +-
>   src/qemu/qemu_domain.c                        |  50 ++++-
>   src/qemu/qemu_domain.h                        |   1 +
>   src/qemu/qemu_domain_address.c                |  37 ++-
>   src/qemu/qemu_driver.c                        | 211 +++++++++++++++++-
>   src/qemu/qemu_hotplug.c                       |  18 ++
>   src/qemu/qemu_hotplug.h                       |   5 +
>   src/qemu/qemu_monitor.c                       |  37 +++
>   src/qemu/qemu_monitor.h                       |  27 +++
>   src/qemu/qemu_monitor_json.c                  |  94 ++++++--
>   src/qemu/qemu_monitor_json.h                  |   5 +
>   src/qemu/qemu_process.c                       | 101 ++++++++-
>   src/qemu/qemu_validate.c                      |   8 +
>   src/security/security_apparmor.c              |   1 +
>   src/security/security_dac.c                   |   2 +
>   src/security/security_selinux.c               |   2 +
>   src/util/virhostmem.c                         |  63 ++++++
>   src/util/virhostmem.h                         |   3 +
>   tests/domaincapsmock.c                        |   9 +
>   .../caps_5.1.0.x86_64.xml                     |   1 +
>   .../caps_5.2.0.x86_64.xml                     |   1 +
>   ...mory-hotplug-virtio-mem.x86_64-latest.args |  49 ++++
>   .../memory-hotplug-virtio-mem.xml             |  66 ++++++
>   tests/qemuxml2argvtest.c                      |   1 +
>   ...emory-hotplug-virtio-mem.x86_64-latest.xml |   1 +
>   tests/qemuxml2xmltest.c                       |   1 +
>   tools/virsh-domain.c                          | 154 +++++++++++++
>   38 files changed, 1165 insertions(+), 60 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
>   create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml
>   create mode 120000 tests/qemuxml2xmloutdata/memory-hotplug-virtio-mem.x86_64-latest.xml
>