We've had some known long-standing compliance bugs in our NBD server
not always honoring a minimum block size in its response to client
requests, when dealing with an image with a large block size backed by
another image with a smaller block size (for example, an encrypted
qcow2 image has a minimum block size of 512, backed by a file whose
size is not a multiple of 512).  Fragmenting our reply to NBD_CMD_READ
or NBD_CMD_BLOCK_STATUS to something smaller than our advertised
minimum block size can confuse a client (in fact, qemu 3.2 would
abort() on such messages, although we patched the client to be
tolerant of non-compliant servers for qemu 4.0).  Thankfully, most
day-to-day uses of NBD don't run into these cases.

Back in 2019, I did propose a fix for the server[1], but it was
incomplete at the time because I couldn't write a reliable iotest
(using blkdebug failed, because it was a filter that blocked access to
exposing the dirty bitmap), and the patches were too close to the 4.0
release for a corner case that did not occur frequently in practice,
so it moved to my back burner.  But now that we have fixed the ability
to see through the blkdebug filter, and have in the meantime added the
qemu:allocation-depth context that also suffers from the same problem,
I have finally updated this series to a state that I'm happy with.

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00589.html

Patch 5 is merely to aid in testing, by disabling the client
workaround that has been present since qemu 4.0 (since the revert is
no longer trivial).

Eric Blake (5):
  iotests: Update 241 to expose backing layer fragmentation
  block: Fix BDRV_BLOCK_RAW status to honor alignment
  nbd/server: Avoid unaligned read/block_status from backing
  nbd/server: Avoid unaligned dirty-bitmap status
  do not apply: Revert "nbd-client: Work around server BLOCK_STATUS
    misalignment at EOF"

 block/coroutines.h         |   2 +
 include/block/block.h      |   2 +
 block/io.c                 | 210 ++++++++++++++++++++++++++++++++++---
 block/nbd.c                |  30 +-----
 block/quorum.c             |   7 +-
 nbd/server.c               |  42 ++++++--
 tests/qemu-iotests/221     |  13 +++
 tests/qemu-iotests/221.out |   9 ++
 tests/qemu-iotests/241     |  58 +++++++++-
 tests/qemu-iotests/241.out |  24 ++++-
 10 files changed, 337 insertions(+), 60 deletions(-)

-- 
2.30.1

Patchew URL: https://patchew.org/QEMU/20210218201528.127099-1-eblake@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210218201528.127099-1-eblake@redhat.com
Subject: [PATCH 0/5] Obey NBD spec regarding block size bounds on reply

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210218201528.127099-1-eblake@redhat.com -> patchew/20210218201528.127099-1-eblake@redhat.com
Switched to a new branch 'test'
7be641f do not apply: Revert "nbd-client: Work around server BLOCK_STATUS misalignment at EOF"
93caf2c nbd/server: Avoid unaligned dirty-bitmap status
f9b0b64 nbd/server: Avoid unaligned read/block_status from backing
c990e1b block: Fix BDRV_BLOCK_RAW status to honor alignment
247619c iotests: Update 241 to expose backing layer fragmentation

=== OUTPUT BEGIN ===
1/5 Checking commit 247619cd55cb (iotests: Update 241 to expose backing layer fragmentation)
2/5 Checking commit c990e1bc847b (block: Fix BDRV_BLOCK_RAW status to honor alignment)
3/5 Checking commit f9b0b646002f (nbd/server: Avoid unaligned read/block_status from backing)
4/5 Checking commit 93caf2c5d549 (nbd/server: Avoid unaligned dirty-bitmap status)
5/5 Checking commit 7be641fea748 (do not apply: Revert "nbd-client: Work around server BLOCK_STATUS misalignment at EOF")
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 41 lines checked

Patch 5/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210218201528.127099-1-eblake@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On 2/18/21 2:33 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20210218201528.127099-1-eblake@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 

> 5/5 Checking commit 7be641fea748 (do not apply: Revert "nbd-client: Work around server BLOCK_STATUS misalignment at EOF")
> ERROR: Missing Signed-off-by: line(s)

Intentional since patch 5 is not part of the final series. (actually,
you can apply patch 5 first to see failures, then 1-4 to see how the
failures are fixed even without the interference of client-side
workarounds).  But at the end of the day, once 1-4 are upstream, we want
to keep the client-side workarounds for communication with older qemu or
with other NBD servers that have a similar compliance problem as what
gets fixed here.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org