mbox

[PULL,00/34] NBD patches for 2021-06-15

Message ID 20210615204756.281505-1-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Pull-request

https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-06-15

Message

Eric Blake June 15, 2021, 8:47 p.m. UTC
The following changes since commit 1ea06abceec61b6f3ab33dadb0510b6e09fb61e2:

  Merge remote-tracking branch 'remotes/berrange-gitlab/tags/misc-fixes-pull-request' into staging (2021-06-14 15:59:13 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-06-15

for you to fetch changes up to 788b68b57dea4ddd0038f73b96c147eb406c386d:

  block/nbd: safer transition to receiving request (2021-06-15 15:42:33 -0500)

----------------------------------------------------------------
nbd patches for 2021-06-15

- bug fixes in coroutine aio context handling
- rework NBD client connection logic to perform more work in coroutine
rather than blocking main loop

----------------------------------------------------------------
Paolo Bonzini (2):
      async: the main AioContext is only "current" if under the BQL
      tests: cover aio_co_enter from a worker thread without BQL taken

Roman Kagan (2):
      block/nbd: fix channel object leak
      block/nbd: ensure ->connection_thread is always valid

Vladimir Sementsov-Ogievskiy (30):
      co-queue: drop extra coroutine_fn marks
      block/nbd: fix how state is cleared on nbd_open() failure paths
      block/nbd: connect_thread_func(): do qio_channel_set_delay(false)
      qemu-sockets: introduce socket_address_parse_named_fd()
      block/nbd: call socket_address_parse_named_fd() in advance
      block/nbd: nbd_client_handshake(): fix leak of s->ioc
      block/nbd: BDRVNBDState: drop unused connect_err and connect_status
      block/nbd: simplify waking of nbd_co_establish_connection()
      block/nbd: drop thr->state
      block/nbd: bs-independent interface for nbd_co_establish_connection()
      block/nbd: make nbd_co_establish_connection_cancel() bs-independent
      block/nbd: rename NBDConnectThread to NBDClientConnection
      block/nbd: introduce nbd_client_connection_new()
      block/nbd: introduce nbd_client_connection_release()
      nbd: move connection code from block/nbd to nbd/client-connection
      nbd/client-connection: use QEMU_LOCK_GUARD
      nbd/client-connection: add possibility of negotiation
      nbd/client-connection: implement connection retry
      nbd/client-connection: shutdown connection on release
      block/nbd: split nbd_handle_updated_info out of nbd_client_handshake()
      block/nbd: use negotiation of NBDClientConnection
      block/nbd: don't touch s->sioc in nbd_teardown_connection()
      block/nbd: drop BDRVNBDState::sioc
      nbd/client-connection: return only one io channel
      block-coroutine-wrapper: allow non bdrv_ prefix
      block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt
      nbd/client-connection: add option for non-blocking connection attempt
      block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
      block/nbd: add nbd_client_connected() helper
      block/nbd: safer transition to receiving request

 scripts/block-coroutine-wrapper.py |   7 +-
 block/coroutines.h                 |   6 +
 include/block/aio.h                |   5 +-
 include/block/nbd.h                |  18 ++
 include/qemu/coroutine.h           |   6 +-
 include/qemu/sockets.h             |  11 +
 block/nbd.c                        | 585 ++++++++-----------------------------
 iothread.c                         |   9 +-
 nbd/client-connection.c            | 385 ++++++++++++++++++++++++
 stubs/iothread-lock.c              |   2 +-
 stubs/iothread.c                   |   8 -
 tests/unit/iothread.c              |   9 +-
 tests/unit/test-aio.c              |  37 +++
 util/async.c                       |  20 ++
 util/main-loop.c                   |   1 +
 util/qemu-sockets.c                |  19 ++
 nbd/meson.build                    |   1 +
 stubs/meson.build                  |   1 -
 18 files changed, 639 insertions(+), 491 deletions(-)
 create mode 100644 nbd/client-connection.c
 delete mode 100644 stubs/iothread.c

Comments

Peter Maydell June 17, 2021, 9:42 a.m. UTC | #1
On Tue, 15 Jun 2021 at 21:50, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit 1ea06abceec61b6f3ab33dadb0510b6e09fb61e2:
>
>   Merge remote-tracking branch 'remotes/berrange-gitlab/tags/misc-fixes-pull-request' into staging (2021-06-14 15:59:13 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-06-15
>
> for you to fetch changes up to 788b68b57dea4ddd0038f73b96c147eb406c386d:
>
>   block/nbd: safer transition to receiving request (2021-06-15 15:42:33 -0500)
>
> ----------------------------------------------------------------
> nbd patches for 2021-06-15
>
> - bug fixes in coroutine aio context handling
> - rework NBD client connection logic to perform more work in coroutine
> rather than blocking main loop

Fails to compile, all hosts:

../../nbd/client-connection.c: In function ‘nbd_co_establish_connection’:
../../nbd/client-connection.c:352:16: error: ‘ioc’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
  352 |             if (ioc) {
      |                ^


clang is more specific:


../../nbd/client-connection.c:298:21: error: variable 'ioc' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
                if (conn->do_negotiation) {
                    ^~~~~~~~~~~~~~~~~~~~
../../nbd/client-connection.c:302:21: note: uninitialized use occurs here
                if (ioc) {
                    ^~~
../../nbd/client-connection.c:298:17: note: remove the 'if' if its
condition is always true
                if (conn->do_negotiation) {
                ^~~~~~~~~~~~~~~~~~~~~~~~~~
../../nbd/client-connection.c:281:20: note: initialize the variable
'ioc' to silence this warning
    QIOChannel *ioc;
                   ^
                    = NULL
1 error generated.


thanks
-- PMM
Vladimir Sementsov-Ogievskiy June 17, 2021, 6:35 p.m. UTC | #2
17.06.2021 12:42, Peter Maydell wrote:
> On Tue, 15 Jun 2021 at 21:50, Eric Blake <eblake@redhat.com> wrote:
>>
>> The following changes since commit 1ea06abceec61b6f3ab33dadb0510b6e09fb61e2:
>>
>>    Merge remote-tracking branch 'remotes/berrange-gitlab/tags/misc-fixes-pull-request' into staging (2021-06-14 15:59:13 +0100)
>>
>> are available in the Git repository at:
>>
>>    https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-06-15
>>
>> for you to fetch changes up to 788b68b57dea4ddd0038f73b96c147eb406c386d:
>>
>>    block/nbd: safer transition to receiving request (2021-06-15 15:42:33 -0500)
>>
>> ----------------------------------------------------------------
>> nbd patches for 2021-06-15
>>
>> - bug fixes in coroutine aio context handling
>> - rework NBD client connection logic to perform more work in coroutine
>> rather than blocking main loop
> 
> Fails to compile, all hosts:
> 
> ../../nbd/client-connection.c: In function ‘nbd_co_establish_connection’:
> ../../nbd/client-connection.c:352:16: error: ‘ioc’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>    352 |             if (ioc) {
>        |                ^
> 
> 
> clang is more specific:
> 
> 
> ../../nbd/client-connection.c:298:21: error: variable 'ioc' is used
> uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
>                  if (conn->do_negotiation) {
>                      ^~~~~~~~~~~~~~~~~~~~
> ../../nbd/client-connection.c:302:21: note: uninitialized use occurs here
>                  if (ioc) {
>                      ^~~
> ../../nbd/client-connection.c:298:17: note: remove the 'if' if its
> condition is always true
>                  if (conn->do_negotiation) {
>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../nbd/client-connection.c:281:20: note: initialize the variable
> 'ioc' to silence this warning
>      QIOChannel *ioc;
>                     ^
>                      = NULL
> 1 error generated.
> 


Sorry for this :(

Only one patch needs fixing: 28. I posted a squash-in. Eric, could you please take a look and make a v2 of pull request?