Merge tag 'pull-nbd-2024-11-18' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-11-18

- Eric Blake: Silence qemu-nbd on harmless client port probes

# -----BEGIN PGP SIGNATURE---

Merge tag 'pull-nbd-2024-11-18' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-11-18

- Eric Blake: Silence qemu-nbd on harmless client port probes

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmc7lg0ACgkQp6FrSiUn
# Q2qamgf/QJXHzF5koncXvpOC58QXthe1EQlTNqTv9lSz7GNif7UXtkv9RBMMqNEP
# 7MDY2L9LzlvMdTqXx6dndQot2YG+PSpqLFQxdjf7J3YUK5N6dQ/w4lCVI+tfVfYF
# DcYva7TBTfSWMByVdqYhnXc1nAn6liJfaaONf80y3wHObUu8T7qkMY/cb5njpjmT
# YPKyIJO7DX+ZNY5EZkFGntAxPge368nIrBsViYqst8kNWjtr8o2Rzc7fqa6sbBtw
# 47tiBDP6usBJb2kapOrpmC5zqHlbb56AbrIsTJ4Ge/iUOKrODtmCC4d7WWGSM3DC
# udaL74DOMgoMfVGjjaX9KW6Wv3/HFw==
# =DnNy
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 18 Nov 2024 19:31:25 GMT
# 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

* tag 'pull-nbd-2024-11-18' of https://repo.or.cz/qemu/ericb:
nbd-server: Silence server warnings on port probes

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

show more ...

nbd-server: Silence server warnings on port probes

While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
got LOTS of log messages of the forms:

qemu-nbd: option negotiation failed:

nbd-server: Silence server warnings on port probes

While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
got LOTS of log messages of the forms:

qemu-nbd: option negotiation failed: Failed to read flags: Unexpected end-of-file before all data were read
qemu-nbd: option negotiation failed: Failed to read flags: Unable to read from socket: Connection reset by peer

While it is nice to warn about clients that aren't following protocol
(in case it helps diagnosing bugs in those clients), a mere port probe
(where the client never write()s any bytes, and where we might even
hit EPIPE in trying to send our greeting to the client) is NOT
abnormal, but merely serves to pollute the log. And Kubernetes
_really_ likes to do port probes to determine whether a given Pod is
up and running.

Easy ways to demonstrate the above port probes:
$ qemu-nbd -r -f raw path/to/file &
$ nc localhost 10809 </dev/null
$ bash -c 'exec </dev/tcp/localhost/10809'
$ kill $!

Silence the noise by not capturing errors until after our first
successful read() from a client.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20241115195638.1132007-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

show more ...

Merge tag 'pull-nbd-2024-08-08' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-08-08

- plug CVE-2024-7409, a DoS attack exploiting nbd-server-stop

# -----BEGIN PGP SIGNATURE---

Merge tag 'pull-nbd-2024-08-08' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-08-08

- plug CVE-2024-7409, a DoS attack exploiting nbd-server-stop

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAma1PVEACgkQp6FrSiUn
# Q2qdHQf/dMydqNcPYnwEI238APyljpNvHNq6p9TYb0l5aVWisXHRlhFWM117hH7T
# Aq2KUgS5ppiEpw8mxa6/OaDa74VpMGyEPgn9w6o7T1xjVBVzpMxOKp5wFa8uICLj
# mFMYXtj9i0Rb+z0iZ+X+CqIV2Wy/FyV00Wr9T4HW94IV/9EK1sWvZvfyGWyxYyBZ
# XKTQV1Co3HYX8gfq7E88SgS064DnHjtRy2no4lwNFkBbVQCSbqwbK63TRPi7kEyC
# DmSLdHCdsD7Ev9kMZ6uNJS5T/9t7hjO5mWJckLt/cXOjHgL7GkoisLH8/nGjVkyc
# 3SUGjMn4TlzqMU99STRP+a48TLCVhA==
# =kDut
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 09 Aug 2024 07:49:05 AM AEST
# 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]

* tag 'pull-nbd-2024-08-08' of https://repo.or.cz/qemu/ericb:
nbd/server: CVE-2024-7409: Close stray clients at server-stop
nbd/server: CVE-2024-7409: Drop non-negotiating clients
nbd/server: CVE-2024-7409: Cap default max-connections to 100
nbd/server: Plumb in new args to nbd_client_add()
nbd: Minor style and typo fixes

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

show more ...

nbd/server: CVE-2024-7409: Drop non-negotiating clients

A client that opens a socket but does not negotiate is merely hogging
qemu's resources (an open fd and a small amount of memory); and a
malici

nbd/server: CVE-2024-7409: Drop non-negotiating clients

A client that opens a socket but does not negotiate is merely hogging
qemu's resources (an open fd and a small amount of memory); and a
malicious client that can access the port where NBD is listening can
attempt a denial of service attack by intentionally opening and
abandoning lots of unfinished connections. The previous patch put a
default bound on the number of such ongoing connections, but once that
limit is hit, no more clients can connect (including legitimate ones).
The solution is to insist that clients complete handshake within a
reasonable time limit, defaulting to 10 seconds. A client that has
not successfully completed NBD_OPT_GO by then (including the case of
where the client didn't know TLS credentials to even reach the point
of NBD_OPT_GO) is wasting our time and does not deserve to stay
connected. Later patches will allow fine-tuning the limit away from
the default value (including disabling it for doing integration
testing of the handshake process itself).

Note that this patch in isolation actually makes it more likely to see
qemu SEGV after nbd-server-stop, as any client socket still connected
when the server shuts down will now be closed after 10 seconds rather
than at the client's whims. That will be addressed in the next patch.

For a demo of this patch in action:
$ qemu-nbd -f raw -r -t -e 10 file &
$ nbdsh --opt-mode -c '
H = list()
for i in range(20):
print(i)
H.insert(i, nbd.NBD())
H[i].set_opt_mode(True)
H[i].connect_uri("nbd://localhost")
'
$ kill $!

where later connections get to start progressing once earlier ones are
forcefully dropped for taking too long, rather than hanging.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-13-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[eblake: rebase to changes earlier in series, reduce scope of timer]
Signed-off-by: Eric Blake <eblake@redhat.com>

show more ...

nbd/server: Plumb in new args to nbd_client_add()

Upcoming patches to fix a CVE need to track an opaque pointer passed
in by the owner of a client object, as well as request for a time
limit on how

nbd/server: Plumb in new args to nbd_client_add()

Upcoming patches to fix a CVE need to track an opaque pointer passed
in by the owner of a client object, as well as request for a time
limit on how fast negotiation must complete. Prepare for that by
changing the signature of nbd_client_new() and adding an accessor to
get at the opaque pointer, although for now the two servers
(qemu-nbd.c and blockdev-nbd.c) do not change behavior even though
they pass in a new default timeout value.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-11-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[eblake: s/LIMIT/MAX_SECS/ as suggested by Dan]
Signed-off-by: Eric Blake <eblake@redhat.com>

show more ...

nbd: Minor style and typo fixes

Touch up a comment with the wrong type name, and an over-long line,
both noticed while working on upcoming patches.

Signed-off-by: Eric Blake <eblake@redhat.com>
Mes

nbd: Minor style and typo fixes

Touch up a comment with the wrong type name, and an over-long line,
both noticed while working on upcoming patches.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-10-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

show more ...

Merge tag 'pull-nbd-2024-04-25' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-04-25

- Avoid calling poll() within coroutine

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiE

Merge tag 'pull-nbd-2024-04-25' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2024-04-25

- Avoid calling poll() within coroutine

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmYqzkMACgkQp6FrSiUn
# Q2ol3wf9HbwiYkyHhqybb4ykEs75N8B2JPbOj6gYRSBn7rz90k1vElDCM2yQhlDN
# Ltuh8lTOaJb+Z4n2dKIF2m5hL2GTm/xtErIIpP7o6A+11mHW9ag/VLaAMdWJxmUr
# WEUIH6mVtuRcxTTCp01l/JAYpUxOoQs1fyQljONH5kg1MAZpTTD61/cuhrXlvPLU
# cVlrLfob90oYhydCq5o6ucW3GhaEYkaZzHIWFy7LphFySebMmnbnPhYf/JD6RZPL
# s5K7njMK1DOyguCLlOzSuRM4gIbYunnr0Ofr/orTlAUZvbhRGKUlH0RTMWVMzgek
# xArnEZYlsqF2wIvrz0GwMDL7BMmG7A==
# =vXJj
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 25 Apr 2024 02:42:27 PM PDT
# 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]

