Re: [PATCH v2] qemu: Don't generate '-machine memory-backend' and '-numa memdev'

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 10/7/20 5:01 AM, Michal Privoznik wrote:
> In 88957116c9 I've switched to -machine memory-backend=ID and
> -object memory-backend-* because QEMU is obsoleting -mem-path
> and -mem-prealloc. However, what I did not foresee was that using
> -machine memory-backend in combination with -numa is not allowed
> in QEMU. This was reported upstream and fortunately not released
> yet.
>
> The problem is that if domain has NUMA nodes then we will
> generate memory-backend-* objects for NUMA nodes (because if QEMU
> is new enough to expose default RAM ID it also supports -numa
> memdev=) and adding non-NUMA memory backend is wrong.
>
> Reported-by: Masayoshi Mizuma <msys.mizuma@xxxxxxxxx>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>   src/qemu/qemu_command.c                       | 32 +++++++++++++++----
>   .../hugepages-nvdimm.x86_64-latest.args       |  6 +---
>   ...memory-default-hugepage.x86_64-latest.args |  5 +--
>   .../memfd-memory-numa.x86_64-latest.args      |  5 +--
>   ...y-hotplug-nvdimm-access.x86_64-latest.args |  4 +--
>   ...ry-hotplug-nvdimm-align.x86_64-latest.args |  4 +--
>   ...ry-hotplug-nvdimm-label.x86_64-latest.args |  4 +--
>   ...ory-hotplug-nvdimm-pmem.x86_64-latest.args |  4 +--
>   ...hotplug-nvdimm-readonly.x86_64-latest.args |  4 +--
>   .../memory-hotplug-nvdimm.x86_64-latest.args  |  4 +--
>   .../numatune-hmat.x86_64-latest.args          |  4 +--
>   ...vhost-user-fs-fd-memory.x86_64-latest.args |  4 +--
>   ...vhost-user-fs-hugepages.x86_64-latest.args |  5 +--
>   ...host-user-gpu-secondary.x86_64-latest.args |  3 +-
>   .../vhost-user-vga.x86_64-latest.args         |  3 +-
>   15 files changed, 39 insertions(+), 52 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 476cf6972e..c347f80f08 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6829,7 +6829,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>       virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
>       virTristateSwitch smm = def->features[VIR_DOMAIN_FEATURE_SMM];
>       virCPUDefPtr cpu = def->cpu;
> -    const char *defaultRAMid = NULL;
>       g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>       size_t i;
>   
> @@ -7046,11 +7045,24 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>       if (virDomainNumaHasHMAT(def->numa))
>           virBufferAddLit(&buf, ",hmat=on");
>   
> -    defaultRAMid = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
> -                                                     def->virtType,
> -                                                     def->os.machine);
> -    if (defaultRAMid)
> -        virBufferAsprintf(&buf, ",memory-backend=%s", defaultRAMid);
> +    if (!virDomainNumaGetNodeCount(def->numa)) {
> +        const char *defaultRAMid = NULL;
> +
> +        /* QEMU is obsoleting -mem-path and -mem-prealloc. That means we have
> +         * to switch to memory-backend-* even for regular RAM and to keep
> +         * domain migratable we have to set the same ID as older QEMUs would.
> +         * If domain has no NUMA nodes and QEMU is new enough to expose ID of
> +         * the default RAM we want to use it for default RAM (construct
> +         * memory-backend-* with corresponding attributes instead of obsolete
> +         * -mem-path and -mem-prealloc.

I think you're missing a closing ')' in this sentence up here ^

Everything else LGTM.

Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>


> +         * This generates only reference for the memory-backend-* object added
> +         * later in qemuBuildMemCommandLine() */
> +        defaultRAMid = virQEMUCapsGetMachineDefaultRAMid(qemuCaps,
> +                                                         def->virtType,
> +                                                         def->os.machine);
> +        if (defaultRAMid)
> +            virBufferAsprintf(&buf, ",memory-backend=%s", defaultRAMid);
> +    }
>   
>       virCommandAddArgBuffer(cmd, &buf);
>   
> @@ -7216,7 +7228,13 @@ qemuBuildMemCommandLine(virCommandPtr cmd,
>                                                        def->os.machine);
>   
>       if (defaultRAMid) {
> -        qemuBuildMemCommandLineMemoryDefaultBackend(cmd, def, priv, defaultRAMid);
> +        /* As documented in qemuBuildMachineCommandLine() if QEMU is new enough
> +         * to expose default RAM ID we must use memory-backend-* even for
> +         * regular memory because -mem-path and -mem-prealloc are obsolete.
> +         * However, if domain has one or more NUMA nodes then there is no
> +         * default RAM and we mustn't generate the memory object. */
> +        if (!virDomainNumaGetNodeCount(def->numa))
> +            qemuBuildMemCommandLineMemoryDefaultBackend(cmd, def, priv, defaultRAMid);
>       } else {
>           if (def->mem.allocation == VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE) {
>               virCommandAddArgList(cmd, "-mem-prealloc", NULL);
> diff --git a/tests/qemuxml2argvdata/hugepages-nvdimm.x86_64-latest.args b/tests/qemuxml2argvdata/hugepages-nvdimm.x86_64-latest.args
> index 192b033e9e..969bb713b5 100644
> --- a/tests/qemuxml2argvdata/hugepages-nvdimm.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/hugepages-nvdimm.x86_64-latest.args
> @@ -12,13 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=1048576k,slots=16,maxmem=1099511627776k \
> --object memory-backend-file,id=pc.ram,\
> -mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,prealloc=yes,\
> -size=1073741824 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-file,id=ram-node0,\
> diff --git a/tests/qemuxml2argvdata/memfd-memory-default-hugepage.x86_64-latest.args b/tests/qemuxml2argvdata/memfd-memory-default-hugepage.x86_64-latest.args
> index b43e7d9c3c..ef15e2e97e 100644
> --- a/tests/qemuxml2argvdata/memfd-memory-default-hugepage.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memfd-memory-default-hugepage.x86_64-latest.args
> @@ -12,12 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-instance-00000092/master-key.aes \
> --machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off,\
> -memory-backend=pc.ram \
> +-machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 14336 \
> --object memory-backend-memfd,id=pc.ram,hugetlb=yes,hugetlbsize=2097152,\
> -share=yes,prealloc=yes,size=15032385536 \
>   -overcommit mem-lock=off \
>   -smp 8,sockets=1,dies=1,cores=8,threads=1 \
>   -object memory-backend-memfd,id=ram-node0,hugetlb=yes,hugetlbsize=2097152,\
> diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
> index b43e7d9c3c..ef15e2e97e 100644
> --- a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
> @@ -12,12 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-instance-00000092/master-key.aes \
> --machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off,\
> -memory-backend=pc.ram \
> +-machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 14336 \
> --object memory-backend-memfd,id=pc.ram,hugetlb=yes,hugetlbsize=2097152,\
> -share=yes,prealloc=yes,size=15032385536 \
>   -overcommit mem-lock=off \
>   -smp 8,sockets=1,dies=1,cores=8,threads=1 \
>   -object memory-backend-memfd,id=ram-node0,hugetlb=yes,hugetlbsize=2097152,\
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-access.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-access.x86_64-latest.args
> index c5c5befdd5..8d3fa2e867 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-access.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-access.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=219136k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.x86_64-latest.args
> index 418bf1158d..4f22f78277 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=219136k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-label.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-label.x86_64-latest.args
> index 66477a2b09..53dda234a2 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-label.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-label.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=219136k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
> index 759d749c2e..cac02a6f6d 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=219136k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-readonly.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-readonly.x86_64-latest.args
> index b195b8625c..5e44496e39 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-readonly.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-readonly.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=219136k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm.x86_64-latest.args
> index ae416d7c7f..94f8444d95 100644
> --- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \
>   -cpu qemu64 \
>   -m size=1048576k,slots=16,maxmem=1099511627776k \
> --object memory-backend-ram,id=pc.ram,size=1073741824 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,dies=1,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=1073741824 \
> diff --git a/tests/qemuxml2argvdata/numatune-hmat.x86_64-latest.args b/tests/qemuxml2argvdata/numatune-hmat.x86_64-latest.args
> index be4a9ed871..c52015caa8 100644
> --- a/tests/qemuxml2argvdata/numatune-hmat.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/numatune-hmat.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,hmat=on,\
> -memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,hmat=on \
>   -cpu qemu64 \
>   -m 12288 \
> --object memory-backend-ram,id=pc.ram,size=12884901888 \
>   -overcommit mem-lock=off \
>   -smp 12,sockets=12,cores=1,threads=1 \
>   -object memory-backend-ram,id=ram-node0,size=2147483648 \
> diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
> index 13359c91fe..dd5f68abc5 100644
> --- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
> @@ -12,11 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-guest/master-key.aes \
> --machine pc,accel=kvm,usb=off,dump-guest-core=off,memory-backend=pc.ram \
> +-machine pc,accel=kvm,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 14336 \
> --object memory-backend-file,id=pc.ram,\
> -mem-path=/var/lib/libvirt/qemu/ram/-1-guest/pc.ram,share=yes,size=15032385536 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,cores=1,threads=1 \
>   -object memory-backend-file,id=ram-node0,\
> diff --git a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args
> index 59f45d466b..e4f5db7a63 100644
> --- a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args
> @@ -12,12 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-guest/master-key.aes \
> --machine q35,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \
> +-machine q35,accel=tcg,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 2048 \
> --object memory-backend-file,id=pc.ram,\
> -mem-path=/dev/hugepages2M/libvirt/qemu/-1-guest,share=yes,prealloc=yes,\
> -size=2147483648 \
>   -overcommit mem-lock=off \
>   -smp 2,sockets=2,cores=1,threads=1 \
>   -object memory-backend-file,id=ram-node0,\
> diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
> index e7d3315b8f..e99a5342dc 100644
> --- a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args
> @@ -12,10 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 214 \
> --object memory-backend-memfd,id=pc.ram,share=yes,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 1,sockets=1,cores=1,threads=1 \
>   -object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \
> diff --git a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
> index df5b1b481d..277bf8c646 100644
> --- a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
> +++ b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args
> @@ -12,10 +12,9 @@ QEMU_AUDIO_DRV=none \
>   -S \
>   -object secret,id=masterKey0,format=raw,\
>   file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> --machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off \
>   -cpu qemu64 \
>   -m 214 \
> --object memory-backend-memfd,id=pc.ram,share=yes,size=224395264 \
>   -overcommit mem-lock=off \
>   -smp 1,sockets=1,cores=1,threads=1 \
>   -object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \