[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC 0/5] nbd: Adapt for dataplane |
Date: |
Wed, 4 Jun 2014 14:50:05 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Sat, May 31, 2014 at 08:43:07PM +0200, Max Reitz wrote:
> For the NBD server to work with dataplane, it needs to correctly access
> the exported BDS. It makes the most sense to run both in the same
> AioContext, therefore this series implements methods for tracking a
> BDS's AioContext and makes NBD make use of this for keeping the clients
> connected to that BDS in the same AioContext.
>
> The reason this is an RFC and not a PATCH is my inexperience with AIO,
> coroutines and the like. Also, I'm not sure about what to do about the
> coroutines. The NBD server has up to two coroutines per client: One for
> receiving and one for sending. Theoretically, both have to be
> "transferred" to the new AioContext if it is changed; however, as far as
> I see it, coroutines are not really bound to an AioContext, they are
> simply run in the AioContext entering them. Therefore, I think a
> transfer is unnecessary. All coroutines are entered from nbd_read() and
> nbd_restart_write(), both of which are AIO routines registered via
> aio_set_fd_handler2().
>
> As bs_aio_detach() unregisters all of these routines, the coroutines can
> no longer be entered, but only after bs_aio_attach() is called again.
> Then, when they are called, they will enter the coroutines in the new
> AioContext. Therefore, I think an explicit transfer unnecessary.
>
> However, if bs_aio_detach() is called from a different thread than the
> old AioContext is running in, we may still have coroutines running for
> which we should wait before returning from bs_aio_detach().
>
> But because of my inexperience with coroutines, I'm not sure. I now have
> these patches nearly unchanged here for about a week and I'm looking for
> ways of testing them, but so far I could only test whether the old use
> cases work, but not whether they will work for what they are intended to
> do: With BDS changing their AioContext.
>
> So, because I'm not sure what else to do and because I don't know how to
> test multiple AIO threads (how do I move a BDS into another iothread?)
> I'm just sending this out as an RFC.
>
>
> Max Reitz (5):
> nbd: Correct name comparison for export_set_name()
> aio: Add io_read_poll() callback
> nbd: Use aio_set_fd_handler2()
> block: Add AIO followers
> nbd: Follow the BDS' AIO context
>
> aio-posix.c | 26 ++++++++++++++++-----
> block.c | 55 +++++++++++++++++++++++++++++++++++++++++++
> include/block/aio.h | 12 ++++++++++
> include/block/block_int.h | 40 ++++++++++++++++++++++++++++++++
> include/qemu/main-loop.h | 1 -
> nbd.c | 59
> ++++++++++++++++++++++++++++++++++++++---------
> 6 files changed, 175 insertions(+), 18 deletions(-)
Thanks for the RFC. It looks like the concept will work correctly. I
left comments, a lot of them are just ideas and you don't have to
implement them if you don't want to.
Stefan