* tag 'pull-nbd-2024-04-25' of https://repo.or.cz/qemu/ericb:
nbd/server: Mark negotiation functions as coroutine_fn
nbd/server: do not poll within a coroutine context

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

show more ...

nbd/server: Mark negotiation functions as coroutine_fn

nbd_negotiate() is already marked coroutine_fn. And given the fix in
the previous patch to have nbd_negotiate_handle_starttls not create
and w

nbd/server: Mark negotiation functions as coroutine_fn

nbd_negotiate() is already marked coroutine_fn. And given the fix in
the previous patch to have nbd_negotiate_handle_starttls not create
and wait on a g_main_loop (as that would violate coroutine
constraints), it is worth marking the rest of the related static
functions reachable only during option negotiation as also being
coroutine_fn.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240408160214.1200629-6-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
[eblake: drop one spurious coroutine_fn marking]
Signed-off-by: Eric Blake <eblake@redhat.com>

show more ...

nbd/server: do not poll within a coroutine context

Coroutines are not supposed to block. Instead, they should yield.

The client performs TLS upgrade outside of an AIOContext, during
synchronous han

nbd/server: do not poll within a coroutine context

Coroutines are not supposed to block. Instead, they should yield.

The client performs TLS upgrade outside of an AIOContext, during
synchronous handshake; this still requires g_main_loop. But the
server responds to TLS upgrade inside a coroutine, so a nested
g_main_loop is wrong. Since the two callbacks no longer share more
than the setting of data.complete and data.error, it's just as easy to
use static helpers instead of trying to share a common code path. It
is also possible to add assertions that no other code is interfering
with the eventual path to qio reaching the callback, whether or not it
required a yield or main loop.

Fixes: f95910f ("nbd: implement TLS support in the protocol negotiation")
Signed-off-by: Zhu Yangyang <zhuyangyang14@huawei.com>
[eblake: move callbacks to their use point, add assertions]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240408160214.1200629-5-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

show more ...

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- mirror: Fix deadlock
- nbd/server: Fix race in draining the export
- qemu-img snapshot: Fix formatting w

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- mirror: Fix deadlock
- nbd/server: Fix race in draining the export
- qemu-img snapshot: Fix formatting with large values
- Fix blockdev-snapshot-sync error reporting for no medium
- iotests fixes

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmX4OG8RHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9YdiQ//faXfGmbK6rBW4AkpwfrRM8SDHvm6hz7L
# 043ujAi3ziSXXoiec2/RK5wZ27nMJkfIrRHXpH41hgQvC6/3a4eIW6KSTaFV1PdG
# JtHCeopmVmgu7TZQ+kt/J6eLUTTLovoO94HgEfmxpr4CGZfx9RJftf2kCKILcYkh
# 9r04zSZLByVd4FJ5ZrqsFulWif5mXoGKdT/YisY3tKiCwFRWQDOoTymvJA012VtO
# MVmID593zwem3O3qtlGiGlK9qodBR4yof66xa/0gaYP98BZgv+LWnwLKha+OzSpX
# bQlxT26LY4JnSQkTdjF0QYnQiH4Q1kveUcNRZrGpA4iZxVDq1aks5DisThDwqoGG
# rhaPOWyJwJsonM1Enzim5Jd60JqvGdpTLjSA5oSyTjw62lAulnYihInERYSAFyyz
# UhQaO7qSog1//RpPEXEsiVkJBq8BE9l5I+L7+l5SCBhNr/UwZAOer/4m4X6d0SKN
# GEPRx0kH1voikzx7gIQs+Oldqvb0sg+zAvOynBxzpd+Ac6s8bFtWe+eSyWYL/ZGr
# Jg9+PL1xir/Uh7KmOnzt/iVBAmfSRpAo1O72xQXvHFYYtIP7hTkPO/vzqF206WMc
# WQFHHjfp5gVcMZ5AYg6txw+Bbtzu8g0AfB054lgnhihuShpf0E923TTDQFdV755s
# NUlrzuGu2fs=
# =+JIK
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 18 Mar 2024 12:49:51 GMT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin:
iotests: adapt to output change for recently introduced 'detached header' field
tests/qemu-iotests: Restrict tests using "--blockdev file" to the file protocol
tests/qemu-iotests: Fix some tests that use --image-opts for other protocols
tests/qemu-iotests: Restrict tests that use --image-opts to the 'file' protocol
tests/qemu-iotests: Restrict test 156 to the 'file' protocol
tests/qemu-iotests: Restrict test 134 and 158 to the 'file' protocol
tests/qemu-iotests: Restrict test 130 to the 'file' protocol
tests/qemu-iotests: Restrict test 114 to the 'file' protocol
tests/qemu-iotests: Restrict test 066 to the 'file' protocol
tests/qemu-iotests: Fix test 033 for running with non-file protocols
qemu-img: Fix Column Width and Improve Formatting in snapshot list
blockdev: Fix blockdev-snapshot-sync error reporting for no medium
iotests: Add test for reset/AioContext switches with NBD exports
nbd/server: Fix race in draining the export
mirror: Don't call job_pause_point() under graph lock

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

