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

Michal Privoznik posted 16 patches 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1631544341.git.mprivozn@redhat.com
There is a newer version of this series
NEWS.rst                                      |   7 +
docs/formatdomain.rst                         |  45 ++-
docs/kbase/index.rst                          |   4 +
docs/kbase/memorydevices.rst                  | 150 ++++++++++
docs/kbase/meson.build                        |   1 +
docs/manpages/virsh.rst                       |  30 ++
docs/schemas/domaincommon.rng                 |  16 ++
examples/c/misc/event-test.c                  |  17 ++
include/libvirt/libvirt-domain.h              |  24 ++
src/conf/domain_conf.c                        | 126 ++++++++-
src/conf/domain_conf.h                        |  16 ++
src/conf/domain_event.c                       |  84 ++++++
src/conf/domain_event.h                       |  10 +
src/conf/domain_validate.c                    |  39 +++
src/libvirt_private.syms                      |   5 +
src/qemu/qemu_alias.c                         |  10 +-
src/qemu/qemu_capabilities.c                  |   6 +
src/qemu/qemu_capabilities.h                  |   4 +
src/qemu/qemu_command.c                       |  25 +-
src/qemu/qemu_domain.c                        |  33 ++-
src/qemu/qemu_domain.h                        |   1 +
src/qemu/qemu_domain_address.c                |  38 ++-
src/qemu/qemu_driver.c                        | 259 +++++++++++++++++-
src/qemu/qemu_hotplug.c                       |  18 ++
src/qemu/qemu_hotplug.h                       |   5 +
src/qemu/qemu_monitor.c                       |  34 +++
src/qemu/qemu_monitor.h                       |  28 ++
src/qemu/qemu_monitor_json.c                  |  97 +++++--
src/qemu/qemu_monitor_json.h                  |   5 +
src/qemu/qemu_process.c                       |  72 +++++
src/qemu/qemu_validate.c                      |   8 +
src/remote/remote_daemon_dispatch.c           |  30 ++
src/remote/remote_driver.c                    |  32 +++
src/remote/remote_protocol.x                  |  15 +-
src/remote_protocol-structs                   |   7 +
src/security/security_apparmor.c              |   1 +
src/security/security_dac.c                   |   2 +
src/security/security_selinux.c               |   2 +
src/util/virhostmem.c                         |  54 ++++
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 +
.../caps_6.0.0.x86_64.xml                     |   1 +
.../caps_6.1.0.x86_64.xml                     |   2 +
...mory-hotplug-virtio-mem.x86_64-latest.args |  41 +++
.../memory-hotplug-virtio-mem.xml             |  67 +++++
tests/qemuxml2argvtest.c                      |   1 +
...emory-hotplug-virtio-mem.x86_64-latest.xml |   1 +
tests/qemuxml2xmltest.c                       |   1 +
tools/virsh-domain.c                          | 181 ++++++++++++
51 files changed, 1613 insertions(+), 56 deletions(-)
create mode 100644 docs/kbase/memorydevices.rst
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
[PATCH v5 00/16] Introduce virtio-mem <memory/> model
Posted by Michal Privoznik 2 years, 6 months ago
v4 of:

https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html

diff to v4:
- Rebased onto current master
- Worked in David's suggestions, e.g. rename from <actual/> to
  <current/>, implemented offline memory update, implemented --node
  argument to virsh update-memory-device, prealloc is OFF and reserve is
  ON for virtio-mem

Some suggestions are left as future work. For instance:
- Don't require memory slots because virtio-mem lives on PCI bus anyway
- Allow path backed backend for virtio-mem
- support .prealloc for virtio-mem object (not memory-backend-* !)


I keep occasionally rebased version on my gitlab:

https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/

