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

Michal Privoznik posted 15 patches 3 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1615982004.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              |  23 ++
src/conf/domain_conf.c                        | 115 ++++++++-
src/conf/domain_conf.h                        |  15 ++
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                  |   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                |  38 ++-
src/qemu/qemu_driver.c                        | 233 +++++++++++++++++-
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                  |  97 ++++++--
src/qemu/qemu_monitor_json.h                  |   5 +
src/qemu/qemu_process.c                       | 118 ++++++++-
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                         |  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 +
.../caps_6.0.0.x86_64.xml                     |   1 +
...mory-hotplug-virtio-mem.x86_64-latest.args |  49 ++++
.../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                          | 169 +++++++++++++
50 files changed, 1612 insertions(+), 67 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 v3 00/15] Introduce virtio-mem <memory/> model
Posted by Michal Privoznik 3 years, 1 month ago
v3 of:

https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html

diff to v2:
- Dropped code that forbade use of virtio-mem and memballoon at the same
  time;
- This meant that I had to adjust memory accounting,
  qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are new.
- Fixed small nits raised by Peter in his review of v2

Michal Prívozník (15):
  virhostmem: Introduce virHostMemGetTHPSize()
  qemu_process: Deduplicate code in qemuProcessNeedHugepagesPath()
  qemu_process: Drop needless check in
    qemuProcessNeedMemoryBackingPath()
  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
  Introduce MEMORY_DEVICE_SIZE_CHANGE event
  qemu: Refresh the actual 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              |  23 ++
 src/conf/domain_conf.c                        | 115 ++++++++-
 src/conf/domain_conf.h                        |  15 ++
 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                  |   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                |  38 ++-
 src/qemu/qemu_driver.c                        | 233 +++++++++++++++++-
 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                  |  97 ++++++--
 src/qemu/qemu_monitor_json.h                  |   5 +
 src/qemu/qemu_process.c                       | 118 ++++++++-
 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                         |  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 +
 .../caps_6.0.0.x86_64.xml                     |   1 +
 ...mory-hotplug-virtio-mem.x86_64-latest.args |  49 ++++
 .../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                          | 169 +++++++++++++
 50 files changed, 1612 insertions(+), 67 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.26.2

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 10 months ago
On 17.03.21 12:57, Michal Privoznik wrote:
> v3 of:
> 
> https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html
> 
> diff to v2:
> - Dropped code that forbade use of virtio-mem and memballoon at the same
>    time;
> - This meant that I had to adjust memory accounting,
>    qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are new.
> - Fixed small nits raised by Peter in his review of v2
> 
>

Hi Michal, do you have a branch somewhere that I can easily checkout to 
play with it/test it?

I can spot that at least patch #2 and #3 are already upstream. The 
remaining patches don't seem to apply cleanly on current upstream/master.

Thanks!

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by Michal Prívozník 2 years, 10 months ago
On 6/17/21 11:44 AM, David Hildenbrand wrote:
> On 17.03.21 12:57, Michal Privoznik wrote:
>> v3 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html
>>
>>
>> diff to v2:
>> - Dropped code that forbade use of virtio-mem and memballoon at the same
>>    time;
>> - This meant that I had to adjust memory accounting,
>>    qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are
>> new.
>> - Fixed small nits raised by Peter in his review of v2
>>
>>
> 
> Hi Michal, do you have a branch somewhere that I can easily checkout to
> play with it/test it?
> 

Yes:

https://gitlab.com/MichalPrivoznik/libvirt/-/tree/virtio_mem_v4

There were some comments in Peter's review and I really should fix my
code according to them and merge/send v4.