show more ...

nbd/server: Fix race in draining the export

When draining an NBD export, nbd_drained_begin() first sets
client->quiescing so that nbd_client_receive_next_request() won't start
any new request corout

nbd/server: Fix race in draining the export

When draining an NBD export, nbd_drained_begin() first sets
client->quiescing so that nbd_client_receive_next_request() won't start
any new request coroutines. Then nbd_drained_poll() tries to makes sure
that we wait for any existing request coroutines by checking that
client->nb_requests has become 0.

However, there is a small window between creating a new request
coroutine and increasing client->nb_requests. If a coroutine is in this
state, it won't be waited for and drain returns too early.

In the context of switching to a different AioContext, this means that
blk_aio_attached() will see client->recv_coroutine != NULL and fail its
assertion.

Fix this by increasing client->nb_requests immediately when starting the
coroutine. Doing this after the checks if we should create a new
coroutine is okay because client->lock is held.

Cc: qemu-stable@nongnu.org
Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server")
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20240314165825.40261-2-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

show more ...

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- virtio-blk: Multiqueue support (configurable iothread per queue)
- Made NBD export and hw/scsi thread-sa

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- virtio-blk: Multiqueue support (configurable iothread per queue)
- Made NBD export and hw/scsi thread-safe without AioContext lock
- Fix crash when loading snapshot on inactive node

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmWEw/8RHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9bX0Q/9G+Qx8mQGmbxJzvtW7/1eaeJ5CPCYT8w3
# 033S5hCil43mvX2aQKTFrh1Nz4aYlqMDyURvNu7nigyADY+kBpzzJ1MFr6WQrzYv
# QEk4jf/FOllfKn8+/A0z2NJDhtpVgqKKHBsFZl8FBUcxd79daTaoPPM3BNNsOHQD
# o7Z7hR/iEdG9dkAh/fpwctsgMO/CoN0BRRyN2OByj03zeu1TlDJ6lX0hxlcJl9Jw
# vLo81rWTCqKRu+SbjBsb0HfYE2hP54A4hvxn4I9vYGYDz8ElucluYyeqUEK+mdrX
# /DQBdb+Osl1FD6MuIaFR+Rgp9Mu5h6ZOdvUyCY0zuByti851hV8qjW9BtrTfqaMh
# LMOKoL6c5B8XJYWVGAGrJexIw1hHq5WKdXN9zp4FZA4tOyHUMRjHuR1+zScU6gnU
# WRSIQR46w75A13clWyJs9Hf/q5Fp/1KT4nfuZ/hmiXvxdsYY5x1w/W3s9tRNjYKL
# d6FVk17cFc6Ksb7lWvDCgg61BNZtGm4Clmw0kJ6V1reiQz7AvDLmduLUQbmrVt7G
# gWAY4b2L9YXJpEx5en0kE50KLAUw/E9ozbOq6ZT9nFUKeNAPC8PS5lK7vYVwebCk
# VA0t8pFzKhdB1bJaG5fMSRPBuqkvhsaDEEDABlSro8dyyjoQBaEdk5P9Kxe66hBc
# xhTmDPdv/JM=
# =E3Zh
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 21 Dec 2023 18:02:23 EST
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (33 commits)
virtio-blk: add iothread-vq-mapping parameter
qdev: add IOThreadVirtQueueMappingList property type
qdev-properties: alias all object class properties
string-output-visitor: show structs as "<omitted>"
block-coroutine-wrapper: use qemu_get_current_aio_context()
block: remove outdated AioContext locking comments
job: remove outdated AioContext locking comments
scsi: remove outdated AioContext lock comment
docs: remove AioContext lock from IOThread docs
aio: remove aio_context_acquire()/aio_context_release() API
aio-wait: draw equivalence between AIO_WAIT_WHILE() and AIO_WAIT_WHILE_UNLOCKED()
scsi: remove AioContext locking
block: remove bdrv_co_lock()
block: remove AioContext locking
graph-lock: remove AioContext locking
aio: make aio_context_acquire()/aio_context_release() a no-op
tests: remove aio_context_acquire() tests
scsi: assert that callbacks run in the correct AioContext
virtio-scsi: replace AioContext lock with tmf_bh_lock
dma-helpers: don't lock AioContext in dma_blk_cb()
...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