Michal Prívozník (16):
  virhostmem: Introduce virHostMemGetTHPSize()
  qemu_capabilities: Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI
  qemu_capabilities: Introduce QEMU_CAPS_MEMORY_BACKEND_RESERVE
  conf: Introduce virtio-mem <memory/> model
  qemu: Build command line for virtio-mem
  qemu: Wire up <memory/> live update
  qemu: Wire up <memory/> offline update
  Introduce <current/> property to virtio-mem
  conf: Introduce virDomainMemoryFindByDeviceAlias()
  qemu: Wire up MEMORY_DEVICE_SIZE_CHANGE event
  qemu: Refresh the current size of virtio-mem on monitor reconnect
  qemu: Account for both memballoon and virtio-mem
  qemuDomainSetMemoryFlags: Take virtio-mem into consideration
  virsh: Introduce update-memory-device command
  news: document recent virtio memory addition
  kbase: Document virtio-mem

 NEWS.rst                                      |   7 +
 docs/formatdomain.rst                         |  45 ++-
 docs/kbase/index.rst                          |   4 +
 docs/kbase/memorydevices.rst                  | 150 ++++++++++
 docs/kbase/meson.build                        |   1 +
 docs/manpages/virsh.rst                       |  30 ++
 docs/schemas/domaincommon.rng                 |  16 ++
 examples/c/misc/event-test.c                  |  17 ++
 include/libvirt/libvirt-domain.h              |  24 ++
 src/conf/domain_conf.c                        | 126 ++++++++-
 src/conf/domain_conf.h                        |  16 ++
 src/conf/domain_event.c                       |  84 ++++++
 src/conf/domain_event.h                       |  10 +
 src/conf/domain_validate.c                    |  39 +++
 src/libvirt_private.syms                      |   5 +
 src/qemu/qemu_alias.c                         |  10 +-
 src/qemu/qemu_capabilities.c                  |   6 +
 src/qemu/qemu_capabilities.h                  |   4 +
 src/qemu/qemu_command.c                       |  25 +-
 src/qemu/qemu_domain.c                        |  33 ++-
 src/qemu/qemu_domain.h                        |   1 +
 src/qemu/qemu_domain_address.c                |  38 ++-
 src/qemu/qemu_driver.c                        | 259 +++++++++++++++++-
 src/qemu/qemu_hotplug.c                       |  18 ++
 src/qemu/qemu_hotplug.h                       |   5 +
 src/qemu/qemu_monitor.c                       |  34 +++
 src/qemu/qemu_monitor.h                       |  28 ++
 src/qemu/qemu_monitor_json.c                  |  97 +++++--
 src/qemu/qemu_monitor_json.h                  |   5 +
 src/qemu/qemu_process.c                       |  72 +++++
 src/qemu/qemu_validate.c                      |   8 +
 src/remote/remote_daemon_dispatch.c           |  30 ++
 src/remote/remote_driver.c                    |  32 +++
 src/remote/remote_protocol.x                  |  15 +-
 src/remote_protocol-structs                   |   7 +
 src/security/security_apparmor.c              |   1 +
 src/security/security_dac.c                   |   2 +
 src/security/security_selinux.c               |   2 +
 src/util/virhostmem.c                         |  54 ++++
 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 +
 .../caps_6.0.0.x86_64.xml                     |   1 +
 .../caps_6.1.0.x86_64.xml                     |   2 +
 ...mory-hotplug-virtio-mem.x86_64-latest.args |  41 +++
 .../memory-hotplug-virtio-mem.xml             |  67 +++++
 tests/qemuxml2argvtest.c                      |   1 +
 ...emory-hotplug-virtio-mem.x86_64-latest.xml |   1 +
 tests/qemuxml2xmltest.c                       |   1 +
 tools/virsh-domain.c                          | 181 ++++++++++++
 51 files changed, 1613 insertions(+), 56 deletions(-)
 create mode 100644 docs/kbase/memorydevices.rst
 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

-- 
2.32.0

Re: [PATCH v5 00/16] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 6 months ago
On 13.09.21 16:52, Michal Privoznik wrote:
> v4 of:
> 
> https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html
> 
> diff to v4:
> - Rebased onto current master
> - Worked in David's suggestions, e.g. rename from <actual/> to
>    <current/>, implemented offline memory update, implemented --node
>    argument to virsh update-memory-device, prealloc is OFF and reserve is
>    ON for virtio-mem
> 
> Some suggestions are left as future work. For instance:
> - Don't require memory slots because virtio-mem lives on PCI bus anyway
> - Allow path backed backend for virtio-mem

