From 00edcf411c7d8e58faa15f2cb07829b8f7f74a60 Mon Sep 17 00:00:00 2001

From: John Snow <jsnow@redhat.com>

Date: Mon, 6 May 2019 17:56:16 +0200

Subject: [PATCH 06/53] nbd-client: Work around server BLOCK_STATUS

 misalignment at EOF

RH-Author: John Snow <jsnow@redhat.com>

Message-id: <20190506175629.11079-7-jsnow@redhat.com>

Patchwork-id: 87194

O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 06/19] nbd-client: Work around server BLOCK_STATUS misalignment at EOF

Bugzilla: 1692018

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>

RH-Acked-by: Thomas Huth <thuth@redhat.com>

From: Eric Blake <eblake@redhat.com>

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 <eblake@redhat.com>

Message-Id: <20190326171317.4036-1-eblake@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

(cherry picked from commit 737d3f524481bb2ef68d3eba1caa636ff143e16a)

Signed-off-by: John Snow <jsnow@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 block/nbd-client.c | 30 ++++++++++++++++++++++++++----

 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c

index eee909f..09e20b2 100644

--- a/block/nbd-client.c

+++ b/block/nbd-client.c

@@ -256,15 +256,37 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client,

     extent->length = payload_advance32(&payload);

     extent->flags = payload_advance32(&payload);

-    if (extent->length == 0 ||

-        (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,

-                                                    client->info.min_block))) {

+    if (extent->length == 0) {

         error_setg(errp, "Protocol error: server sent status chunk with "

-                   "invalid length");

+                   "zero length");

         return -EINVAL;

     }

     /*

+     * A server sending unaligned block status is in violation of the

+     * protocol, but as qemu-nbd 3.1 is such a server (at least for

+     * POSIX files that are not a multiple of 512 bytes, since qemu

+     * rounds files up to 512-byte multiples but lseek(SEEK_HOLE)

+     * still sees an implicit hole beyond the real EOF), it's nicer to

+     * work around the misbehaving server. If the request included

+     * more than the final unaligned block, truncate it back to an

+     * aligned result; if the request was only the final block, round

+     * up to the full block and change the status to fully-allocated

+     * (always a safe status, even if it loses information).

+     */

+    if (client->info.min_block && !QEMU_IS_ALIGNED(extent->length,

+                                                   client->info.min_block)) {

+        trace_nbd_parse_blockstatus_compliance("extent length is unaligned");

+        if (extent->length > client->info.min_block) {

+            extent->length = QEMU_ALIGN_DOWN(extent->length,

+                                             client->info.min_block);

+        } else {

+            extent->length = client->info.min_block;

+            extent->flags = 0;

+        }

+    }

+

+    /*

      * We used NBD_CMD_FLAG_REQ_ONE, so the server should not have

      * sent us any more than one extent, nor should it have included

      * status beyond our request in that extent. However, it's easy

-- 

1.8.3.1