Michal

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 10 months ago
On 17.06.21 13:18, Michal Prívozník wrote:
> On 6/17/21 11:44 AM, David Hildenbrand wrote:
>> On 17.03.21 12:57, Michal Privoznik wrote:
>>> v3 of:
>>>
>>> https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html
>>>
>>>
>>> diff to v2:
>>> - Dropped code that forbade use of virtio-mem and memballoon at the same
>>>     time;
>>> - This meant that I had to adjust memory accounting,
>>>     qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are
>>> new.
>>> - Fixed small nits raised by Peter in his review of v2
>>>
>>>
>>
>> Hi Michal, do you have a branch somewhere that I can easily checkout to
>> play with it/test it?
>>
> 
> Yes:
> 
> https://gitlab.com/MichalPrivoznik/libvirt/-/tree/virtio_mem_v4
> 
> There were some comments in Peter's review and I really should fix my
> code according to them and merge/send v4.
> 
> Michal
> 

Thanks! I started with a single virtio-mem device.

1. NUMA requirement

Right now, one can really only configure "maxMemory" with NUMA specified,
otherwise there will be "error: unsupported configuration: At least
one numa node has to be configured when enabling memory hotplug".

I recall this is a limitation of older QEMU which would not create ACPI SRAT
tables otherwise. In QEMU, this is no longer the case. As soon as "maxmem"
is specified on the QEMU cmdline, we fake a single NUMA node:

hw/core/numa.c:numa_complete_configuration()

"Enable NUMA implicitly by adding a new NUMA node automatically"
-> m->auto_enable_numa_with_memdev / mc->auto_enable_numa_with_memhp

m->auto_enable_numa_with_memdev (slots=0) is set since 5.1 on x86-64 and arm64
m->auto_enable_numa_with_memhp (slots>0) is set since 2.11 on x86-64 and 4.1 on arm64

So in theory, with newer QEMU on x86-64 and arm64 we could drop that
limitation in libvirt (might require some changes eventually
regarding the "node" specification handling). ppc64 shouldn't care as there is no ACPI.


2. "virsh update-memory-device"

My domain looks something like:

<domain type='kvm' id='3'>
   <name>Fedora 34</name>
...
   <maxMemory slots='16' unit='KiB'>20971520</maxMemory>
   <memory unit='KiB'>20971520</memory>
   <currentMemory unit='KiB'>4194304</currentMemory>
   <vcpu placement='static'>16</vcpu>
...
   <cpu mode='custom' match='exact' check='full'>
     <numa>
       <cell id='0' cpus='0-15' memory='4194304' unit='KiB'/>
     </numa>
   </cpu>
...
   <devices>
...
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>0</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>524288</requested>
         <actual unit='KiB'>0</actual>
       </target>
       <alias name='virtiomem0'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
     </memory>

   </devices>
...


It starts just fine and I can query the current properties

# virsh dumpxml "Fedora 34"
...
     <memory model='virtio-mem'>
       <target>
         <size unit='KiB'>16777216</size>
         <node>0</node>
         <block unit='KiB'>2048</block>
         <requested unit='KiB'>524288</requested>
         <actual unit='KiB'>524288</actual>
       </target>
       <alias name='virtiomem0'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
     </memory>

Trying to update the requested size, however results in

[root@virtlab704 ~]# virsh update-memory-device "Fedora 34" --requested-size 0
error: unsupported configuration: Attaching memory device with size '16777216' would exceed domain's maxMemory config size '20971520'

Tried with "--alias" with the same result.

Is it maybe an issue regarding handling of "maxMemory" vs "memory" ?


3. "memory" item handling

Whenever I edit the XML and set e.g., "<memory unit='GiB'>4</memory>", it's silently converted back to "20 GiB".
Maybe that's just always implicitly calculated from the NUMA spec and the defined devices.