Just a note that

   <memoryBacking>
     <source type='file'/>
     <access mode='shared'/>
   </memoryBacking>

is doing what it's supposed to do. So only explicit file paths are not 
supported yet.

> - support .prealloc for virtio-mem object (not memory-backend-* !)
> 
> 
> I keep occasionally rebased version on my gitlab:
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/

I just played with it and "virsh update-memory-device" is working like a 
charm now:

a) with "--node"
b) with "--alias", including manually specified alias like "<alias 
name='ua-virtiomem1'/>"
c) with --config, --live, --current

I see that "aliases" prefixed with "ua-" are an existing concept. Maybe 
we want to cross-reference that in the virtio-mem documentation?

Nothing unusual found during my testing. I did not play with huge pages, 
as it's initially not supported.

Thanks a bunch!

-- 
Thanks,

David / dhildenb

Re: [PATCH v5 00/16] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 6 months ago
On 14.09.21 14:34, David Hildenbrand wrote:
> On 13.09.21 16:52, Michal Privoznik wrote:
>> v4 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html
>>
>> diff to v4:
>> - Rebased onto current master
>> - Worked in David's suggestions, e.g. rename from <actual/> to
>>     <current/>, implemented offline memory update, implemented --node
>>     argument to virsh update-memory-device, prealloc is OFF and reserve is
>>     ON for virtio-mem
>>
>> Some suggestions are left as future work. For instance:
>> - Don't require memory slots because virtio-mem lives on PCI bus anyway
>> - Allow path backed backend for virtio-mem
> 
> Just a note that
> 
>     <memoryBacking>
>       <source type='file'/>
>       <access mode='shared'/>
>     </memoryBacking>
> 
> is doing what it's supposed to do. So only explicit file paths are not
> supported yet.
> 
>> - support .prealloc for virtio-mem object (not memory-backend-* !)
>>
>>
>> I keep occasionally rebased version on my gitlab:
>>
>> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/
> 
> I just played with it and "virsh update-memory-device" is working like a
> charm now:
> 
> a) with "--node"
> b) with "--alias", including manually specified alias like "<alias
> name='ua-virtiomem1'/>"
> c) with --config, --live, --current
> 
> I see that "aliases" prefixed with "ua-" are an existing concept. Maybe
> we want to cross-reference that in the virtio-mem documentation?
> 
> Nothing unusual found during my testing. I did not play with huge pages,
> as it's initially not supported.

... and I just played with huge pages, and due to the added 
"reserve=off" it works just as expected, nice. (prealloc support to be 
added to make it actually safe to use)


-- 
Thanks,

David / dhildenb

[private] Re: [PATCH v5 00/16] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 6 months ago
On 13.09.21 16:52, Michal Privoznik wrote:
> v4 of:
> 
> https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html
> 
> diff to v4:
> - Rebased onto current master
> - Worked in David's suggestions, e.g. rename from <actual/> to
>    <current/>, implemented offline memory update, implemented --node
>    argument to virsh update-memory-device, prealloc is OFF and reserve is
>    ON for virtio-mem
> 
> Some suggestions are left as future work. For instance:
> - Don't require memory slots because virtio-mem lives on PCI bus anyway
> - Allow path backed backend for virtio-mem
> - support .prealloc for virtio-mem object (not memory-backend-* !)
> 
> 
> I keep occasionally rebased version on my gitlab:
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/


Hi Michal,

I noticed one minor thing:

If I start a VM with

     <numa>
       <cell id='0' cpus='0-7' memory='2097152' unit='KiB'/>
       <cell id='1' cpus='8-15' memory='2097152' unit='KiB'/>
     </numa>
     ...
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>0</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>2097152</requested>
       </target>
       <alias name='ua-virtiomem0'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
     </memory>
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>1</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>2097152</requested>
       </target>
       <alias name='ua-virtiomem1'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </memory>


I get after it booted up

   <maxMemory slots='2' unit='KiB'>41943040</maxMemory>
   <memory unit='KiB'>37748736</memory>
   <currentMemory unit='KiB'>9437184</currentMemory>
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>0</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>2097152</requested>
         <current unit='KiB'>131072</current>
       </target>
       <alias name='ua-virtiomem0'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
     </memory>
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>1</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>2097152</requested>
         <current unit='KiB'>2097152</current>
       </target>
       <alias name='ua-virtiomem1'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </memory>

