On 5/19/20 2:55 PM, Vladimir Sementsov-Ogievskiy wrote:
These cases are fixed by previous patches around block_status and
is_allocated.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
tests/qemu-iotests/274 | 20 ++++++++++++
tests/qemu-iotests/274.out | 65 ++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+)
Okay, so this test fails when applied in isolation without the rest of your
series.
diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274
index 5d1bf34dff..e910455f13 100755
--- a/tests/qemu-iotests/274
+++ b/tests/qemu-iotests/274
@@ -115,6 +115,26 @@ with iotests.FilePath('base') as base, \
iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, mid)
iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff),
mid)
+ iotests.log('=== Testing qemu-img commit (top -> base) ===')
+
+ create_chain()
+ iotests.qemu_img_log('commit', '-b', base, top)
+ iotests.img_info_log(base)
+ iotests.qemu_io_log('-c', 'read -P 1 0 %d' % size_short, base)
+ iotests.qemu_io_log('-c', 'read -P 0 %d %d' % (size_short, size_diff),
base)
So if I understand it, we are going from:
base 11111111
mid ----
top --------
guest 11110000
and we want to go to:
base 11110000
except that we are not properly writing the zeroes into base, because we
grabbed the wrong status, ending up with:
base 11111111
The status of top from 1M onwards is unallocated, and if we were to commit to
just mid, Kevin's truncate fixes solve that (we now zero out the tail of mid as
part of resizing it to be large enough). But you are instead skipping mid, and
committing all the way to base. So we need _something_ that can tell qemu-img
commit that even though the region 1m-2m is unallocated in top, we must behave
as though the status of mid reports it as allocated (because when reading
beyond EOF in mid, we DO read zero). Since the data is allocated not in top,
but acts as though it was allocated in mid, which is above base, then the
commit operation has to do something to preserve that allocation.
Okay, you've convinced me we have a bug. > However, I'm still not sold that
patches 1 and 4 are quite the right fix. Going back to the original setup,
unpatched qemu.git head reports:
$ ./qemu-img map --output=json top.qcow2
[{ "start": 0, "length": 1048576, "depth": 2, "zero": false, "data": true,
"offset": 327680},
{ "start": 1048576, "length": 1048576, "depth": 0, "zero": true, "data": false}]
I think what we really want is:
[{ "start": 0, "length": 1048576, "depth": 2, "zero": false, "data": true,
"offset": 327680},
{ "start": 1048576, "length": 1048576, "depth": 1, "zero": true, "data": false}]
because then we would be _accurately_ reporting that the zeroes that we read from 1m-2m come _because_ we read from mid (beyond EOF), which is different from our current answer that the zeroes come from top (they don't, because top deferred to mid).