[PATCH 0/3] qemu: Fix alias generation for 'dimm' devices
Peter Krempa (3): qemu: hotplug: Remove legacy quirk for 'dimm' address generation qemu: alias: Remove 'oldAlias' argument of qemuAssignDeviceMemoryAlias qemu: Remove 'memAliasOrderMismatch' field from VM private data src/qemu/qemu_alias.c | 13 ++++--------- src/qemu/qemu_alias.h | 3 +-- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_hotplug.c | 4 +--- src/qemu/qemu_process.c | 24 ------------------------ 5 files changed, 6 insertions(+), 41 deletions(-) -- 2.38.1
Commit b7798a07f93 (in fall of 2016) changed the way we generate aliases for 'dimm' memory devices as the alias itself is part of the migration stream section naming and thus must be treated as ABI. The code added compatibility layer for VMs with memory hotplug started with the old scheme to prevent from generating wrong aliases. The compatibility layer broke though later when 'nvdimm' and 'pmem' devices were introduced as it wrongly detected them as old configuration. Now rather than attempting to fix the legacy compat layer to treat other devices properly we'll be better off simply removing it as it's extremely unlikely that somebody has a VM started in 2016 running with today's libvirt and attempts to hotplug more memory. This fixes a corner case when a user hot-adds a 'dimm' into a VM with a 'dimm' and a 'nvdimm' after restart of libvirtd and then attempts to migrate the VM. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2158701 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 026e1ee5ad..5840504d13 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2275,9 +2275,7 @@ qemuDomainAttachMemory(virQEMUDriver *driver, goto cleanup; releaseaddr = true; - /* in cases where we are using a VM with aliases generated according to the - * index of the memory device we need to keep continue using that scheme */ - if (qemuAssignDeviceMemoryAlias(vm->def, mem, priv->memAliasOrderMismatch) < 0) + if (qemuAssignDeviceMemoryAlias(vm->def, mem, false) < 0) goto cleanup; objalias = g_strdup_printf("mem%s", mem->info.alias); -- 2.38.1
All callers pass 'false' so we no longer need it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_alias.c | 13 ++++--------- src/qemu/qemu_alias.h | 3 +-- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index ef8e87ab58..0f1310a0e5 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -454,7 +454,6 @@ qemuAssignDeviceRNGAlias(virDomainDef *def, static int qemuDeviceMemoryGetAliasID(virDomainDef *def, virDomainMemoryDef *mem, - bool oldAlias, const char *prefix) { size_t i; @@ -462,8 +461,7 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def, /* virtio-pmem and virtio-mem go onto PCI bus and thus DIMM address is not * valid */ - if (!oldAlias && - mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM && + if (mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM && mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM && mem->model != VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) return mem->info.addr.dimm.slot; @@ -482,8 +480,6 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def, * qemuAssignDeviceMemoryAlias: * @def: domain definition. Necessary only if @oldAlias is true. * @mem: memory device definition - * @oldAlias: Generate the alias according to the order of the device in @def - * rather than according to the slot number for legacy reasons. * * Generates alias for a memory device according to slot number if @oldAlias is * false or according to order in @def->mems otherwise. @@ -492,8 +488,7 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def, */ int qemuAssignDeviceMemoryAlias(virDomainDef *def, - virDomainMemoryDef *mem, - bool oldAlias) + virDomainMemoryDef *mem) { const char *prefix = NULL; int idx = 0; @@ -525,7 +520,7 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def, break; } - idx = qemuDeviceMemoryGetAliasID(def, mem, oldAlias, prefix); + idx = qemuDeviceMemoryGetAliasID(def, mem, prefix); mem->info.alias = g_strdup_printf("%s%d", prefix, idx); return 0; @@ -685,7 +680,7 @@ qemuAssignDeviceAliases(virDomainDef *def) qemuAssignDeviceTPMAlias(def->tpms[i], i); } for (i = 0; i < def->nmems; i++) { - if (qemuAssignDeviceMemoryAlias(def, def->mems[i], false) < 0) + if (qemuAssignDeviceMemoryAlias(def, def->mems[i]) < 0) return -1; } if (def->vsock) { diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 6433ae4cec..af9c3f62d3 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -55,8 +55,7 @@ void qemuAssignDeviceRNGAlias(virDomainDef *def, virDomainRNGDef *rng); int qemuAssignDeviceMemoryAlias(virDomainDef *def, - virDomainMemoryDef *mems, - bool oldAlias); + virDomainMemoryDef *mems); void qemuAssignDeviceShmemAlias(virDomainDef *def, virDomainShmemDef *shmem, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5840504d13..2df59873db 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2275,7 +2275,7 @@ qemuDomainAttachMemory(virQEMUDriver *driver, goto cleanup; releaseaddr = true; - if (qemuAssignDeviceMemoryAlias(vm->def, mem, false) < 0) + if (qemuAssignDeviceMemoryAlias(vm->def, mem) < 0) goto cleanup; objalias = g_strdup_printf("mem%s", mem->info.alias); -- 2.38.1
The field is no longer used so we can remove it and the code filling it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_process.c | 24 ------------------------ 2 files changed, 27 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 08430b67b9..eca5404cdc 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -177,9 +177,6 @@ struct _qemuDomainObjPrivate { uint8_t *masterKey; size_t masterKeyLen; - /* note whether memory device alias does not correspond to slot number */ - bool memAliasOrderMismatch; - /* for migrations using TLS with a secret (not to be saved in our */ /* private XML). */ qemuDomainSecretInfo *migSecinfo; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ee9f0784d3..29716ecb19 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3896,28 +3896,6 @@ qemuDomainPerfRestart(virDomainObj *vm) } -static void -qemuProcessReconnectCheckMemAliasOrderMismatch(virDomainObj *vm) -{ - size_t i; - int aliasidx; - virDomainDef *def = vm->def; - qemuDomainObjPrivate *priv = vm->privateData; - - if (!virDomainDefHasMemoryHotplug(def) || def->nmems == 0) - return; - - for (i = 0; i < def->nmems; i++) { - aliasidx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm"); - - if (def->mems[i]->info.addr.dimm.slot != aliasidx) { - priv->memAliasOrderMismatch = true; - break; - } - } -} - - static bool qemuProcessDomainMemoryDefNeedHugepagesPath(const virDomainMemoryDef *mem, const long system_pagesize) @@ -9091,8 +9069,6 @@ qemuProcessReconnect(void *opaque) if (qemuProcessRefreshFdsetIndex(obj) < 0) goto error; - qemuProcessReconnectCheckMemAliasOrderMismatch(obj); - if (qemuConnectAgent(driver, obj) < 0) goto error; -- 2.38.1
On Thu, Jan 19, 2023 at 03:27:51PM +0100, Peter Krempa wrote:
Peter Krempa (3): qemu: hotplug: Remove legacy quirk for 'dimm' address generation qemu: alias: Remove 'oldAlias' argument of qemuAssignDeviceMemoryAlias qemu: Remove 'memAliasOrderMismatch' field from VM private data
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
src/qemu/qemu_alias.c | 13 ++++--------- src/qemu/qemu_alias.h | 3 +-- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_hotplug.c | 4 +--- src/qemu/qemu_process.c | 24 ------------------------ 5 files changed, 6 insertions(+), 41 deletions(-)
-- 2.38.1
participants (2)
-
Martin Kletzander -
Peter Krempa