[v2] nbd: accept a large NBD_CMD_CACHE, cover the transmission commands

[PATCH v2 0/5] nbd: accept a large NBD_CMD_CACHE, cover the transmission commands

Posted by Denis V. Lunev 1 month ago

NBD_CMD_CACHE carries no payload in either direction, yet the server
rejects any effect length above the advertised maximum payload size.
The specification explicitly allows the oversize request, and we
already accept it for NBD_CMD_TRIM and NBD_CMD_WRITE_ZEROES. Patch 2
drops the check.

The rest of the series adds tests/qemu-iotests/tests/nbd-commands, a
small client which speaks the NBD transmission phase itself. Nothing
in the tree does that today, so the protocol surface of the server -
reply modes, command flags, error codes, sparse and extended replies -
is covered only as far as our own client happens to exercise it. It is
useful both as coverage and as a regression tool.

Changes since v1:
- the libnbd handle now lives in setUp() and tearDown() instead of a
  context manager entered by every test case, which drops a level of
  indent everywhere (Vladimir)
- one block_status() helper returning a context to extents map, in
  place of the three near identical extent collecting callbacks
  (Vladimir)
- the unused callback arguments are named with a leading underscore,
  so that iotest 297 stays pylint clean; v1 added eight
  unused-argument warnings, which would have failed 297 wherever
  pylint is installed
- Vladimir's Reviewed-by is kept on patch 2, which is unchanged, and
  dropped from the four test patches, since the two points above
  rewrote most of the test

v1: https://lore.kernel.org/qemu-devel/20260826165747.3768674-1-den@openvz.org/

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Denis V. Lunev (5):
  iotests: add coverage for NBD transmission commands
  nbd/server: accept NBD_CMD_CACHE above the maximum payload size
  iotests/nbd-commands: exercise the simple and structured reply modes
  iotests/nbd-commands: cover NBD_CMD_BLOCK_STATUS with a payload
  iotests/nbd-commands: cover the command flags and sparse replies

 nbd/server.c                              |   4 +-
 tests/qemu-iotests/tests/nbd-commands     | 244 ++++++++++++++++++++++
 tests/qemu-iotests/tests/nbd-commands.out |   5 +
 3 files changed, 250 insertions(+), 3 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/nbd-commands
 create mode 100644 tests/qemu-iotests/tests/nbd-commands.out


base-commit: bde2492aace2b5acb755a5b057013e915163a77f
-- 
2.53.0

Re: [PATCH v2 0/5] nbd: accept a large NBD_CMD_CACHE, cover the transmission commands

Posted by Vladimir Sementsov-Ogievskiy 3 weeks, 5 days ago

On 27.08.26 19:09, Denis V. Lunev wrote:
> NBD_CMD_CACHE carries no payload in either direction, yet the server
> rejects any effect length above the advertised maximum payload size.
> The specification explicitly allows the oversize request, and we
> already accept it for NBD_CMD_TRIM and NBD_CMD_WRITE_ZEROES. Patch 2
> drops the check.
> 
> The rest of the series adds tests/qemu-iotests/tests/nbd-commands, a
> small client which speaks the NBD transmission phase itself. Nothing
> in the tree does that today, so the protocol surface of the server -
> reply modes, command flags, error codes, sparse and extended replies -
> is covered only as far as our own client happens to exercise it. It is
> useful both as coverage and as a regression tool.
> 
> Changes since v1:
> - the libnbd handle now lives in setUp() and tearDown() instead of a
>    context manager entered by every test case, which drops a level of
>    indent everywhere (Vladimir)
> - one block_status() helper returning a context to extents map, in
>    place of the three near identical extent collecting callbacks
>    (Vladimir)
> - the unused callback arguments are named with a leading underscore,
>    so that iotest 297 stays pylint clean; v1 added eight
>    unused-argument warnings, which would have failed 297 wherever
>    pylint is installed
> - Vladimir's Reviewed-by is kept on patch 2, which is unchanged, and
>    dropped from the four test patches, since the two points above
>    rewrote most of the test
> 
> v1:https://lore.kernel.org/qemu-devel/20260826165747.3768674-1-den@openvz.org/
> 
> Signed-off-by: Denis V. Lunev<den@openvz.org>
> CC: Eric Blake<eblake@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>

Thanks,

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

and applied to my block branch.

-- 
Best regards,
Vladimir