Thanks!

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by Peter Krempa 2 years, 10 months ago
On Thu, Jun 17, 2021 at 14:03:44 +0200, David Hildenbrand wrote:
> On 17.06.21 13:18, Michal Prívozník wrote:
> > On 6/17/21 11:44 AM, David Hildenbrand wrote:
> > > On 17.03.21 12:57, Michal Privoznik wrote:
> > > > v3 of:
> > > > 
> > > > https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html
> > > > 
> > > > 
> > > > diff to v2:
> > > > - Dropped code that forbade use of virtio-mem and memballoon at the same
> > > >     time;
> > > > - This meant that I had to adjust memory accounting,
> > > >     qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are
> > > > new.
> > > > - Fixed small nits raised by Peter in his review of v2
> > > > 
> > > > 
> > > 
> > > Hi Michal, do you have a branch somewhere that I can easily checkout to
> > > play with it/test it?
> > > 
> > 
> > Yes:
> > 
> > https://gitlab.com/MichalPrivoznik/libvirt/-/tree/virtio_mem_v4
> > 
> > There were some comments in Peter's review and I really should fix my
> > code according to them and merge/send v4.
> > 
> > Michal
> > 
> 
> Thanks! I started with a single virtio-mem device.
> 
> 1. NUMA requirement
> 
> Right now, one can really only configure "maxMemory" with NUMA specified,
> otherwise there will be "error: unsupported configuration: At least
> one numa node has to be configured when enabling memory hotplug".
> 
> I recall this is a limitation of older QEMU which would not create ACPI SRAT
> tables otherwise. In QEMU, this is no longer the case. As soon as "maxmem"
> is specified on the QEMU cmdline, we fake a single NUMA node:
> 
> hw/core/numa.c:numa_complete_configuration()
> 
> "Enable NUMA implicitly by adding a new NUMA node automatically"
> -> m->auto_enable_numa_with_memdev / mc->auto_enable_numa_with_memhp
> 
> m->auto_enable_numa_with_memdev (slots=0) is set since 5.1 on x86-64 and arm64
> m->auto_enable_numa_with_memhp (slots>0) is set since 2.11 on x86-64 and 4.1 on arm64
> 
> So in theory, with newer QEMU on x86-64 and arm64 we could drop that
> limitation in libvirt (might require some changes eventually
> regarding the "node" specification handling). ppc64 shouldn't care as there is no ACPI.

The main reason for the check to be present is actually exactly what you
are describing. qemu fakes the numa node in case none are configured,
this means that in case where libvirt would not enforce that you'd get a
discrepancy between the config and what qemu exposes.


[...]

> 3. "memory" item handling
> 
> Whenever I edit the XML and set e.g., "<memory unit='GiB'>4</memory>", it's silently converted back to "20 GiB".
> Maybe that's just always implicitly calculated from the NUMA spec and the defined devices.

