From d9935a5c4fa5b9ca4b25b1b6b31f9391216fdfd0 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Thu, 6 Jun 2024 17:43:12 +0200 Subject: [PATCH] qemu: process: Ensure that 'beingDestroyed' gets cleared only after VM id is reset Prevent the possibility that a VM could be considered as alive while inside qemuProcessStop. A recently fixed bug which unlocked the domain object while inside qemuProcessStop showed that there's possibility to confuse the state of the VM to be considered active while 'qemuProcessStop' is processing shutdown of the VM. Ensure that this doesn't happen by clearing the 'beingDestroyed' flag only after the VM id is cleared. Signed-off-by: Peter Krempa Reviewed-by: Michal Privoznik --- src/qemu/qemu_process.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a7cbda16e..ae6594e10e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8419,29 +8419,31 @@ qemuProcessBeginStopJob(virDomainObj *vm, { qemuDomainObjPrivate *priv = vm->privateData; unsigned int killFlags = forceKill ? VIR_QEMU_PROCESS_KILL_FORCE : 0; - int ret = -1; /* We need to prevent monitor EOF callback from doing our work (and * sending misleading events) while the vm is unlocked inside - * BeginJob/ProcessKill API - */ + * BeginJob/ProcessKill API or any other code path before 'vm->def->id' is + * cleared inside qemuProcessStop */ priv->beingDestroyed = true; if (qemuProcessKill(vm, killFlags) < 0) - goto cleanup; + goto error; /* Wake up anything waiting on domain condition */ VIR_DEBUG("waking up all jobs waiting on the domain condition"); virDomainObjBroadcast(vm); if (virDomainObjBeginJob(vm, job) < 0) - goto cleanup; + goto error; - ret = 0; + /* priv->beingDestroyed is deliberately left set to 'true' here. Caller + * is supposed to call qemuProcessStop, which will reset it after + * 'vm->def->id' is set to -1 */ + return 0; - cleanup: + error: priv->beingDestroyed = false; - return ret; + return -1; } @@ -8528,7 +8530,13 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDBusStop(driver, vm); + /* Only after this point we can reset 'priv->beingDestroyed' so that + * there's no point at which the VM could be considered as alive between + * entering the destroy job and this point where the active "flag" is + * cleared. + */ vm->def->id = -1; + priv->beingDestroyed = false; /* Wake up anything waiting on domain condition */ virDomainObjBroadcast(vm); -- 2.39.5