On Tue, Jan 12, 2021 at 20:21:19 +0100, Igor Mammedov wrote: > On Tue, 12 Jan 2021 09:29:50 +0100 > Michal Privoznik <mprivozn@xxxxxxxxxx> wrote: > > > In commit v6.9.0-rc1~450 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 v5.0.0-rc0~75^2~1^2~76 and > > look at function create_default_memdev(). > > > > However, then commit v5.0.0-rc1~11^2~3 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 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. > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201 > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > > --- > > > > I'll replace both occurrences of 'QEMU commit XXX' once QEMU patch is > > merged. > > > > src/qemu/qemu_command.c | 30 ++++++++++++++++--- > > src/qemu/qemu_command.h | 3 +- > > src/qemu/qemu_hotplug.c | 2 +- > > .../hugepages-memaccess3.x86_64-latest.args | 4 +-- > > 4 files changed, 31 insertions(+), 8 deletions(-) > > > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > > index 6f970a3128..b99d4e5faf 100644 > > --- a/src/qemu/qemu_command.c > > +++ b/src/qemu/qemu_command.c ... > > @@ -3122,10 +3127,27 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, > > > > if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) > > return -1; > > + > > + if (systemMemory) > > + useCanonicalPath = false; > > + > > } 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'. > > + * See QEMU commit XXX. > > + */ > > + if (!useCanonicalPath && > > + 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; > > is it possible to do it only for old machine types <= 4.0, to limit hack exposure? We consume machine types as opaque strings, we don't parse them and thus we don't have any ordering on them. Without this telling what <= 4.0 means is impossible. And I don't think we should start doing it, and especially not for limiting this hack as it would be limiting a hack with another one. A reasonable solution would be if we could tell which machine types need (or perhaps don't need) such treatment by probing QEMU for available machine types. Jirka
