mirror of https://github.com/xemu-project/xemu.git
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>
This commit is contained in:
parent
3e28c9adf4
commit
213189ab65
22
hw/ide.c
22
hw/ide.c
|
@ -501,6 +501,7 @@ typedef struct BMDMAState {
|
||||||
QEMUIOVector qiov;
|
QEMUIOVector qiov;
|
||||||
int64_t sector_num;
|
int64_t sector_num;
|
||||||
uint32_t nsector;
|
uint32_t nsector;
|
||||||
|
QEMUBH *bh;
|
||||||
} BMDMAState;
|
} BMDMAState;
|
||||||
|
|
||||||
typedef struct PCIIDEState {
|
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;
|
BMDMAState *bm = opaque;
|
||||||
if (!running)
|
|
||||||
return;
|
qemu_bh_delete(bm->bh);
|
||||||
|
bm->bh = NULL;
|
||||||
|
|
||||||
if (bm->status & BM_STATUS_DMA_RETRY) {
|
if (bm->status & BM_STATUS_DMA_RETRY) {
|
||||||
bm->status &= ~BM_STATUS_DMA_RETRY;
|
bm->status &= ~BM_STATUS_DMA_RETRY;
|
||||||
ide_dma_restart(bm->ide_if);
|
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)
|
static void ide_write_dma_cb(void *opaque, int ret)
|
||||||
{
|
{
|
||||||
BMDMAState *bm = opaque;
|
BMDMAState *bm = opaque;
|
||||||
|
|
|
@ -74,6 +74,7 @@ struct SCSIDeviceState
|
||||||
scsi_completionfn completion;
|
scsi_completionfn completion;
|
||||||
void *opaque;
|
void *opaque;
|
||||||
char drive_serial_str[21];
|
char drive_serial_str[21];
|
||||||
|
QEMUBH *bh;
|
||||||
};
|
};
|
||||||
|
|
||||||
/* Global pool of SCSIRequest structures. */
|
/* Global pool of SCSIRequest structures. */
|
||||||
|
@ -308,12 +309,13 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
|
||||||
return 0;
|
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;
|
SCSIDeviceState *s = opaque;
|
||||||
SCSIRequest *r = s->requests;
|
SCSIRequest *r = s->requests;
|
||||||
if (!running)
|
|
||||||
return;
|
qemu_bh_delete(s->bh);
|
||||||
|
s->bh = NULL;
|
||||||
|
|
||||||
while (r) {
|
while (r) {
|
||||||
if (r->status & SCSI_REQ_STATUS_RETRY) {
|
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. */
|
/* Return a pointer to the data buffer. */
|
||||||
static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
|
static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
|
||||||
{
|
{
|
||||||
|
|
|
@ -26,6 +26,7 @@ typedef struct VirtIOBlock
|
||||||
VirtQueue *vq;
|
VirtQueue *vq;
|
||||||
void *rq;
|
void *rq;
|
||||||
char serial_str[BLOCK_SERIAL_STRLEN + 1];
|
char serial_str[BLOCK_SERIAL_STRLEN + 1];
|
||||||
|
QEMUBH *bh;
|
||||||
} VirtIOBlock;
|
} VirtIOBlock;
|
||||||
|
|
||||||
static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
|
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;
|
VirtIOBlock *s = opaque;
|
||||||
VirtIOBlockReq *req = s->rq;
|
VirtIOBlockReq *req = s->rq;
|
||||||
|
|
||||||
if (!running)
|
qemu_bh_delete(s->bh);
|
||||||
return;
|
s->bh = NULL;
|
||||||
|
|
||||||
s->rq = 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)
|
static void virtio_blk_reset(VirtIODevice *vdev)
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
|
|
Loading…
Reference in New Issue