diff mbox series

[v3,3/6] nbd/client: Report offsets in bdrv_block_status

Message ID 20190329042750.14704-4-eblake@redhat.com
State New
Headers show
Series NBD server alignment improvement | expand

Commit Message

Eric Blake March 29, 2019, 4:27 a.m. UTC 
It is desirable for 'qemu-img map' to have the same output for a file
whether it is served over file or nbd protocols. However, ever since
we implemented block status for NBD (2.12), the NBD protocol forgot to
inform the block layer that as the final layer in the chain, the
offset is valid; without an offset, the human-readable form of
qemu-img map gives up with the unhelpful:

$ nbdkit -U - data data="1" size=512 --run 'qemu-img map $nbd'
Offset          Length          Mapped to       File
qemu-img: File contains external, encrypted or compressed clusters.

The --output=json form always works, because it is reporting the
lower-level bdrv_block_status results directly rather than trying to
filter out sparse ranges for human consumption.

With this patch, the output changes to:

Offset          Length          Mapped to       File
0               0x200           0               nbd+unix://?socket=/tmp/nbdkitOxeoLa/socket

This change is observable to several iotests.

Fixes: 78a33ab5
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nbd-client.c         |  9 +++++++--
 tests/qemu-iotests/209.out |  4 ++--
 tests/qemu-iotests/223.out | 18 +++++++++---------
 tests/qemu-iotests/241.out |  2 +-
 4 files changed, 19 insertions(+), 14 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy March 29, 2019, 2 p.m. UTC | #1 
29.03.2019 7:27, Eric Blake wrote:
> It is desirable for 'qemu-img map' to have the same output for a file
> whether it is served over file or nbd protocols. However, ever since
> we implemented block status for NBD (2.12), the NBD protocol forgot to
> inform the block layer that as the final layer in the chain, the
> offset is valid; without an offset, the human-readable form of
> qemu-img map gives up with the unhelpful:
> 
> $ nbdkit -U - data data="1" size=512 --run 'qemu-img map $nbd'
> Offset          Length          Mapped to       File
> qemu-img: File contains external, encrypted or compressed clusters.
> 
> The --output=json form always works, because it is reporting the
> lower-level bdrv_block_status results directly rather than trying to
> filter out sparse ranges for human consumption.
> 
> With this patch, the output changes to:
> 
> Offset          Length          Mapped to       File
> 0               0x200           0               nbd+unix://?socket=/tmp/nbdkitOxeoLa/socket
> 
> This change is observable to several iotests.
> 
> Fixes: 78a33ab5
> Reported-by: Richard W.M. Jones<rjones@redhat.com>
> Signed-off-by: Eric Blake<eblake@redhat.com>


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

Patch

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 150af9cc46f..3edb508f668 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -972,7 +972,9 @@  int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs,

     if (!client->info.base_allocation) {
         *pnum = bytes;
-        return BDRV_BLOCK_DATA;
+        *map = offset;
+        *file = bs;
+        return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
     }

     ret = nbd_co_send_request(bs, &request, NULL);
@@ -995,8 +997,11 @@  int coroutine_fn nbd_client_co_block_status(BlockDriverState *bs,

     assert(extent.length);
     *pnum = extent.length;
+    *map = offset;
+    *file = bs;
     return (extent.flags & NBD_STATE_HOLE ? 0 : BDRV_BLOCK_DATA) |
-           (extent.flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0);
+        (extent.flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0) |
+        BDRV_BLOCK_OFFSET_VALID;
 }

 void nbd_client_detach_aio_context(BlockDriverState *bs)
diff --git a/tests/qemu-iotests/209.out b/tests/qemu-iotests/209.out
index 0d29724e84a..214e27bfcec 100644
--- a/tests/qemu-iotests/209.out
+++ b/tests/qemu-iotests/209.out
@@ -1,2 +1,2 @@ 
-[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true},
-{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false}]
+[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true, "offset": 0},
+{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false, "offset": 524288}]
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index 95c40a17ad7..7828a447392 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -67,18 +67,18 @@  read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 2097152/2097152 bytes at offset 2097152
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true},
-{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false},
-{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
+{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false},
-{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true},
+{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]

 === Contrast to small granularity dirty-bitmap ===

-[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true},
+[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false},
-{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true},
+{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]

 === End qemu NBD server ===
@@ -94,10 +94,10 @@  read 2097152/2097152 bytes at offset 2097152
 === Use qemu-nbd as server ===

 [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false},
-{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true},
+{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
-[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true},
+[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false},
-{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true},
+{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
 *** done
diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
index 044afc0c6f8..91dcde97560 100644
--- a/tests/qemu-iotests/241.out
+++ b/tests/qemu-iotests/241.out
@@ -2,6 +2,6 @@  QA output created by 241

 === Exporting unaligned raw image ===

-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
+[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 1 KiB (0x400) bytes     allocated at offset 0 bytes (0x0)
 *** done