From b5a591f73b681baa9b9b1efe38bbf3f7ab704b11 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 12 Apr 2023 10:22:42 +0200 Subject: [PATCH] qemuDomainRemoveChrDevice: Deal with qemuDomainChrRemove() failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When cleaning up after removed device, qemuDomainChrRemove() is called. But this may fail, in which case we successfully ignore the failure and virDomainChrDefFree() the device anyway. While it decreases our memory consumption, it's a bit too far, especially if the next step is 'virsh dumpxml'. Then our memory consumption decreases all the way down to zero as we crash. Signed-off-by: Michal Privoznik Reviewed-by: Ján Tomko --- src/qemu/qemu_hotplug.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a6407f074b..d9a102191f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4695,6 +4695,7 @@ qemuDomainRemoveChrDevice(virQEMUDriver *driver, virObjectEvent *event; g_autofree char *charAlias = NULL; qemuDomainObjPrivate *priv = vm->privateData; + virDomainChrDef *chrRemoved = NULL; int rc = 0; VIR_DEBUG("Removing character device %s from domain %p %s", @@ -4728,17 +4729,24 @@ qemuDomainRemoveChrDevice(virQEMUDriver *driver, if (qemuDomainNamespaceTeardownChardev(vm, chr) < 0) VIR_WARN("Unable to remove chr device from /dev"); - qemuDomainReleaseDeviceAddress(vm, &chr->info); - qemuDomainChrRemove(vm->def, chr); + if (!(chrRemoved = qemuDomainChrRemove(vm->def, chr))) { + /* At this point, we only have bad options. The device + * was successfully removed from QEMU, denied in CGropus, + * etc. and yet, we failed to remove it from domain + * definition. */ + VIR_WARN("Unable to remove chr device from domain definition"); + } else { + qemuDomainReleaseDeviceAddress(vm, &chrRemoved->info); - /* The caller does not emit the event, so we must do it here. Note - * that the event should be reported only after all backend - * teardown is completed. - */ - event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias); - virObjectEventStateQueue(driver->domainEventState, event); + /* The caller does not emit the event, so we must do it here. Note + * that the event should be reported only after all backend + * teardown is completed. + */ + event = virDomainEventDeviceRemovedNewFromObj(vm, chrRemoved->info.alias); + virObjectEventStateQueue(driver->domainEventState, event); - virDomainChrDefFree(chr); + virDomainChrDefFree(chrRemoved); + } return 0; }