Note the "<current unit='KiB'>131072</current>". Inside of the guest, I can see that it really is 2G.

If I trigger a new "virsh update-memory-device Fedora34", it gets updated
properly.

I assume the devices get initialized in the guest in parallel. Could it be that
libvirt gets confused when there are concurrent notifications about
two devices or could it be QEMU accidentally swallows some events? I'll investigate the
latter regarding rate limiting the output in QEMU, maybe something is messed up.


-- 
Thanks,

David / dhildenb

Re: [private] Re: [PATCH v5 00/16] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 6 months ago
On 21.09.21 11:33, David Hildenbrand wrote:
> On 13.09.21 16:52, Michal Privoznik wrote:
>> v4 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2021-June/msg00679.html
>>
>> diff to v4:
>> - Rebased onto current master
>> - Worked in David's suggestions, e.g. rename from <actual/> to
>>     <current/>, implemented offline memory update, implemented --node
>>     argument to virsh update-memory-device, prealloc is OFF and reserve is
>>     ON for virtio-mem
>>
>> Some suggestions are left as future work. For instance:
>> - Don't require memory slots because virtio-mem lives on PCI bus anyway
>> - Allow path backed backend for virtio-mem
>> - support .prealloc for virtio-mem object (not memory-backend-* !)
>>
>>
>> I keep occasionally rebased version on my gitlab:
>>
>> https://gitlab.com/MichalPrivoznik/libvirt/-/commits/virtio_mem_v5/
> 
> 
> Hi Michal,
> 
> I noticed one minor thing:
> 
> If I start a VM with
> 
>       <numa>
>         <cell id='0' cpus='0-7' memory='2097152' unit='KiB'/>
>         <cell id='1' cpus='8-15' memory='2097152' unit='KiB'/>
>       </numa>
>       ...
>       <memory model='virtio-mem'>
>         <target>
>           <size unit='KiB'>16777216</size>
>           <node>0</node>
>           <block unit='KiB'>2048</block>
>           <requested unit='KiB'>2097152</requested>
>         </target>
>         <alias name='ua-virtiomem0'/>
>         <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
>       </memory>
>       <memory model='virtio-mem'>
>         <target>
>           <size unit='KiB'>16777216</size>
>           <node>1</node>
>           <block unit='KiB'>2048</block>
>           <requested unit='KiB'>2097152</requested>
>         </target>
>         <alias name='ua-virtiomem1'/>
>         <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>       </memory>
> 
> 
> I get after it booted up
> 
>     <maxMemory slots='2' unit='KiB'>41943040</maxMemory>
>     <memory unit='KiB'>37748736</memory>
>     <currentMemory unit='KiB'>9437184</currentMemory>
>       <memory model='virtio-mem'>
>         <target>
>           <size unit='KiB'>16777216</size>
>           <node>0</node>
>           <block unit='KiB'>2048</block>
>           <requested unit='KiB'>2097152</requested>
>           <current unit='KiB'>131072</current>
>         </target>
>         <alias name='ua-virtiomem0'/>
>         <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
>       </memory>
>       <memory model='virtio-mem'>
>         <target>
>           <size unit='KiB'>16777216</size>
>           <node>1</node>
>           <block unit='KiB'>2048</block>
>           <requested unit='KiB'>2097152</requested>
>           <current unit='KiB'>2097152</current>
>         </target>
>         <alias name='ua-virtiomem1'/>
>         <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>       </memory>
> 
> Note the "<current unit='KiB'>131072</current>". Inside of the guest, I can see that it really is 2G.
> 
> If I trigger a new "virsh update-memory-device Fedora34", it gets updated
> properly.
> 
> I assume the devices get initialized in the guest in parallel. Could it be that
> libvirt gets confused when there are concurrent notifications about
> two devices or could it be QEMU accidentally swallows some events? I'll investigate the
> latter regarding rate limiting the output in QEMU, maybe something is messed up.

QEMU BUG, will send a fix :)


-- 
Thanks,

David / dhildenb