qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 5f50be: async: the main AioContext is only "c


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 5f50be: async: the main AioContext is only "current" if un...
Date: Sun, 20 Jun 2021 13:25:42 -0700

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: 5f50be9b5810293141bb53cfd0cb46e765367d56
      
https://github.com/qemu/qemu/commit/5f50be9b5810293141bb53cfd0cb46e765367d56
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M include/block/aio.h
    M iothread.c
    M stubs/iothread-lock.c
    R stubs/iothread.c
    M stubs/meson.build
    M tests/unit/iothread.c
    M util/async.c
    M util/main-loop.c

  Log Message:
  -----------
  async: the main AioContext is only "current" if under the BQL

If we want to wake up a coroutine from a worker thread, aio_co_wake()
currently does not work.  In that scenario, aio_co_wake() calls
aio_co_enter(), but there is no current AioContext and therefore
qemu_get_current_aio_context() returns the main thread.  aio_co_wake()
then attempts to call aio_context_acquire() instead of going through
aio_co_schedule().

The default case of qemu_get_current_aio_context() was added to cover
synchronous I/O started from the vCPU thread, but the main and vCPU
threads are quite different.  The main thread is an I/O thread itself,
only running a more complicated event loop; the vCPU thread instead
is essentially a worker thread that occasionally calls
qemu_mutex_lock_iothread().  It is only in those critical sections
that it acts as if it were the home thread of the main AioContext.

Therefore, this patch detaches qemu_get_current_aio_context() from
iothreads, which is a useless complication.  The AioContext pointer
is stored directly in the thread-local variable, including for the
main loop.  Worker threads (including vCPU threads) optionally behave
as temporary home threads if they have taken the big QEMU lock,
but if that is not the case they will always schedule coroutines
on remote threads via aio_co_schedule().

With this change, the stub qemu_mutex_iothread_locked() must be changed
from true to false.  The previous value of true was needed because the
main thread did not have an AioContext in the thread-local variable,
but now it does have one.

Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210609122234.544153-1-pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[eblake: tweak commit message per Vladimir's review]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 55159c34b8788ae00984341356d3ea4774912665
      
https://github.com/qemu/qemu/commit/55159c34b8788ae00984341356d3ea4774912665
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M tests/unit/test-aio.c

  Log Message:
  -----------
  tests: cover aio_co_enter from a worker thread without BQL taken

Add a testcase for the test fixed by commit 'async: the main AioContext
is only "current" if under the BQL.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210614110214.726722-1-pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 0e70260b65814fe7c016a63c3081ac39617294a0
      
https://github.com/qemu/qemu/commit/0e70260b65814fe7c016a63c3081ac39617294a0
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M include/qemu/coroutine.h

  Log Message:
  -----------
  co-queue: drop extra coroutine_fn marks

qemu_co_queue_next() and qemu_co_queue_restart_all() just call
aio_co_wake() which works well in non-coroutine context. So these
functions can be called from non-coroutine context as well. And
actually qemu_co_queue_restart_all() is called from
nbd_cancel_in_flight(), which is called from non-coroutine context.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-2-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 3687ad49038e13103f7382316e16dff79abddf95
      
https://github.com/qemu/qemu/commit/3687ad49038e13103f7382316e16dff79abddf95
  Author: Roman Kagan <rvkagan@yandex-team.ru>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: fix channel object leak

nbd_free_connect_thread leaks the channel object if it hasn't been
stolen.

Unref it and fix the leak.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: bbba1c376b8b1ba5171bd14eb6bf212fa1173ddb
      
https://github.com/qemu/qemu/commit/bbba1c376b8b1ba5171bd14eb6bf212fa1173ddb
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: fix how state is cleared on nbd_open() failure paths

We have two "return error" paths in nbd_open() after
nbd_process_options(). Actually we should call nbd_clear_bdrvstate()
on these paths. Interesting that nbd_process_options() calls
nbd_clear_bdrvstate() by itself.

Let's fix leaks and refactor things to be more obvious:

- intialize yank at top of nbd_open()
- move yank cleanup to nbd_clear_bdrvstate()
- refactor nbd_open() so that all failure paths except for
  yank-register goes through nbd_clear_bdrvstate()

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: fb392b548eb4c6c2b2c7689e7fc6b1d2077d4f02
      
https://github.com/qemu/qemu/commit/fb392b548eb4c6c2b2c7689e7fc6b1d2077d4f02
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: connect_thread_func(): do qio_channel_set_delay(false)

nbd_open() does it (through nbd_establish_connection()).
Actually we lost that call on reconnect path in 1dc4718d849e1a1fe
"block/nbd: use non-blocking connect: fix vm hang on connect()"
when we have introduced reconnect thread.

Fixes: 1dc4718d849e1a1fe665ce5241ed79048cfa2cfc
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-5-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: c5423704184c43cadd7b3c5ff0aea3925c5509bc
      
https://github.com/qemu/qemu/commit/c5423704184c43cadd7b3c5ff0aea3925c5509bc
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M include/qemu/sockets.h
    M util/qemu-sockets.c

  Log Message:
  -----------
  qemu-sockets: introduce socket_address_parse_named_fd()

Add function that transforms named fd inside SocketAddress structure
into number representation. This way it may be then used in a context
where current monitor is not available.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-6-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: comment tweak]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 6cc702beac795a6de7b5f97700b140dcd9936055
      
https://github.com/qemu/qemu/commit/6cc702beac795a6de7b5f97700b140dcd9936055
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: call socket_address_parse_named_fd() in advance

Detecting monitor by current coroutine works bad when we are not in
coroutine context. And that's exactly so in nbd reconnect code, where
qio_channel_socket_connect_sync() is called from thread.

Monitor is needed only to parse named file descriptor. So, let's just
parse it during nbd_open(), so that all further users of s->saddr don't
need to access monitor.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-7-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e8b35bf5dc8d4e98d91855a9c7b2ed905c8e6888
      
https://github.com/qemu/qemu/commit/e8b35bf5dc8d4e98d91855a9c7b2ed905c8e6888
  Author: Roman Kagan <rvkagan@yandex-team.ru>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: ensure ->connection_thread is always valid

Simplify lifetime management of BDRVNBDState->connect_thread by
delaying the possible cleanup of it until the BDRVNBDState itself goes
away.

This also reverts
 0267101af6 "block/nbd: fix possible use after free of s->connect_thread"
as now s->connect_thread can't be cleared until the very end.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
 [vsementsov: rebase, revert 0267101af6 changes]
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 [eblake: tweak comment]
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-8-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 2a25def4be09714c543713f111813b521b2356ee
      
https://github.com/qemu/qemu/commit/2a25def4be09714c543713f111813b521b2356ee
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: nbd_client_handshake(): fix leak of s->ioc

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Message-Id: <20210610100802.5888-9-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 2def3edb4bdc6913c83b14beb0140c395e68ac17
      
https://github.com/qemu/qemu/commit/2def3edb4bdc6913c83b14beb0140c395e68ac17
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: BDRVNBDState: drop unused connect_err and connect_status

These fields are write-only. Drop them.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-10-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 08ea55d0681333c8c6475a82b71f7bc946042986
      
https://github.com/qemu/qemu/commit/08ea55d0681333c8c6475a82b71f7bc946042986
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: simplify waking of nbd_co_establish_connection()

Instead of managing connect_bh, bh_ctx, and wait_connect fields, we
can use a single link to the waiting coroutine with proper mutex
protection.

So new logic is:

nbd_co_establish_connection() sets wait_co under the mutex, releases
the mutex, then yield()s.  Note that wait_co may be scheduled by the
thread immediately after unlocking the mutex.  Still, the main thread
(or iothread) will not reach the code for entering the coroutine until
the yield(), so we are safe.

connect_thread_func() and nbd_co_establish_connection_cancel() do
the following to handle wait_co:

Under the mutex, if thr->wait_co is not NULL, make it NULL and
schedule it. This way, we avoid scheduling the coroutine twice.

Still scheduling is a bit different:

In connect_thread_func() we can just call aio_co_wake under mutex,
after commit
   [async: the main AioContext is only "current" if under the BQL]
we are sure that aio_co_wake() will not try to acquire the aio context
and do qemu_aio_coroutine_enter() but simply schedule the coroutine by
aio_co_schedule().

nbd_co_establish_connection_cancel() will be called from non-coroutine
context in further patch and will be able to go through
qemu_aio_coroutine_enter() path of aio_co_wake(). So keep current
behavior of waking the coroutine after the critical section.

Also, this commit reduces the dependence of
nbd_co_establish_connection() on the internals of bs (we now use a
generic pointer to the coroutine, instead of direct use of
s->connection_co).  This is a step towards splitting the connection
API out of nbd.c.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-11-vsementsov@virtuozzo.com>
Reviewied-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: b8e8a3d116d2ba0f80ff47290604ece8c6ed09ca
      
https://github.com/qemu/qemu/commit/b8e8a3d116d2ba0f80ff47290604ece8c6ed09ca
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: drop thr->state

We don't need all these states. The code refactored to use two boolean
variables looks simpler.

While moving the comment in nbd_co_establish_connection() rework it to
give better information. Also, we are going to move the connection code
to separate file and mentioning drained section would be confusing.

Improve also the comment in NBDConnectThread, while dropping removed
state names from it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-12-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: comment tweak]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: d33833d7af73641d26b836a40f0bc697b656859b
      
https://github.com/qemu/qemu/commit/d33833d7af73641d26b836a40f0bc697b656859b
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: bs-independent interface for nbd_co_establish_connection()

We are going to split connection code to a separate file. Now we are
ready to give nbd_co_establish_connection() clean and bs-independent
interface.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-13-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: c3e77304855040ffd390cb7abaf7ec9ebb9b714c
      
https://github.com/qemu/qemu/commit/c3e77304855040ffd390cb7abaf7ec9ebb9b714c
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: make nbd_co_establish_connection_cancel() bs-independent

nbd_co_establish_connection_cancel() actually needs only pointer to
NBDConnectThread. So, make it clean.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-14-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 90ddc64fb2b9b1d698efc6d76026e76d5fe224ce
      
https://github.com/qemu/qemu/commit/90ddc64fb2b9b1d698efc6d76026e76d5fe224ce
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: rename NBDConnectThread to NBDClientConnection

We are going to move the connection code to its own file, and want
clear names and APIs first.

The structure is shared between user and (possibly) several runs of
connect-thread. So it's wrong to call it "thread". Let's rename to
something more generic.

Appropriately rename connect_thread and thr variables to conn.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-15-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: f68729747da6b770e895fa88fedf7997666bc735
      
https://github.com/qemu/qemu/commit/f68729747da6b770e895fa88fedf7997666bc735
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: introduce nbd_client_connection_new()

This is a step of creating bs-independent nbd connection interface.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-16-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 248d4701989dbe8de1c06aa8f65ef38f289df87b
      
https://github.com/qemu/qemu/commit/248d4701989dbe8de1c06aa8f65ef38f289df87b
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: introduce nbd_client_connection_release()

This is a last step of creating bs-independent nbd connection
interface. With next commit we can finally move it to separate file.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-17-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 5276c87c12f4c2a2db0bf343f6d3092816f0afc6
      
https://github.com/qemu/qemu/commit/5276c87c12f4c2a2db0bf343f6d3092816f0afc6
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c
    M include/block/nbd.h
    A nbd/client-connection.c
    M nbd/meson.build

  Log Message:
  -----------
  nbd: move connection code from block/nbd to nbd/client-connection

We now have bs-independent connection API, which consists of four
functions:

  nbd_client_connection_new()
  nbd_client_connection_release()
  nbd_co_establish_connection()
  nbd_co_establish_connection_cancel()

Move them to a separate file together with NBDClientConnection
structure which becomes private to the new API.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-18-vsementsov@virtuozzo.com>
[eblake: comment tweaks]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e70da5ff6445bf09db55e4828c08c2a30d816137
      
https://github.com/qemu/qemu/commit/e70da5ff6445bf09db55e4828c08c2a30d816137
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: use QEMU_LOCK_GUARD

