From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37279) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsXBQ-0002SR-31 for qemu-devel@nongnu.org; Thu, 05 Jun 2014 08:54:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsXBG-0007cB-W4 for qemu-devel@nongnu.org; Thu, 05 Jun 2014 08:54:16 -0400 Received: from mail-wi0-x234.google.com ([2a00:1450:400c:c05::234]:51720) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsXBG-0007bz-LE for qemu-devel@nongnu.org; Thu, 05 Jun 2014 08:54:06 -0400 Received: by mail-wi0-f180.google.com with SMTP id hi2so3381763wib.1 for ; Thu, 05 Jun 2014 05:54:05 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Thu, 5 Jun 2014 14:53:58 +0200 Message-Id: <1401972839-25213-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1401972839-25213-1-git-send-email-pbonzini@redhat.com> References: <1401972839-25213-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH v2 1/2] vl: allow other threads to do qemu_system_vmstop_request List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, stefanha@redhat.com There patch protects vmstop_requested with a lock and introduces qemu_system_vmstop_request_prepare. Together with the new call to qemu_vmstop_requested in vm_start, qemu_system_vmstop_request_prepare avoids a race where the VM could remain stopped even though the iostatus of a block device has already been set (for example). qemu_system_vmstop_request_prepare however also lets the caller thread delay observation of the state change until it has itself communicated that change to the user. This delay avoids any possibility of a wrong reordering of the BLOCK_IO_ERROR event and the subsequent STOP event. Signed-off-by: Paolo Bonzini --- cpus.c | 1 + include/sysemu/sysemu.h | 1 + target-lm32/op_helper.c | 2 +- vl.c | 85 +++++++++++++++++++++++++++++++------------------ 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/cpus.c b/cpus.c index dd7ac13..f566a88 100644 --- a/cpus.c +++ b/cpus.c @@ -1206,6 +1206,7 @@ void cpu_stop_current(void) int vm_stop(RunState state) { if (qemu_in_vcpu_thread()) { + qemu_system_vmstop_request_prepare(); qemu_system_vmstop_request(state); /* * FIXME: should not return to device code in case diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index ba5c7f8..0a2f2bb 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -60,6 +60,7 @@ void qemu_system_powerdown_request(void); void qemu_register_powerdown_notifier(Notifier *notifier); void qemu_system_debug_request(void); void qemu_system_vmstop_request(RunState reason); +void qemu_system_vmstop_request_prepare(void); int qemu_shutdown_requested_get(void); int qemu_reset_requested_get(void); void qemu_system_killed(int signal, pid_t pid); diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c index 40fbed6..25ad09a 100644 --- a/target-lm32/op_helper.c +++ b/target-lm32/op_helper.c @@ -52,7 +52,7 @@ void HELPER(ill)(CPULM32State *env) fprintf(stderr, "VM paused due to illegal instruction. " "Connect a debugger or switch to the monitor console " "to find out more.\n"); - qemu_system_vmstop_request(RUN_STATE_PAUSED); + vm_stop(RUN_STATE_PAUSED); cs->halted = 1; raise_exception(env, EXCP_HALTED); #endif diff --git a/vl.c b/vl.c index 0c15608..229a8d1 100644 --- a/vl.c +++ b/vl.c @@ -567,6 +567,10 @@ static int default_driver_check(QemuOpts *opts, void *opaque) static RunState current_run_state = RUN_STATE_PRELAUNCH; +/* We use RUN_STATE_MAX but any invalid value will do */ +static RunState vmstop_requested = RUN_STATE_MAX; +static QemuMutex vmstop_lock; + typedef struct { RunState from; RunState to; @@ -643,10 +647,11 @@ static void runstate_init(void) const RunStateTransition *p; memset(&runstate_valid_transitions, 0, sizeof(runstate_valid_transitions)); - for (p = &runstate_transitions_def[0]; p->from != RUN_STATE_MAX; p++) { runstate_valid_transitions[p->from][p->to] = true; } + + qemu_mutex_init(&vmstop_lock); } /* This function will abort() on invalid state transitions */ @@ -686,6 +691,54 @@ StatusInfo *qmp_query_status(Error **errp) return info; } +static bool qemu_vmstop_requested(RunState *r) +{ + qemu_mutex_lock(&vmstop_lock); + *r = vmstop_requested; + vmstop_requested = RUN_STATE_MAX; + qemu_mutex_unlock(&vmstop_lock); + return *r < RUN_STATE_MAX; +} + +void qemu_system_vmstop_request_prepare(void) +{ + qemu_mutex_lock(&vmstop_lock); +} + +void qemu_system_vmstop_request(RunState state) +{ + vmstop_requested = state; + qemu_mutex_unlock(&vmstop_lock); + qemu_notify_event(); +} + +void vm_start(void) +{ + RunState requested; + + qemu_vmstop_requested(&requested); + if (runstate_is_running() && requested == RUN_STATE_MAX) { + return; + } + + /* Ensure that a STOP/RESUME pair of events is emitted if a + * vmstop request was pending. The BLOCK_IO_ERROR event, for + * example, according to documentation is always followed by + * the STOP event. + */ + if (runstate_is_running()) { + monitor_protocol_event(QEVENT_STOP, NULL); + } else { + cpu_enable_ticks(); + runstate_set(RUN_STATE_RUNNING); + vm_state_notify(1, RUN_STATE_RUNNING); + resume_all_vcpus(); + } + + monitor_protocol_event(QEVENT_RESUME, NULL); +} + + /***********************************************************/ /* real time host monotonic timer */ @@ -1747,17 +1800,6 @@ void vm_state_notify(int running, RunState state) } } -void vm_start(void) -{ - if (!runstate_is_running()) { - cpu_enable_ticks(); - runstate_set(RUN_STATE_RUNNING); - vm_state_notify(1, RUN_STATE_RUNNING); - resume_all_vcpus(); - monitor_protocol_event(QEVENT_RESUME, NULL); - } -} - /* reset/shutdown handler */ typedef struct QEMUResetEntry { @@ -1782,7 +1824,6 @@ static NotifierList suspend_notifiers = static NotifierList wakeup_notifiers = NOTIFIER_LIST_INITIALIZER(wakeup_notifiers); static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE); -static RunState vmstop_requested = RUN_STATE_MAX; int qemu_shutdown_requested_get(void) { @@ -1850,18 +1891,6 @@ static int qemu_debug_requested(void) return r; } -/* We use RUN_STATE_MAX but any invalid value will do */ -static bool qemu_vmstop_requested(RunState *r) -{ - if (vmstop_requested < RUN_STATE_MAX) { - *r = vmstop_requested; - vmstop_requested = RUN_STATE_MAX; - return true; - } - - return false; -} - void qemu_register_reset(QEMUResetHandler *func, void *opaque) { QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry)); @@ -2011,12 +2040,6 @@ void qemu_system_debug_request(void) qemu_notify_event(); } -void qemu_system_vmstop_request(RunState state) -{ - vmstop_requested = state; - qemu_notify_event(); -} - static bool main_loop_should_exit(void) { RunState r; -- 1.8.3.1