Kevin Wolf <kw...@redhat.com> writes: > Am 11.09.2019 um 18:15 hat Sergio Lopez geschrieben: >> On creation, the export's AioContext is set to the same one as the >> BlockBackend, while the AioContext in the client QIOChannel is left >> untouched. >> >> As a result, when using data-plane, nbd_client_receive_next_request() >> schedules coroutines in the IOThread AioContext, while the client's >> QIOChannel is serviced from the main_loop, potentially triggering the >> assertion at qio_channel_restart_[read|write]. >> >> To fix this, as soon we have the export corresponding to the client, >> we call qio_channel_attach_aio_context() to attach the QIOChannel >> context to the export's AioContext. This matches with the logic in >> blk_aio_attached(). >> >> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1748253 >> Signed-off-by: Sergio Lopez <s...@redhat.com> > > Oh, looks like I only fixed switching the AioContext after the fact, but > not starting the NBD server for a node that is already in a different > AioContext... :-/ > >> diff --git a/nbd/server.c b/nbd/server.c >> index 10faedcfc5..51322e2343 100644 >> --- a/nbd/server.c >> +++ b/nbd/server.c >> @@ -471,6 +471,7 @@ static int nbd_negotiate_handle_export_name(NBDClient >> *client, >> QTAILQ_INSERT_TAIL(&client->exp->clients, client, next); >> nbd_export_get(client->exp); >> nbd_check_meta_export(client); >> + qio_channel_attach_aio_context(client->ioc, client->exp->ctx); >> >> return 0; >> } >> @@ -673,6 +674,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, >> uint16_t myflags, >> QTAILQ_INSERT_TAIL(&client->exp->clients, client, next); >> nbd_export_get(client->exp); >> nbd_check_meta_export(client); >> + qio_channel_attach_aio_context(client->ioc, exp->ctx); >> rc = 1; >> } >> return rc; > > I think I would rather do this once at the end of nbd_negotiate() > instead of duplicating it across the different way to open an export. > During the negotiation phase, we don't start requests yet, so doing > everything from the main thread should be fine.
OK. > Actually, not doing everything from the main thread sounds nasty because > I think the next QIOChannel callback could then already be executed in > the iothread while this one hasn't completed yet. Or do we have any > locking in place for the negotiation? This is the first time I look at NBD code, but IIUC all the negotiation is done with synchronous nbd_[read|write]() calls, so even if the coroutine yields due to EWOULDBLOCK, nothing else should be making progress. Sergio.
signature.asc
Description: PGP signature