We don't update connect_thread_func() to use QEMU_LOCK_GUARD, as it
will get more complex critical sections logic in further commit, where
QEMU_LOCK_GUARD doesn't help.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-19-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 130d49baa50655729f09efb72e77bebf09421dd7
      
https://github.com/qemu/qemu/commit/130d49baa50655729f09efb72e77bebf09421dd7
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c
    M include/block/nbd.h
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: add possibility of negotiation

Add arguments and logic to support nbd negotiation in the same thread
after successful connection.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-20-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e0e67cbe58f42500e3451c46b3caba572f2a965f
      
https://github.com/qemu/qemu/commit/e0e67cbe58f42500e3451c46b3caba572f2a965f
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M include/block/nbd.h
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: implement connection retry

Add an option for a thread to retry connecting until it succeeds. We'll
use nbd/client-connection both for reconnect and for initial connection
in nbd_open(), so we need a possibility to use same NBDClientConnection
instance to connect once in nbd_open() and then use retry semantics for
reconnect.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-21-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: grammar tweak]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: f58b2dfe3e815d0c8491b33c36622824e8a08e40
      
https://github.com/qemu/qemu/commit/f58b2dfe3e815d0c8491b33c36622824e8a08e40
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: shutdown connection on release

Now, when a thread can do negotiation and retry, it may run relatively
long. We need a mechanism to stop it, when the user is not interested
in a result any more. So, on nbd_client_connection_release() let's
shutdown the socket, and do not retry connection if thread is detached.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-22-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e9ba7788b0c4328f7123eccb60cbb68b0b62bacb
      
https://github.com/qemu/qemu/commit/e9ba7788b0c4328f7123eccb60cbb68b0b62bacb
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: split nbd_handle_updated_info out of nbd_client_handshake()

To be reused in the following patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-23-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 6d2b0332d3a2d85bb37786a914c6865a4386ef87
      
https://github.com/qemu/qemu/commit/6d2b0332d3a2d85bb37786a914c6865a4386ef87
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: use negotiation of NBDClientConnection

Now that we can opt in to negotiation as part of the client connection
thread, use that to simplify connection_co.  This is another step on
the way to moving all reconnect code into NBDClientConnection.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-24-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: c2405af0e418a3f4cca0840f31161f7ac17b9697
      
https://github.com/qemu/qemu/commit/c2405af0e418a3f4cca0840f31161f7ac17b9697
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: don't touch s->sioc in nbd_teardown_connection()

Negotiation during reconnect is now done in a thread, and s->sioc is
not available during negotiation. Negotiation in thread will be
cancelled by nbd_client_connection_release() called from
nbd_clear_bdrvstate().  So, we don't need this code chunk anymore.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-25-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 95a078ea3e4863c0d516cf19ebcb5130bc760f49
      
https://github.com/qemu/qemu/commit/95a078ea3e4863c0d516cf19ebcb5130bc760f49
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: drop BDRVNBDState::sioc

Currently sioc pointer is used just to pass from socket-connection to
nbd negotiation. Drop the field, and use local variables instead. With
next commit we'll update nbd/client-connection.c to behave
appropriately (return only top-most ioc, not two channels).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-26-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 43cb34dede464c2e9a51ea33bc246b40db5d68d4
      
https://github.com/qemu/qemu/commit/43cb34dede464c2e9a51ea33bc246b40db5d68d4
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c
    M include/block/nbd.h
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: return only one io channel

