[Qemu-commits] [qemu/qemu] 1b5c15: nbd/client: Add hint when TLS is missing

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 1b5c15cebd5cc7595337a8a7cf14f6a707605e99
      
https://github.com/qemu/qemu/commit/1b5c15cebd5cc7595337a8a7cf14f6a707605e99
  Author: Eric Blake <address@hidden>
  Date:   2019-09-24 (Tue, 24 Sep 2019)

  Changed paths:
    M nbd/client.c
    M tests/qemu-iotests/233.out

  Log Message:
  -----------
  nbd/client: Add hint when TLS is missing

I received an off-list report of failure to connect to an NBD server
expecting an x509 certificate, when the client was attempting something
similar to this command line:

$ ./x86_64-softmmu/qemu-system-x86_64 -name 'blah' -machine q35 -nodefaults \
  -object tls-creds-x509,id=tls0,endpoint=client,dir=$path_to_certs \
  -device virtio-scsi-pci,id=virtio_scsi_pci0,bus=pcie.0,addr=0x6 \
  -drive 
id=drive_image1,if=none,snapshot=off,aio=threads,cache=none,format=raw,file=nbd:localhost:9000,werror=stop,rerror=stop,tls-creds=tls0
 \
  -device scsi-hd,id=image1,drive=drive_image1,bootindex=0
qemu-system-x86_64: -drive 
id=drive_image1,if=none,snapshot=off,aio=threads,cache=none,format=raw,file=nbd:localhost:9000,werror=stop,rerror=stop,tls-creds=tls0:
 TLS negotiation required before option 7 (go)
server reported: Option 0x7 not permitted before TLS

The problem?  As specified, -drive is trying to pass tls-creds to the
raw format driver instead of the nbd protocol driver, but before we
get to the point where we can detect that raw doesn't know what to do
with tls-creds, the nbd driver has already failed because the server
complained.  The fix to the broken command line?  Pass
'...,file.tls-creds=tls0' to ensure the tls-creds option is handed to
nbd, not raw.  But since the error message was rather cryptic, I'm
trying to improve the error message.

With this patch, the error message adds a line:

qemu-system-x86_64: -drive 
id=drive_image1,if=none,snapshot=off,aio=threads,cache=none,format=raw,file=nbd:localhost:9000,werror=stop,rerror=stop,tls-creds=tls0:
 TLS negotiation required before option 7 (go)
Did you forget a valid tls-creds?
server reported: Option 0x7 not permitted before TLS

And with luck, someone grepping for that error message will find this
commit message and figure out their command line mistake.  Sadly, the
only mention of file.tls-creds in our docs relates to an --image-opts
use of PSK encryption with qemu-img as the client, rather than x509
certificate encryption with qemu-kvm as the client.

CC: Tingting Mao <address@hidden>
CC: Daniel P. Berrangé <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[eblake: squash in iotest 233 fix]
Reviewed-by: Daniel P. Berrangé <address@hidden>


  Commit: b4961249af0403fa55aae57c4c8806b24f7a7b33
      
https://github.com/qemu/qemu/commit/b4961249af0403fa55aae57c4c8806b24f7a7b33
  Author: Sergio Lopez <address@hidden>
  Date:   2019-09-24 (Tue, 24 Sep 2019)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: attach client channel to the export's AioContext

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 at
blk_aio_attached().

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1748253
Signed-off-by: Sergio Lopez <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 61bc846d8c58535af6884b637a4005dd6111ea95
      
https://github.com/qemu/qemu/commit/61bc846d8c58535af6884b637a4005dd6111ea95
  Author: Eric Blake <address@hidden>
  Date:   2019-09-24 (Tue, 24 Sep 2019)

  Changed paths:
    M blockdev-nbd.c
    M include/block/nbd.h
    M nbd/server.c

  Log Message:
  -----------
  nbd: Grab aio context lock in more places

When iothreads are in use, the failure to grab the aio context results
in an assertion failure when trying to unlock things during blk_unref,
when trying to unlock a mutex that was not locked.  In short, all
calls to nbd_export_put need to done while within the correct aio
context.  But since nbd_export_put can recursively reach itself via
nbd_export_close, and recursively grabbing the context would deadlock,
we can't do the context grab directly in those functions, but must do
so in their callers.

Hoist the use of the correct aio_context from nbd_export_new() to its
caller qmp_nbd_server_add().  Then tweak qmp_nbd_server_remove(),
nbd_eject_notifier(), and nbd_esport_close_all() to grab the right
context, so that all callers during qemu now own the context before
nbd_export_put() can call blk_unref().

Remaining uses in qemu-nbd don't matter (since that use case does not
support iothreads).

Suggested-by: Kevin Wolf <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Sergio Lopez <address@hidden>


  Commit: 506902c6fa80210b002e30ff33794bfc718b15c6
      
https://github.com/qemu/qemu/commit/506902c6fa80210b002e30ff33794bfc718b15c6
  Author: Eric Blake <address@hidden>
  Date:   2019-09-24 (Tue, 24 Sep 2019)

  Changed paths:
    M tests/qemu-iotests/223
    M tests/qemu-iotests/223.out

  Log Message:
  -----------
  tests: Use iothreads during iotest 223

Doing so catches the bugs we just fixed with NBD not properly using
correct contexts.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: da5e1169183ca6eb6fb470dc32ed1bfc24d1d406
      
https://github.com/qemu/qemu/commit/da5e1169183ca6eb6fb470dc32ed1bfc24d1d406
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-09-25 (Wed, 25 Sep 2019)

  Changed paths:
    M util/qemu-sockets.c

  Log Message:
  -----------
  util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

In "if (saddr->keep_alive) {" we may already be on error path, with
invalid sock < 0. Fix it by returning error earlier.

Reported-by: Coverity (CID 1405300)
Fixes: aec21d31756cbd
Suggested-by: Peter Maydell <address@hidden>
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel P. Berrangé <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: d4e536f336d3d26c9fafa2a2549aaa0b014f5b6b
      
https://github.com/qemu/qemu/commit/d4e536f336d3d26c9fafa2a2549aaa0b014f5b6b
  Author: Peter Maydell <address@hidden>
  Date:   2019-09-26 (Thu, 26 Sep 2019)

  Changed paths:
    M blockdev-nbd.c
    M include/block/nbd.h
    M nbd/client.c
    M nbd/server.c
    M tests/qemu-iotests/223
    M tests/qemu-iotests/223.out
    M tests/qemu-iotests/233.out
    M util/qemu-sockets.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2019-09-24-v2' into 
staging

nbd patches for 2019-09-24

- Improved error message for plaintext client of encrypted server
- Fix various assertions when -object iothread is in use
- Silence a Coverity error for use-after-free on error path

# gpg: Signature made Wed 25 Sep 2019 14:35:52 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <address@hidden>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<address@hidden>" [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-2019-09-24-v2:
  util/qemu-sockets: fix keep_alive handling in inet_connect_saddr
  tests: Use iothreads during iotest 223
  nbd: Grab aio context lock in more places
  nbd/server: attach client channel to the export's AioContext
  nbd/client: Add hint when TLS is missing

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/4142b011cac4...d4e536f336d3