Re: [libvirt] [PATCH] fix deadlock when qemu cannot start

When qemu cannot start, we may call qemuProcessStop() twice. We have check whether the vm is running at the beginning of qemuProcessStop() to avoid libvirt deadlock. We call qemuProcessStop() with driver and vm locked. It seems that we can avoid libvirt deadlock. But unfortunately we may unlock driver and vm in the function qemuProcessKill() while vm->def->id is not -1. So qemuProcessStop() will be run twice, and monitor will be freed unexpectedly. So we should set vm->def->id to -1 at the beginning of qemuProcessStop(). --- src/qemu/qemu_process.c | 20 ++++++++++++++------ src/qemu/qemu_process.h | 2 ++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0af3751..44814df 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver, VIR_DEBUG("vm=%s pid=%d flags=%x", vm->def->name, vm->pid, flags); - if (!virDomainObjIsActive(vm)) { - VIR_DEBUG("VM '%s' not active", vm->def->name); - return 0; - } + if (!(flags & VIR_QEMU_PROCESS_KILL_NOCHECK)) + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("VM '%s' not active", vm->def->name); + return 0; + } /* This loop sends SIGTERM (or SIGKILL if flags has * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT), @@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver, return; } + /* + * We may unlock driver and vm in qemuProcessKill(), so the other thread + * can lock driver and vm, and then call qemuProcessStop(). So we should + * set vm->def->id to -1 here to avoid qemuProcessStop() called twice. + */ + vm->def->id = -1; + if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) { /* To not break the normal domain shutdown process, skip the * timestamp log writing if failed on opening log file. */ @@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver, } /* shut it off for sure */ - ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE)); + ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE| + VIR_QEMU_PROCESS_KILL_NOCHECK)); /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); @@ -3892,7 +3901,6 @@ retry: vm->taint = 0; vm->pid = -1; - vm->def->id = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 761db6f..891f7a5 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn, typedef enum { VIR_QEMU_PROCESS_KILL_FORCE = 1 << 0, VIR_QEMU_PROCESS_KILL_NOWAIT = 1 << 1, + VIR_QEMU_PROCESS_KILL_NOCHECK = 1 << 2, /* donot check whether the vm is + running */ } virQemuProcessKillMode; int qemuProcessKill(struct qemud_driver *driver,