show more ...

nbd/server: introduce NBDClient->lock to protect fields

NBDClient has a number of fields that are accessed by both the export
AioContext and the main loop thread. When the AioContext lock is removed

nbd/server: introduce NBDClient->lock to protect fields

NBDClient has a number of fields that are accessed by both the export
AioContext and the main loop thread. When the AioContext lock is removed
these fields will need another form of protection.

Add NBDClient->lock and protect fields that are accessed by both
threads. Also add assertions where possible and otherwise add doc
comments stating assumptions about which thread and lock holding.

Note this patch moves the client->recv_coroutine assertion from
nbd_co_receive_request() to nbd_trip() where client->lock is held.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20231221192452.1785567-7-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

show more ...

nbd/server: only traverse NBDExport->clients from main loop thread

The NBD clients list is currently accessed from both the export
AioContext and the main loop thread. When the AioContext lock is re

nbd/server: only traverse NBDExport->clients from main loop thread

The NBD clients list is currently accessed from both the export
AioContext and the main loop thread. When the AioContext lock is removed
there will be nothing protecting the clients list.

Adding a lock around the clients list is tricky because NBDClient
structs are refcounted and may be freed from the export AioContext or
the main loop thread. nbd_export_request_shutdown() -> client_close() ->
nbd_client_put() is also tricky because the list lock would be held
while indirectly dropping references to NDBClients.

A simpler approach is to only allow nbd_client_put() and client_close()
calls from the main loop thread. Then the NBD clients list is only
accessed from the main loop thread and no fancy locking is needed.

nbd_trip() just needs to reschedule itself in the main loop AioContext
before calling nbd_client_put() and client_close(). This costs more CPU
cycles per NBD request so add nbd_client_put_nonzero() to optimize the
common case where more references to NBDClient remain.

Note that nbd_client_get() can still be called from either thread, so
make NBDClient->refcount atomic.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20231221192452.1785567-6-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

show more ...

nbd/server: avoid per-NBDRequest nbd_client_get/put()

nbd_trip() processes a single NBD request from start to finish and holds
an NBDClient reference throughout. NBDRequest does not outlive the scop

nbd/server: avoid per-NBDRequest nbd_client_get/put()

nbd_trip() processes a single NBD request from start to finish and holds
an NBDClient reference throughout. NBDRequest does not outlive the scope
of nbd_trip(). Therefore it is unnecessary to ref/unref NBDClient for
each NBDRequest.

Removing these nbd_client_get()/nbd_client_put() calls will make
thread-safety easier in the commits that follow.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <20231221192452.1785567-5-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

show more ...

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- Graph locking part 6 (bs->file/backing)
- ahci: trigger either error IRQ or regular IRQ, not both

