From 213189ab65d83ecd9072f27c80a15dcb91b6bdbf Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 28 Jul 2009 14:33:41 -0400 Subject: Fix VM state change handlers running out of order When a VM state change handler changes VM state, other VM state change handlers can see the state transitions out of order. bmdma_map(), scsi_disk_init() and virtio_blk_init() install VM state change handlers to restart DMA. These handlers can vm_stop() by running into a write error on a drive with werror=stop. This throws the VM state change handler callback into disarray. Here's an example case I observed: 0. The virtual IDE drive goes south. All future writes return errors. 1. Something encounters a write error, and duly stops the VM with vm_stop(). 2. vm_stop() calls vm_state_notify(0). 3. vm_state_notify() runs the callbacks in list vm_change_state_head. It contains ide_dma_restart_cb() installed by bmdma_map(). It also contains audio_vm_change_state_handler() installed by audio_init(). 4. audio_vm_change_state_handler() stops audio stuff. 5. User continues VM with monitor command "c". This runs vm_start(). 6. vm_start() calls vm_state_notify(1). 7. vm_state_notify() runs the callbacks in vm_change_state_head. 8. ide_dma_restart_cb() happens to come first. It does its work, runs into a write error, and duly stops the VM with vm_stop(). 9. vm_stop() runs vm_state_notify(0). 10. vm_state_notify() runs the callbacks in vm_change_state_head. 11. audio_vm_change_state_handler() stops audio stuff. Which isn't running. 12. vm_stop() finishes, ide_dma_restart_cb() finishes, step 7's vm_state_notify() resumes running handlers. 13. audio_vm_change_state_handler() starts audio stuff. Oopsie. Fix this by moving the actual write from each VM state change handler into a new bottom half (suggested by Gleb Natapov). Signed-off-by: Markus Armbruster Signed-off-by: Anthony Liguori --- hw/scsi-disk.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'hw/scsi-disk.c') diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 8b6426fd8..5b825c9f7 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -74,6 +74,7 @@ struct SCSIDeviceState scsi_completionfn completion; void *opaque; char drive_serial_str[21]; + QEMUBH *bh; }; /* Global pool of SCSIRequest structures. */ @@ -308,12 +309,13 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag) return 0; } -static void scsi_dma_restart_cb(void *opaque, int running, int reason) +static void scsi_dma_restart_bh(void *opaque) { SCSIDeviceState *s = opaque; SCSIRequest *r = s->requests; - if (!running) - return; + + qemu_bh_delete(s->bh); + s->bh = NULL; while (r) { if (r->status & SCSI_REQ_STATUS_RETRY) { @@ -324,6 +326,19 @@ static void scsi_dma_restart_cb(void *opaque, int running, int reason) } } +static void scsi_dma_restart_cb(void *opaque, int running, int reason) +{ + SCSIDeviceState *s = opaque; + + if (!running) + return; + + if (!s->bh) { + s->bh = qemu_bh_new(scsi_dma_restart_bh, s); + qemu_bh_schedule(s->bh); + } +} + /* Return a pointer to the data buffer. */ static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag) { -- cgit v1.2.3