[PATCH v3 2/2] qemu: Do not Use canonical path for system memory

[Peter Krempa]

On Wed, Feb 10, 2021 at 14:38:14 +0100, Michal Privoznik wrote:
> In commit 88957116c9d3cb4705380c3702c9d4315fb500bb I've adapted
> libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
> switched to memory-backend-* even for system memory. My claim was
> that that's what QEMU does under the hood anyway. And indeed it
> was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
> look at function create_default_memdev().
> 
> However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
> merged into QEMU. While it was fixing a bug, it also changed the
> create_default_memdev() function in which it started turning off
> use of canonical path (by setting
> "x-use-canonical-path-for-ramblock-id" attribute to false). This
> wasn't documented until QEMU commit XXX. The path affects

s/XXX/actual commit id/

> migration - the same path has to be used on the source and on the
> destination. Therefore, if there is old guest started with '-m X'
> it has "pc.ram" block which doesn't use canonical path and thus
> when migrating to newer QEMU which uses memory-backend-* we have
> to turn off the canonical path explicitly. Otherwise,
> "/objects/pc.ram" path would be expected by QEMU which doesn't
> match the source.
> 
> Ideally, we would need to set it only for some machine types
> (4.0 and older) because newer machine types already do what we
> are doing. However, we treat machine types as opaque strings and
> therefore we don't want to parse nor inspect their versions. But
> then again, newer machine types already do what we are doing in
> this commit, so when old machine types are deprecated and removed
> we can remove our hack and forget it ever happened.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_command.c                       | 35 ++++++++++++++++---
>  src/qemu/qemu_command.h                       |  3 +-
>  src/qemu/qemu_hotplug.c                       |  2 +-
>  .../disk-vhostuser.x86_64-latest.args         |  3 +-
>  .../hugepages-memaccess3.x86_64-latest.args   |  4 +--
>  5 files changed, 38 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 92036d26c0..699d89de1d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c

[...]

> @@ -3148,10 +3156,29 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
>  
>          if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
>              return -1;
> +
> +        if (systemMemory)
> +            disableCanonicalPath = true;
> +
>      } else {
>          backendType = "memory-backend-ram";
>      }
>  
> +    /* This is a terrible hack, but unfortunately there is no better way.
> +     * The replacement for '-m X' argument is not simple '-machine
> +     * memory-backend' and '-object memory-backend-*,size=X' (which was the
> +     * idea). This is because of create_default_memdev() in QEMU sets
> +     * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
> +     * documented in QEMU in qemu-options.hx under 'memory-backend'. Note
> +     * that QEMU consideres 'x-use-canonical-path-for-ramblock-id' stable
> +     * and supported despite the 'x-' prefix.
> +     * See QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9.
> +     */
> +    if (disableCanonicalPath &&
> +        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID) &&
> +        virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id", false, NULL) < 0)
> +        return -1;

I don't remember whether you and Daniel reached some consensus about the
machine type version check, but if the consensus is to always output it
then okay.

>From my side:

Reviewed-by: Peter Krempa <pkrempa at redhat.com>