# ---

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- Graph locking part 6 (bs->file/backing)
- ahci: trigger either error IRQ or regular IRQ, not both

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmVLvccRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9ZkFg//awQoPiGnYzHpqcx2tGCM2AqBV+mFkbZr
# BKI5vp8FYfJtgMuHjC8jabL24NRMPpT+HbCzoxwjJU+nnnr85qr7R5iGwG6kfgX6
# HJlAXYXdY6e7l+FV44PBJ52vOoGCsh1GHg8HlKsHMaxSdXi9C1axHJ6rCAjnWXE0
# FQ4znCBVs/9HiKsvu4Wdm5muX2ShftFRM/toAwA+fLEOealX8WEXoRFJXI40bYbR
# OR7aJXWMDQrljlqdKk2FXvK337/tpofXmXf3NIE1R2pmY4x5Fg8bfChZn4UaaCdN
# n+0AhmE4ScI0rXuaXXYOvTO9vdTzXeBROG6tX03t9rrQfB6wPcGVeXRo/uusslAW
# sDH8NLz7uHFOooV02Fs8CKDdVrNNw5qjziclSGa0Po7vqOV1TKI8OTiNpsDLmdI5
# +DQvC6N+IU1pSOXImATSHkheGWggsegrsgN6PdrlzHEXJwWoAaRD0T06MRn74/pL
# gCegK2ez4RJYsci7C5b0gaqY/QBsMj8EUfEGVHvVyuVSoPRwiq4ehPqSQ+siA3xP
# KxYR0e4+QIfRmxqCzaJhiQ3DDGdt8UcO3yF0XcKXEqWwgFAGQKNeUG314jginvmA
# iaJzC0dHbiGcagAk7Ey8iyzfxQDWM6ixzJtGv7VLILepzCuu8vaJXy5qeEtTM/ZI
# EXoDGceNSvw=
# =ikBW
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 09 Nov 2023 00:56:39 HKT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (25 commits)
hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
block: Protect bs->file with graph_lock
block: Take graph lock for most of .bdrv_open
vhdx: Take locks for accessing bs->file
qcow2: Take locks for accessing bs->file
block: Add missing GRAPH_RDLOCK annotations
block: Introduce bdrv_co_change_backing_file()
blkverify: Add locking for request_fn
block: Protect bs->backing with graph_lock
block: Mark bdrv_replace_node() GRAPH_WRLOCK
block: Mark bdrv_replace_node_common() GRAPH_WRLOCK
block: Inline bdrv_set_backing_noperm()
block: Mark bdrv_set_backing_hd_drained() GRAPH_WRLOCK
block: Mark bdrv_cow_child() and callers GRAPH_RDLOCK
block: Mark bdrv_filter_child() and callers GRAPH_RDLOCK
block: Mark bdrv_chain_contains() and callers GRAPH_RDLOCK
block: Mark bdrv_(un)freeze_backing_chain() and callers GRAPH_RDLOCK
block: Mark bdrv_skip_filters() and callers GRAPH_RDLOCK
block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK
block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK
...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

show more ...

block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because
it

block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_filter_or_cow_bs() need to hold a reader lock for the graph because
it calls bdrv_filter_or_cow_child(), which accesses bs->file/backing.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20231027155333.420094-7-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

show more ...

Merge tag 'pull-nbd-2023-10-05' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2023-10-05

- various: mailmap cleanups
- Eric Blake: enable use of NBD 64-bit extended headers

# -----

Merge tag 'pull-nbd-2023-10-05' of https://repo.or.cz/qemu/ericb into staging

NBD patches for 2023-10-05

- various: mailmap cleanups
- Eric Blake: enable use of NBD 64-bit extended headers

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmUfA94ACgkQp6FrSiUn
# Q2o2hAf/Q4q6RoEG9WoOIa6WB1nk9tZN6GRfW4jS+09hJTFVbYUhJOcvQwwZjPT6
# 6oIwde8w7uE+AxBOA3XPbgTOBnTnpt2RH2AYVctNYB2vonuSrx3/KE3XSi4nrJRo
# +XiPbOsDzQu+vYsI7XJ+5e13BE4iZVmzbyke0U6hO88uR6tQstDJV1Mhem4jIbCG
# uLzkVDs3yY5sUFoUe77lFGWgKqfh87eJXV18T76df97ZZ5O2/w9G8MknG/CTSsR0
# fmzzC1Q4a2UEFtX8M3etRQ/b5WbFYhM+XwFevm2YBpod89ejGEA0ohhpn+GpZDU9
# SU66lXL/5jM9N7RkiSFwvupaot9hvw==
# =UieX
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 05 Oct 2023 14:43:42 EDT
# 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