In cases where you've got numa confuigured, the <memory> element is
re-calculated back from the size of the numa nodes, as when you change
the value there isn't any obvious algorithm on picking NUMA nodes where
to pull from.

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 10 months ago
On 17.06.21 14:17, Peter Krempa wrote:
> On Thu, Jun 17, 2021 at 14:03:44 +0200, David Hildenbrand wrote:
>> On 17.06.21 13:18, Michal Prívozník wrote:
>>> On 6/17/21 11:44 AM, David Hildenbrand wrote:
>>>> On 17.03.21 12:57, Michal Privoznik wrote:
>>>>> v3 of:
>>>>>
>>>>> https://listman.redhat.com/archives/libvir-list/2021-February/msg00961.html
>>>>>
>>>>>
>>>>> diff to v2:
>>>>> - Dropped code that forbade use of virtio-mem and memballoon at the same
>>>>>      time;
>>>>> - This meant that I had to adjust memory accounting,
>>>>>      qemuDomainSetMemoryFlags() - see patches 11/15 and 12/15 which are
>>>>> new.
>>>>> - Fixed small nits raised by Peter in his review of v2
>>>>>
>>>>>
>>>>
>>>> Hi Michal, do you have a branch somewhere that I can easily checkout to
>>>> play with it/test it?
>>>>
>>>
>>> Yes:
>>>
>>> https://gitlab.com/MichalPrivoznik/libvirt/-/tree/virtio_mem_v4
>>>
>>> There were some comments in Peter's review and I really should fix my
>>> code according to them and merge/send v4.
>>>
>>> Michal
>>>
>>
>> Thanks! I started with a single virtio-mem device.
>>
>> 1. NUMA requirement
>>
>> Right now, one can really only configure "maxMemory" with NUMA specified,
>> otherwise there will be "error: unsupported configuration: At least
>> one numa node has to be configured when enabling memory hotplug".
>>
>> I recall this is a limitation of older QEMU which would not create ACPI SRAT
>> tables otherwise. In QEMU, this is no longer the case. As soon as "maxmem"
>> is specified on the QEMU cmdline, we fake a single NUMA node:
>>
>> hw/core/numa.c:numa_complete_configuration()
>>
>> "Enable NUMA implicitly by adding a new NUMA node automatically"
>> -> m->auto_enable_numa_with_memdev / mc->auto_enable_numa_with_memhp
>>
>> m->auto_enable_numa_with_memdev (slots=0) is set since 5.1 on x86-64 and arm64
>> m->auto_enable_numa_with_memhp (slots>0) is set since 2.11 on x86-64 and 4.1 on arm64
>>
>> So in theory, with newer QEMU on x86-64 and arm64 we could drop that
>> limitation in libvirt (might require some changes eventually
>> regarding the "node" specification handling). ppc64 shouldn't care as there is no ACPI.
> 
> The main reason for the check to be present is actually exactly what you
> are describing. qemu fakes the numa node in case none are configured,
> this means that in case where libvirt would not enforce that you'd get a
> discrepancy between the config and what qemu exposes.
> 

Right, but it fakes a single NUMA node just for the purpose of creating 
the ACPI SRAT. (the same thing Linux will do implicitly if there are no 
NUMA nodes, because no NUMA == single NUMA node)

Anyhow, just something I found awkward to run into, because QEMU doesn't 
have this limitation anymore and that handling is properly glued to 
compatibility machines so there is no change when migrating etc. But 
it's certainly more a "nice to have for smaller XML files" :)

I guess auto_enable_numa handling might result in a different QEMU 
behavior (like the result for HMP "info numa" etc.), so if we'd ever 
want to go down that path, we'd have to double check what the effects are.

> 
> [...]
> 
>> 3. "memory" item handling
>>
>> Whenever I edit the XML and set e.g., "<memory unit='GiB'>4</memory>", it's silently converted back to "20 GiB".
>> Maybe that's just always implicitly calculated from the NUMA spec and the defined devices.
> 
> In cases where you've got numa confuigured, the <memory> element is
> re-calculated back from the size of the numa nodes, as when you change
> the value there isn't any obvious algorithm on picking NUMA nodes where
> to pull from.

Makes sense. Another minor thing that is similar to 1. (suboptimal but 
certainly no show stopper)


4. QEMU does no longer require a "slots" specification when maxmem is 
set (because virtio-based memory devices don't require ACPI memory 
module slots).

Specifying "<maxMemory unit='KiB'>20971520</maxMemory>" results in (IMHO 
confusing) error:

   "error: XML document failed to validate against schema: Unable to
    validate doc against /usr/local/share/libvirt/schemas/domain.rng
    Extra element maxMemory in interleave
    Invalid sequence in interleave
    Element domain failed to validate content"


"Extra element maxMemory in interleave
Invalid sequence in interleave
Element domain failed to validate content"

Specifying "<maxMemory slots='0' unit='KiB'>20971520</maxMemory>" results in
   "error: XML error: both maximum memory size and memory slot count must
    be specified"

However, older QEMU version have that requirement. Supported since 3.0 I 
think:

$ git tag --contains 951f2269af2
...
v3.0.0
...

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by Peter Krempa 2 years, 10 months ago
On Thu, Jun 17, 2021 at 14:34:21 +0200, David Hildenbrand wrote:
> On 17.06.21 14:17, Peter Krempa wrote:
> > On Thu, Jun 17, 2021 at 14:03:44 +0200, David Hildenbrand wrote:

[...]

> 
> 4. QEMU does no longer require a "slots" specification when maxmem is set
> (because virtio-based memory devices don't require ACPI memory module
> slots).
> 
> Specifying "<maxMemory unit='KiB'>20971520</maxMemory>" results in (IMHO
> confusing) error:
> 
>   "error: XML document failed to validate against schema: Unable to
>    validate doc against /usr/local/share/libvirt/schemas/domain.rng
>    Extra element maxMemory in interleave
>    Invalid sequence in interleave
>    Element domain failed to validate content"
> 
> 
> "Extra element maxMemory in interleave
> Invalid sequence in interleave
> Element domain failed to validate content"

This is because the XML schema for maxMemory requires the element.
Unfortunately the XML schema validator from libxml2 has extremely
user-unfriendly errors.

> Specifying "<maxMemory slots='0' unit='KiB'>20971520</maxMemory>" results in
>   "error: XML error: both maximum memory size and memory slot count must
>    be specified"

This is the error from the XML parser which has human-crafter errors.

> However, older QEMU version have that requirement. Supported since 3.0 I
> think:
> 
> $ git tag --contains 951f2269af2
> ...
> v3.0.0
> ...

Is there a way how to detect that it's not needed? E.g. via the QMP
schema or such?

In this instance we could perhaps drop it and let qemu report the error
... well if it's reasonable though.

Re: [PATCH v3 00/15] Introduce virtio-mem <memory/> model
Posted by David Hildenbrand 2 years, 10 months ago
On 17.06.21 14:42, Peter Krempa wrote:
> On Thu, Jun 17, 2021 at 14:34:21 +0200, David Hildenbrand wrote:
>> On 17.06.21 14:17, Peter Krempa wrote:
>>> On Thu, Jun 17, 2021 at 14:03:44 +0200, David Hildenbrand wrote:
> 
> [...]
> 
>>
>> 4. QEMU does no longer require a "slots" specification when maxmem is set
>> (because virtio-based memory devices don't require ACPI memory module
>> slots).
>>
>> Specifying "<maxMemory unit='KiB'>20971520</maxMemory>" results in (IMHO
>> confusing) error:
>>
>>    "error: XML document failed to validate against schema: Unable to
>>     validate doc against /usr/local/share/libvirt/schemas/domain.rng
>>     Extra element maxMemory in interleave
>>     Invalid sequence in interleave
>>     Element domain failed to validate content"
>>
>>
>> "Extra element maxMemory in interleave
>> Invalid sequence in interleave
>> Element domain failed to validate content"
> 
> This is because the XML schema for maxMemory requires the element.
> Unfortunately the XML schema validator from libxml2 has extremely
> user-unfriendly errors.
> 
>> Specifying "<maxMemory slots='0' unit='KiB'>20971520</maxMemory>" results in
>>    "error: XML error: both maximum memory size and memory slot count must
>>     be specified"
> 
> This is the error from the XML parser which has human-crafter errors.
> 
>> However, older QEMU version have that requirement. Supported since 3.0 I
>> think:
>>
>> $ git tag --contains 951f2269af2
>> ...
>> v3.0.0
>> ...
> 
> Is there a way how to detect that it's not needed? E.g. via the QMP
> schema or such?
> 
> In this instance we could perhaps drop it and let qemu report the error
> ... well if it's reasonable though.
> 

I guess only implicitly, for example, if virtio-mem-pci or 
virtio-pmem-pci are possible (I assume supported devices can be queried 
via QMP). And without these devices, it doesn't make sense to have 
"slots=0".

virtio-pmem-pci was introduced with v4.1.0, virtio-mem-pci was 
introduced with v5.1.0.

-- 
Thanks,

David / dhildenb