[libvirt] [PATCH 0/2] Fix alias asignment code for hotplugged RNGs/memory
I messed up the original impl and then copied it to memory devices. Sigh. Peter Krempa (2): qemu: alias: Fix calculation of RNG device aliases qemu: alias: Fix calculation of memory device aliases src/qemu/qemu_alias.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_alias.h | 7 ++++++- src/qemu/qemu_hotplug.c | 4 ++-- 3 files changed, 53 insertions(+), 5 deletions(-) -- 2.8.0
For device hotplug, the new alias ID needs to be checked in the list rather than using the count of devices. Unplugging a device that is not last in the array will make further hotplug impossible due to alias collision. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1324551 --- src/qemu/qemu_alias.c | 20 ++++++++++++++++++-- src/qemu/qemu_alias.h | 3 ++- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 010d6b9..052a829 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -382,9 +382,25 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, int -qemuAssignDeviceRNGAlias(virDomainRNGDefPtr rng, +qemuAssignDeviceRNGAlias(virDomainDefPtr def, + virDomainRNGDefPtr rng, size_t idx) { + if (idx == -1) { + size_t i; + idx = 0; + for (i = 0; i < def->nrngs; i++) { + int thisidx; + if ((thisidx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info, "rng")) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to determine device index for rng device")); + return -1; + } + if (thisidx >= idx) + idx = thisidx + 1; + } + } + if (virAsprintf(&rng->info.alias, "rng%zu", idx) < 0) return -1; @@ -480,7 +496,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return -1; } for (i = 0; i < def->nrngs; i++) { - if (qemuAssignDeviceRNGAlias(def->rngs[i], i) < 0) + if (virAsprintf(&def->rngs[i]->info.alias, "rng%zu", i) < 0) return -1; } if (def->tpm) { diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index b915f73..f311583 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -54,7 +54,8 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, int idx); -int qemuAssignDeviceRNGAlias(virDomainRNGDefPtr rng, +int qemuAssignDeviceRNGAlias(virDomainDefPtr def, + virDomainRNGDefPtr rng, size_t idx); int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 48bea6a..93cc8e2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1625,7 +1625,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, const char *type; int ret = -1; - if (qemuAssignDeviceRNGAlias(rng, vm->def->nrngs) < 0) + if (qemuAssignDeviceRNGAlias(vm->def, rng, -1) < 0) return -1; /* preallocate space for the device definition */ -- 2.8.0
On Wed, Apr 06, 2016 at 05:52:48PM +0200, Peter Krempa wrote:
For device hotplug, the new alias ID needs to be checked in the list rather than using the count of devices. Unplugging a device that is not last in the array will make further hotplug impossible due to alias collision.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1324551 --- src/qemu/qemu_alias.c | 20 ++++++++++++++++++-- src/qemu/qemu_alias.h | 3 ++- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 010d6b9..052a829 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -382,9 +382,25 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
int -qemuAssignDeviceRNGAlias(virDomainRNGDefPtr rng, +qemuAssignDeviceRNGAlias(virDomainDefPtr def, + virDomainRNGDefPtr rng, size_t idx) { + if (idx == -1) { + size_t i; + idx = 0; + for (i = 0; i < def->nrngs; i++) { + int thisidx; + if ((thisidx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info, "rng")) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to determine device index for rng device")); + return -1;
For net and hostdev alias this was updated by recent commit 8f74f527 to ignore that fact and just continue with next device. This error isn't required since we are trying only to create a non-collision alias matching 'rng[0-9]+'. ACK with that fixed. Pavle
On Wed, Apr 06, 2016 at 18:14:22 +0200, Pavel Hrdina wrote:
On Wed, Apr 06, 2016 at 05:52:48PM +0200, Peter Krempa wrote:
For device hotplug, the new alias ID needs to be checked in the list rather than using the count of devices. Unplugging a device that is not last in the array will make further hotplug impossible due to alias collision.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1324551 ---
..
ACK with that fixed.
With the proposed change and removal of the @idx parameter which is always -1 the code could be simplified. I've pushed the patches. Thanks. Peter
For device hotplug, the new alias ID needs to be checked in the list rather than using the count of devices. Unplugging a device that is not last in the array will make further hotplug impossible due to alias collision. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1324551 --- src/qemu/qemu_alias.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_alias.h | 4 ++++ src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 052a829..c56e5e4 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -409,6 +409,33 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def, int +qemuAssignDeviceMemoryAlias(virDomainDefPtr def, + virDomainMemoryDefPtr mem, + size_t idx) +{ + if (idx == -1) { + size_t i; + idx = 0; + for (i = 0; i < def->nmems; i++) { + int thisidx; + if ((thisidx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, "dimm")) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to determine device index for memory device")); + return -1; + } + if (thisidx >= idx) + idx = thisidx + 1; + } + } + + if (virAsprintf(&mem->info.alias, "dimm%zu", idx) < 0) + return -1; + + return 0; +} + + +int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { size_t i; diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index f311583..ba91d32 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -58,6 +58,10 @@ int qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng, size_t idx); +int qemuAssignDeviceMemoryAlias(virDomainDefPtr def, + virDomainMemoryDefPtr mems, + size_t idx); + int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 93cc8e2..010eccd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1751,7 +1751,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) < 0) goto cleanup; - if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0) + if (qemuAssignDeviceMemoryAlias(vm->def, mem, -1) < 0) goto cleanup; if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) -- 2.8.0
On Wed, Apr 06, 2016 at 05:52:49PM +0200, Peter Krempa wrote:
For device hotplug, the new alias ID needs to be checked in the list rather than using the count of devices. Unplugging a device that is not last in the array will make further hotplug impossible due to alias collision.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1324551 --- src/qemu/qemu_alias.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_alias.h | 4 ++++ src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 32 insertions(+), 1 deletion(-)
ACK with the same update as for first patch. Pavel
participants (2)
-
Pavel Hrdina -
Peter Krempa