* tag 'pull-nbd-2023-10-05' of https://repo.or.cz/qemu/ericb:
nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS
nbd/server: Prepare for per-request filtering of BLOCK_STATUS
nbd/server: Refactor list of negotiated meta contexts
nbd/client: Request extended headers during negotiation
nbd/client: Accept 64-bit block status chunks
nbd/client: Initial support for extended headers
nbd/client: Plumb errp through nbd_receive_replies
nbd/server: Enable initial support for extended headers
nbd/server: Support 64-bit block status
nbd/server: Prepare to send extended header replies
nbd/server: Prepare to receive extended header requests
nbd/server: Support a request payload
mailmap: Fix BALATON Zoltan author email
maint: Tweak comments in mailmap regarding SPF
mailmap: Fix Andrey Drobyshev author email

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

show more ...

nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS

Allow a client to request a subset of negotiated meta contexts. For
example, a client may ask to use a single connection to learn about
both

nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS

Allow a client to request a subset of negotiated meta contexts. For
example, a client may ask to use a single connection to learn about
both block status and dirty bitmaps, but where the dirty bitmap
queries only need to be performed on a subset of the disk; forcing the
server to compute that information on block status queries in the rest
of the disk is wasted effort (both at the server, and on the amount of
traffic sent over the wire to be parsed and ignored by the client).

Qemu as an NBD client never requests to use more than one meta
context, so it has no need to use block status payloads. Testing this
instead requires support from libnbd, which CAN access multiple meta
contexts in parallel from a single NBD connection; an interop test
submitted to the libnbd project at the same time as this patch
demonstrates the feature working, as well as testing some corner cases
(for example, when the payload length is longer than the export
length), although other corner cases (like passing the same id
duplicated) requires a protocol fuzzer because libnbd is not wired up
to break the protocol that badly.

This also includes tweaks to 'qemu-nbd --list' to show when a server
is advertising the capability, and to the testsuite to reflect the
addition to that output.

Of note: qemu will always advertise the new feature bit during
NBD_OPT_INFO if extended headers have alreay been negotiated
(regardless of whether any NBD_OPT_SET_META_CONTEXT negotiation has
occurred); but for NBD_OPT_GO, qemu only advertises the feature if
block status is also enabled (that is, if the client does not
negotiate any contexts, then NBD_CMD_BLOCK_STATUS cannot be used, so
the feature is not advertised).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230925192229.3186470-26-eblake@redhat.com>
[eblake: fix logic to reject unnegotiated contexts]
Signed-off-by: Eric Blake <eblake@redhat.com>

show more ...

nbd/server: Prepare for per-request filtering of BLOCK_STATUS

The next commit will add support for the optional extension
NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
reque

nbd/server: Prepare for per-request filtering of BLOCK_STATUS

The next commit will add support for the optional extension
NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
request that the server only return a subset of negotiated contexts,
rather than all contexts. To make that task easier, this patch
populates the list of contexts to return on a per-command basis (for
now, identical to the full set of negotiated contexts).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230925192229.3186470-25-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

show more ...

nbd/server: Refactor list of negotiated meta contexts

Peform several minor refactorings of how the list of negotiated meta
contexts is managed, to make upcoming patches easier: Promote the
internal

nbd/server: Refactor list of negotiated meta contexts

Peform several minor refactorings of how the list of negotiated meta
contexts is managed, to make upcoming patches easier: Promote the
internal type NBDExportMetaContexts to the public opaque type
NBDMetaContexts, and mark exp const. Use a shorter member name in
NBDClient. Hoist calls to nbd_check_meta_context() earlier in their
callers, as the number of negotiated contexts may impact the flags
exposed in regards to an export, which in turn requires a new
parameter. Drop a redundant parameter to nbd_negotiate_meta_queries.
No semantic change intended on the success path; on the failure path,
dropping context in nbd_check_meta_export even when reporting an error
is safer.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230925192229.3186470-24-eblake@redhat.com>

show more ...

nbd/server: Enable initial support for extended headers

