diff options
author | Markus Armbruster <armbru@redhat.com> | 2009-07-28 14:33:41 -0400 |
---|---|---|
committer | Anthony Liguori <aliguori@us.ibm.com> | 2009-08-27 21:23:35 -0500 |
commit | 9d61a50bdde64762be68ee9fe7f7d5ba11f086df (patch) | |
tree | 0ed0cfa8fa6832cc7d4537f29f4aeb4f9fcc5e1a /hw | |
parent | 82944b87db46af80bfacb02bbd4864581e519308 (diff) |
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 <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Diffstat (limited to 'hw')
-rw-r--r-- | hw/ide.c | 22 | ||||
-rw-r--r-- | hw/scsi-disk.c | 21 | ||||
-rw-r--r-- | hw/virtio-blk.c | 20 |
3 files changed, 54 insertions, 9 deletions
@@ -501,6 +501,7 @@ typedef struct BMDMAState { QEMUIOVector qiov; int64_t sector_num; uint32_t nsector; + QEMUBH *bh; } BMDMAState; typedef struct PCIIDEState { @@ -1109,11 +1110,13 @@ static void ide_sector_write(IDEState *s) } } -static void ide_dma_restart_cb(void *opaque, int running, int reason) +static void ide_dma_restart_bh(void *opaque) { BMDMAState *bm = opaque; - if (!running) - return; + + qemu_bh_delete(bm->bh); + bm->bh = NULL; + if (bm->status & BM_STATUS_DMA_RETRY) { bm->status &= ~BM_STATUS_DMA_RETRY; ide_dma_restart(bm->ide_if); @@ -1123,6 +1126,19 @@ static void ide_dma_restart_cb(void *opaque, int running, int reason) } } +static void ide_dma_restart_cb(void *opaque, int running, int reason) +{ + BMDMAState *bm = opaque; + + if (!running) + return; + + if (!bm->bh) { + bm->bh = qemu_bh_new(ide_dma_restart_bh, bm); + qemu_bh_schedule(bm->bh); + } +} + static void ide_write_dma_cb(void *opaque, int ret) { BMDMAState *bm = opaque; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 8b6426fd8c..5b825c9f79 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) { diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 2beff5224a..5036b5b024 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -26,6 +26,7 @@ typedef struct VirtIOBlock VirtQueue *vq; void *rq; char serial_str[BLOCK_SERIAL_STRLEN + 1]; + QEMUBH *bh; } VirtIOBlock; static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev) @@ -302,13 +303,13 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) */ } -static void virtio_blk_dma_restart_cb(void *opaque, int running, int reason) +static void virtio_blk_dma_restart_bh(void *opaque) { VirtIOBlock *s = opaque; VirtIOBlockReq *req = s->rq; - if (!running) - return; + qemu_bh_delete(s->bh); + s->bh = NULL; s->rq = NULL; @@ -318,6 +319,19 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running, int reason) } } +static void virtio_blk_dma_restart_cb(void *opaque, int running, int reason) +{ + VirtIOBlock *s = opaque; + + if (!running) + return; + + if (!s->bh) { + s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s); + qemu_bh_schedule(s->bh); + } +} + static void virtio_blk_reset(VirtIODevice *vdev) { /* |