From 8c9ff9960b29d4703a99efdd1cadcf6f48799cc0 Mon Sep 17 00:00:00 2001 From: Jiri Denemark Date: Thu, 11 Feb 2016 15:32:48 +0100 Subject: [PATCH] qemu: Process monitor EOF in a job Stopping a domain without a job risks a race condition with another thread which started a job a which does not expect anyone else to be messing around with the same domain object. Signed-off-by: Jiri Denemark --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 14 +++++++---- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_process.c | 49 ++++++++++++++------------------------- 5 files changed, 80 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29f5864393..8359b1afd0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -234,6 +234,7 @@ typedef enum { QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED, QEMU_PROCESS_EVENT_SERIAL_CHANGED, QEMU_PROCESS_EVENT_BLOCK_JOB, + QEMU_PROCESS_EVENT_MONITOR_EOF, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index de36685153..8b8cb1a97c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4537,13 +4537,59 @@ processBlockJobEvent(virQEMUDriverPtr driver, } +static void +processMonitorEOFEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int eventReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN; + int stopReason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; + const char *auditReason = "shutdown"; + unsigned int stopFlags = 0; + virObjectEventPtr event = NULL; + + if (qemuProcessBeginStopJob(driver, vm, QEMU_JOB_DESTROY, true) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain %p '%s' is not active, ignoring EOF", + vm, vm->def->name); + goto endjob; + } + + if (priv->monJSON && !priv->gotShutdown) { + VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; " + "assuming the domain crashed", vm->def->name); + eventReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; + stopReason = VIR_DOMAIN_SHUTOFF_CRASHED; + auditReason = "failed"; + } + + if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) { + stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; + qemuMigrationErrorSave(driver, vm->def->name, + qemuMonitorLastError(priv->mon)); + } + + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + eventReason); + qemuProcessStop(driver, vm, stopReason, stopFlags); + virDomainAuditStop(vm, auditReason); + qemuDomainEventQueue(driver, event); + + endjob: + qemuDomainObjEndJob(driver, vm); + qemuDomainRemoveInactive(driver, vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; virDomainObjPtr vm = processEvent->vm; virQEMUDriverPtr driver = opaque; - VIR_DEBUG("vm=%p", vm); + VIR_DEBUG("vm=%p, event=%d", vm, processEvent->eventType); virObjectLock(vm); @@ -4570,6 +4616,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) processEvent->action, processEvent->status); break; + case QEMU_PROCESS_EVENT_MONITOR_EOF: + processMonitorEOFEvent(driver, vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 58a74754fe..cb87412a8d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -914,6 +914,15 @@ qemuMonitorOpenFD(virDomainObjPtr vm, } +void +qemuMonitorUnregister(qemuMonitorPtr mon) +{ + if (mon->watch) { + virEventRemoveHandle(mon->watch); + mon->watch = 0; + } +} + void qemuMonitorClose(qemuMonitorPtr mon) { @@ -927,10 +936,7 @@ qemuMonitorClose(qemuMonitorPtr mon) qemuMonitorSetDomainLog(mon, NULL, NULL, NULL); if (mon->fd >= 0) { - if (mon->watch) { - virEventRemoveHandle(mon->watch); - mon->watch = 0; - } + qemuMonitorUnregister(mon); VIR_FORCE_CLOSE(mon->fd); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d1a667aec6..6a2a985ad8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -244,6 +244,8 @@ qemuMonitorPtr qemuMonitorOpenFD(virDomainObjPtr vm, void *opaque) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void qemuMonitorUnregister(qemuMonitorPtr mon) + ATTRIBUTE_NONNULL(1); void qemuMonitorClose(qemuMonitorPtr mon); virErrorPtr qemuMonitorLastError(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fd301c3e1f..a372c3da1e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -286,54 +286,39 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, void *opaque) { virQEMUDriverPtr driver = opaque; - virObjectEventPtr event = NULL; qemuDomainObjPrivatePtr priv; - int eventReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN; - int stopReason = VIR_DOMAIN_SHUTOFF_SHUTDOWN; - const char *auditReason = "shutdown"; - unsigned int stopFlags = 0; - - VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); + struct qemuProcessEvent *processEvent; virObjectLock(vm); - priv = vm->privateData; + VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); + priv = vm->privateData; if (priv->beingDestroyed) { VIR_DEBUG("Domain is being destroyed, EOF is expected"); goto cleanup; } - if (!virDomainObjIsActive(vm)) { - VIR_DEBUG("Domain %p is not active, ignoring EOF", vm); + if (VIR_ALLOC(processEvent) < 0) + goto cleanup; + + processEvent->eventType = QEMU_PROCESS_EVENT_MONITOR_EOF; + processEvent->vm = vm; + + virObjectRef(vm); + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + ignore_value(virObjectUnref(vm)); + VIR_FREE(processEvent); goto cleanup; } - if (priv->monJSON && !priv->gotShutdown) { - VIR_DEBUG("Monitor connection to '%s' closed without SHUTDOWN event; " - "assuming the domain crashed", vm->def->name); - eventReason = VIR_DOMAIN_EVENT_STOPPED_FAILED; - stopReason = VIR_DOMAIN_SHUTOFF_CRASHED; - auditReason = "failed"; - } - - if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) { - stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; - qemuMigrationErrorSave(driver, vm->def->name, - qemuMonitorLastError(priv->mon)); - } - - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_STOPPED, - eventReason); - qemuProcessStop(driver, vm, stopReason, stopFlags); - virDomainAuditStop(vm, auditReason); - - qemuDomainRemoveInactive(driver, vm); + /* We don't want this EOF handler to be called over and over while the + * thread is waiting for a job. + */ + qemuMonitorUnregister(mon); cleanup: virObjectUnlock(vm); - qemuDomainEventQueue(driver, event); }