Re: [Qemu-block] [Qemu-devel] [PATCH for-4.0] nbd-client: Work around server BLOCK_STATUS misalignment at EOF

[Eric Blake]

On 3/26/19 12:13 PM, Eric Blake wrote:
> The NBD spec is clear that a server that advertises a minimum block
> size should reply to NBD_CMD_BLOCK_STATUS with extents aligned
> accordingly. However, we know that the qemu NBD server implementation
> has had a corner-case bug where it is not compliant with the spec,
> present since the introduction of NBD_CMD_BLOCK_STATUS in qemu 2.12
> (and unlikely to be patched in time for 4.0). Namely, when qemu is
> serving a file that is not a multiple of 512 bytes, it rounds the size
> advertised over NBD up to the next sector boundary (someday, I'd like
> to fix that to be byte-accurate, but it's a much bigger audit not
> appropriate for this release); yet if the final sector contains data
> prior to EOF, lseek(SEEK_HOLE) will point to the implicit hole
> mid-sector which qemu then reported over NBD.
> 
> We are well within our rights to hang up on a server that can't follow
> the spec, but it is more useful to try and keep the connection alive
> in spite of the problem. Do so by tracing a message about the problem,
> and then either truncating the request back to an aligned boundary (if
> it covered more than the final sector) or widening it out to the full
> boundary with a forced status of data (since truncating would result
> in 0 bytes, but we have to make progress, and valid since data is a
> default-safe answer). And in practice, since the problem only happens
> on a sector that starts with data and ends with a hole, we are going
> to want to read that full sector anyway (where qemu as the server
> fills in the tail beyond EOF with appropriate NUL bytes).
> 
> Easy reproduction:
> $ printf %1000d 1 > file
> $ qemu-nbd -f raw -t file & pid=$!
> $ qemu-img map --output=json -f raw nbd://localhost:10809
> qemu-img: Could not read file metadata: Invalid argument
> $ kill $pid
> 
> where the patched version instead succeeds with:
> [{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>  block/nbd-client.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)

I haven't seen patchew reports in a few days. But just in case it tries
to apply this,

Based-on: <address@hidden>
[v2 0/2] nbd: simple errors to BLOCK_STATUS

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

signature.asc
Description: OpenPGP digital signature