Rich reported a bug when using qemu as client to nbdkit serving
a non-sector-aligned image; in turn, I found a second bug with
qemu as server of such an image.

Both bugs were present in 2.12, and thus are not new regressions
in 3.0. If there is a reason to spin -rc4, then these could be
included; but this series alone is not a driving reason to cause
-rc4.

Eric Blake (4):
  block: Add bdrv_get_request_alignment()
  nbd/server: Advertise actual minimum block size
  iotests: Add 228 to test NBD on unaligned images
  nbd/client: Deal with unaligned size from server

 include/sysemu/block-backend.h |  1 +
 block/block-backend.c          |  7 +++
 block/nbd.c                    | 11 ++++-
 nbd/server.c                   | 10 +++--
 tests/qemu-iotests/228         | 96 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/228.out     |  8 ++++
 tests/qemu-iotests/group       |  1 +
 7 files changed, 129 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/228
 create mode 100644 tests/qemu-iotests/228.out

-- 
2.14.4

Hi, has this series been superseded or do we need a respin for 3.1?

(It's not quite been a month, but it caught my eye.)

--js

On 08/02/2018 10:48 AM, Eric Blake wrote:
> Rich reported a bug when using qemu as client to nbdkit serving
> a non-sector-aligned image; in turn, I found a second bug with
> qemu as server of such an image.
> 
> Both bugs were present in 2.12, and thus are not new regressions
> in 3.0. If there is a reason to spin -rc4, then these could be
> included; but this series alone is not a driving reason to cause
> -rc4.
> 
> Eric Blake (4):
>   block: Add bdrv_get_request_alignment()
>   nbd/server: Advertise actual minimum block size
>   iotests: Add 228 to test NBD on unaligned images
>   nbd/client: Deal with unaligned size from server
> 
>  include/sysemu/block-backend.h |  1 +
>  block/block-backend.c          |  7 +++
>  block/nbd.c                    | 11 ++++-
>  nbd/server.c                   | 10 +++--
>  tests/qemu-iotests/228         | 96 ++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/228.out     |  8 ++++
>  tests/qemu-iotests/group       |  1 +
>  7 files changed, 129 insertions(+), 5 deletions(-)
>  create mode 100755 tests/qemu-iotests/228
>  create mode 100644 tests/qemu-iotests/228.out
> 

On 9/10/18 2:25 PM, John Snow wrote:
> Hi, has this series been superseded or do we need a respin for 3.1?

Needs a respin.

There are two issues still to be solved:

1. Asking the top-most block layer for its alignment isn't necessarily 
right for qemu as server.  If we have:

backing (512) <- active (4k)

and tell the client that they can't access anything smaller than 4k 
granularity, but then read through to the backing layer which does just 
that, then we're wrong.  Either the block layer has to be beefed up to 
make bdrv_block_status() round backing file's smaller granularity into 
the size we advertised (perhaps by adding a flag to bdrv_block_status() 
to declare whether we prefer the most accurate information vs. the 
rounded information) - or else qemu as NBD server should ALWAYS 
advertise a blocksize of 1-512 (1 because we can, even if by 
read-modify-write; or 512 because except for EOF issues on a 
non-sector-aligned file, we don't encounter mid-sector transitions 
anywhere else, and EOF is easier to special case than everywhere).

2. Patch 5 fixes qemu as client to not round valid requests past EOF, 
but does not fix the case of a request that intentionally exceeds EOF 
but fits within the rounded file size from reaching the server. Either 
the NBD client code should itself perform EOF validation (reads from the 
EOF hole see zero, writes of anything other than zero fail with ENOSPC), 
or the NBD client code should round the server's file size down instead 
of up.  I haven't decided which approach is better.

But right now, these fixes are taking a back seat to me trying to get a 
libvirt incremental backup demo working.

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