On Mon, Jan 14, 2019 at 04:23:58PM +0200, Alberto Garcia wrote: > This series acquires the AioContext in the _realize() functions of > several devices before making use of their block backends. This fixes > at least a couple of crashes (in virtio-blk and scsi). The other > devices don't currently support iothreads so there's no crashes. > > Berto
My assumption has been that drives are in the main loop AioContext until the device model (e.g. virtio-blk) decides to move them into the IOThread configured by the user. (And when the VM is stopped or the device is removed, the drive is moved back into the main loop AioContext by the device.) The x-blockdev-set-iothread command is for low-level tests so I don't expect users to invoke it. So I'm not sure which real-world scenario is being tested here? This patch series makes virtio-blk and virtio-scsi more robust, although I haven't checked what happens if the drive is attached to a different IOThread than the device - will the switchover work? What I'm unclear about is why fdc, ide, usb-msd, and nvme should worry about IOThreads/AioContext in .realize() since they don't support it in their data path. What happens when you submit I/O requests to devices configured like this? I guess they would crash or hit assertion failures since they invoke blk_aio_*() APIs from outside the appropriate AioContext. Maybe fdc and friends should forbid this scenario: /* Fail if @blk is attached to an IOThread */ static bool blk_check_main_aio_context(BlockBackend *blk, Error **errp) { if (blk_get_aio_context(blk) != qemu_get_aio_context()) { error_setg(errp, "Device does not support iothreads"); return false; } return true; } ...in a realize() function... if (!blk_check_main_aio_context(blk, errp)) { return; }
signature.asc
Description: PGP signature