[Qemu-devel] -drive werror=stop can cause state change handlers run out of order

[Markus Armbruster]

Consider the following scenario[1]:

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()[2].  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. Say 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.

What happens here is that when a VM state change handler changes VM
state, other VM state change handlers can see the state transitions out
of order.

I showed this to Gleb, and he suggested to have ide_dma_restart_cb()[3]
set up a bottom half to retry writes.  I'm not familiar with the block
code, so I figure I ask here before I try it: Is that the way to fix
this?


[1] Note: I didn't actually reproduce it in this form with upstream
code.

[2] Actually two of them, for the IDE device's bmdma[0] and bmdma[1],
but that doesn't matter.

[3] Same for SCSI and virtio-blk.