Re: [Qemu-devel] [RFC 3/5] nbd: Use aio_set_fd_handler2()

[Max Reitz <mreitz@redhat.com>]

On 04.06.2014 14:37, Stefan Hajnoczi wrote:
> On Sat, May 31, 2014 at 08:43:10PM +0200, Max Reitz wrote:
>> Instead of using the main loop function qemu_set_fd_handler2(), use the
>> AIO function in the context of the exported BDS.
> Managing fd handlers shouldn't be necessary at the NBD code level.  The
> current NBD code hasn't been fully converted to coroutines.
>
> This email explains how the NBD code can be fully converted to
> coroutines.  It should simplify the code and reduce the chance of bugs.
> Whether you want to actually do the conversion is up to you, since it's
> somewhat orthogonal to the purpose of this patch series.
>
> The point of coroutines is that blocking operations like send/recv on a
> socket should look like regular blocking calls.  Let coroutines handle
> the event loop housekeeping (registering fd handlers, callbacks).  Only
> use aio explicitly when concurrency is needed.
>
> Here is how I would structure NBD using coroutines:
>
> 1 coroutine per connection to receive NBD commands and submit I/O
> requests:
>
> def nbd_server_receive_co(conn):
>      while True:
>          req = nbd_read_req(conn)
>      if req is None:
>          break
>      if req.type == NBD_READ:
>          bdrv_aio_readv(bs, ...)
>      elif req.type == NBD_WRITE:
>          ...
>
> Notice that bdrv_aio_*() is used since we want concurrent I/O requests.
>
> 1 coroutine per connection to send NBD replies:
>
> def nbd_server_send_co(conn):
>      while True:
>          while conn.send_queue:
>              resp = conn.send_queue.pop()
>              nbd_write_resp(conn, resp)
>          qemu_coroutine_yield()
>
> And finally the bdrv_aio_*() callback to put responses on to the send
> queue:
>
> def nbd_server_aio_cb(conn):
>      resp = NBDResponse(...)
>      conn.send_queue.push(resp)
>      conn.send_co.enter()
>
> Why is this design cleaner?  Because NBD code doesn't have to worry
> about fd handlers.  It uses straightforward coroutine send/recv for
> socket I/O inside nbd_read_req() and nbd_write_resp().  It's easy to see
> that only one coroutine receives from the socket and that only one
> coroutine writes to the socket.

Yes, this sounds better. I'll take a look into it and see how far I can get.

Max