Time to start supporting clients that request extended headers. Now
we can finally reach the code added across several previous patches.

Eve

nbd/server: Enable initial support for extended headers

Time to start supporting clients that request extended headers. Now
we can finally reach the code added across several previous patches.

Even though the NBD spec has been altered to allow us to accept
NBD_CMD_READ larger than the max payload size (provided our response
is a hole or broken up over more than one data chunk), we are not
planning to take advantage of that, and continue to cap NBD_CMD_READ
to 32M regardless of header size.

For NBD_CMD_WRITE_ZEROES and NBD_CMD_TRIM, the block layer already
supports 64-bit operations without any effort on our part. For
NBD_CMD_BLOCK_STATUS, the client's length is a hint, and the previous
patch took care of implementing the required
NBD_REPLY_TYPE_BLOCK_STATUS_EXT.

We do not yet support clients that want to do request payload
filtering of NBD_CMD_BLOCK_STATUS; that will be added in later
patches, but is not essential for qemu as a client since qemu only
requests the single context base:allocation.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230925192229.3186470-19-eblake@redhat.com>

show more ...

nbd/server: Support 64-bit block status

The NBD spec states that if the client negotiates extended headers,
the server must avoid NBD_REPLY_TYPE_BLOCK_STATUS and instead use
NBD_REPLY_TYPE_BLOCK_STA

nbd/server: Support 64-bit block status

The NBD spec states that if the client negotiates extended headers,
the server must avoid NBD_REPLY_TYPE_BLOCK_STATUS and instead use
NBD_REPLY_TYPE_BLOCK_STATUS_EXT which supports 64-bit lengths, even if
the reply does not need more than 32 bits. As of this patch,
client->mode is still never NBD_MODE_EXTENDED, so the code added here
does not take effect until the next patch enables negotiation.

For now, all metacontexts that we know how to export never populate
more than 32 bits of information, so we don't have to worry about
NBD_REP_ERR_EXT_HEADER_REQD or filtering during handshake, and we
always send all zeroes for the upper 32 bits of status during
NBD_CMD_BLOCK_STATUS.

Note that we previously had some interesting size-juggling on call
chains, such as:

nbd_co_send_block_status(uint32_t length)
-> blockstatus_to_extents(uint32_t bytes)
-> bdrv_block_status_above(bytes, &uint64_t num)
-> nbd_extent_array_add(uint64_t num)
-> store num in 32-bit length

But we were lucky that it never overflowed: bdrv_block_status_above
never sets num larger than bytes, and we had previously been capping
'bytes' at 32 bits (since the protocol does not allow sending a larger
request without extended headers). This patch adds some assertions
that ensure we continue to avoid overflowing 32 bits for a narrow
client, while fully utilizing 64-bits all the way through when the
client understands that. Even in 64-bit math, overflow is not an
issue, because all lengths are coming from the block layer, and we
know that the block layer does not support images larger than off_t
(if lengths were coming from the network, the story would be
different).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230925192229.3186470-18-eblake@redhat.com>

show more ...

nbd/server: Prepare to send extended header replies

Although extended mode is not yet enabled, once we do turn it on, we
need to reply with extended headers to all messages. Update the low
level en

nbd/server: Prepare to send extended header replies

Although extended mode is not yet enabled, once we do turn it on, we
need to reply with extended headers to all messages. Update the low
level entry points necessary so that all other callers automatically
get the right header based on the current mode.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230925192229.3186470-17-eblake@redhat.com>

show more ...

nbd/server: Prepare to receive extended header requests

Although extended mode is not yet enabled, once we do turn it on, we
need to accept extended requests for all messages. Previous patches
have

nbd/server: Prepare to receive extended header requests

Although extended mode is not yet enabled, once we do turn it on, we
need to accept extended requests for all messages. Previous patches
have already taken care of supporting 64-bit lengths, now we just need
to read it off the wire.

Note that this implementation will block indefinitely on a buggy
client that sends a non-extended payload (that is, we try to read a
full packet before we ever check the magic number, but a client that
mistakenly sends a simple request after negotiating extended headers
doesn't send us enough bytes), but it's no different from any other
client that stops talking to us partway through a packet and thus not
worth coding around.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230925192229.3186470-16-eblake@redhat.com>

show more ...