Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/6] Acquire the AioContext during _realize()

[Stefan Hajnoczi]

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