block/nbd doesn't need underlying sioc channel anymore. So, we can
update nbd/client-connection interface to return only one top-most io
channel, which is more straight forward.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-27-vsementsov@virtuozzo.com>
[eblake: squash in Vladimir's fixes for uninit usage caught by clang]
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: bb43694872c344e27d498c0980c50c7effcb448a
      
https://github.com/qemu/qemu/commit/bb43694872c344e27d498c0980c50c7effcb448a
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M scripts/block-coroutine-wrapper.py

  Log Message:
  -----------
  block-coroutine-wrapper: allow non bdrv_ prefix

We are going to reuse the script to generate a nbd_ function in
further commit. Prepare the script now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-28-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 51edbf537d2cbf97c8e9defd098b95ca8a18aa8c
      
https://github.com/qemu/qemu/commit/51edbf537d2cbf97c8e9defd098b95ca8a18aa8c
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt

Split out the part that we want to reuse for nbd_open().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610100802.5888-29-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 97cf89259e4e0455c3b2742911737de5969dc0de
      
https://github.com/qemu/qemu/commit/97cf89259e4e0455c3b2742911737de5969dc0de
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c
    M include/block/nbd.h
    M nbd/client-connection.c

  Log Message:
  -----------
  nbd/client-connection: add option for non-blocking connection attempt

We'll need a possibility of non-blocking nbd_co_establish_connection(),
so that it returns immediately, and it returns success only if a
connections was previously established in background.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-30-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: a71d597b989fd701b923f09b3c20ac4fcaa55e81
      
https://github.com/qemu/qemu/commit/a71d597b989fd701b923f09b3c20ac4fcaa55e81
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/coroutines.h
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()

The only last step we need to reuse the function is coroutine-wrapper.
nbd_open() may be called from non-coroutine context. So, generate the
wrapper and use it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-31-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: 91e0998f5ab88e575b5d1b9bc55e0d179b9224f1
      
https://github.com/qemu/qemu/commit/91e0998f5ab88e575b5d1b9bc55e0d179b9224f1
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: add nbd_client_connected() helper

We already have two similar helpers for other state. Let's add another
one for convenience.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-32-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: bbfb7c2f350262f893642433dea66352fc168295
      
https://github.com/qemu/qemu/commit/bbfb7c2f350262f893642433dea66352fc168295
  Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  Date:   2021-06-18 (Fri, 18 Jun 2021)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: safer transition to receiving request

req->receiving is a flag of request being in one concrete yield point
in nbd_co_do_receive_one_chunk().

Such kind of boolean flag is always better to unset before scheduling
the coroutine, to avoid double scheduling. So, let's be more careful.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210610100802.5888-33-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>


  Commit: e4bfa6cd68e0b19f42c0c4ef26c024d39ebab044
      
https://github.com/qemu/qemu/commit/e4bfa6cd68e0b19f42c0c4ef26c024d39ebab044
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-06-20 (Sun, 20 Jun 2021)

  Changed paths:
    M block/coroutines.h
    M block/nbd.c
    M include/block/aio.h
    M include/block/nbd.h
    M include/qemu/coroutine.h
    M include/qemu/sockets.h
    M iothread.c
    A nbd/client-connection.c
    M nbd/meson.build
    M scripts/block-coroutine-wrapper.py
    M stubs/iothread-lock.c
    R stubs/iothread.c
    M stubs/meson.build
    M tests/unit/iothread.c
    M tests/unit/test-aio.c
    M util/async.c
    M util/main-loop.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2021-06-15-v2' into 
staging

nbd patches for 2021-06-15

- bug fixes in coroutine aio context handling
- rework NBD client connection logic to perform more work in coroutine

# gpg: Signature made Fri 18 Jun 2021 18:29:39 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2021-06-15-v2: (34 commits)
  block/nbd: safer transition to receiving request
  block/nbd: add nbd_client_connected() helper
  block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
  nbd/client-connection: add option for non-blocking connection attempt
  block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt
  block-coroutine-wrapper: allow non bdrv_ prefix
  nbd/client-connection: return only one io channel
  block/nbd: drop BDRVNBDState::sioc
  block/nbd: don't touch s->sioc in nbd_teardown_connection()
  block/nbd: use negotiation of NBDClientConnection
  block/nbd: split nbd_handle_updated_info out of nbd_client_handshake()
  nbd/client-connection: shutdown connection on release
  nbd/client-connection: implement connection retry
  nbd/client-connection: add possibility of negotiation
  nbd/client-connection: use QEMU_LOCK_GUARD
  nbd: move connection code from block/nbd to nbd/client-connection
  block/nbd: introduce nbd_client_connection_release()
  block/nbd: introduce nbd_client_connection_new()
  block/nbd: rename NBDConnectThread to NBDClientConnection
  block/nbd: make nbd_co_establish_connection_cancel() bs-independent
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/8f521741e128...e4bfa6cd68e0



reply via email to

[Prev in Thread] Current Thread [Next in Thread]