Re: [PATCH] iotests/MRCE: Write data to source

[Hanna Reitz <hreitz@redhat.com>]

On 23.12.21 18:50, Vladimir Sementsov-Ogievskiy wrote:
> 23.12.2021 19:53, Hanna Reitz wrote:
>> This test assumes that mirror flushes the source when entering the READY
>> state, and that the format level will pass that flush on to the protocol
>> level (where we intercept it with blkdebug).
>>
>> However, apparently that does not happen when using a VMDK image with
>> zeroed_grain=on, which actually is the default set by testenv.py.  Right
>> now, Python tests ignore IMGOPTS, though, so this has no effect; but
>> Vladimir has a series that will change this, so we need to fix this test
>> before that series lands.
>>
>> We can fix it by writing data to the source before we start the mirror
>> job; apparently that makes the (VMDK) format layer change its mind and
>> pass on the pre-READY flush to the protocol level, so the test passes
>> again.  (I presume, without any data written, mirror just does a 64M
>> zero write on the target, which VMDK with zeroed_grain=on basically just
>> ignores.)
>>
>> Without this, we do not get a flush, and so blkdebug only sees a single
>> flush at the end of the job instead of two, and therefore does not
>> inject an error, which makes the block job complete instead of raising
>> an error.
>>
>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Hmm, relying on flush requests sequence doesn't seem reliable..
> Maybe, it's better to insert blockdev-add filter after JOB_READY? 
> blockdev-reopen should work now for "file" link changing.

Oh, yes, that would be nice, good idea!

But, on the other hand...  I tried, but unfortunately blockdev-reopen 
doesn’t seem to work that well with vmdk in particular:

$ ./qemu-img create -f vmdk /tmp/test.vmdk 64M
Formatting '/tmp/test.vmdk', fmt=vmdk size=67108864 compat6=off 
hwversion=undefined

$ ./qemu-system-x86_64 \
     -qmp stdio \
     -blockdev file,node-name=file,filename=/tmp/test.vmdk \
     -blockdev vmdk,node-name=vmdk,file=file \
     -blockdev raw,node-name=raw,file=file \
     <<EOF
{"execute": "qmp_capabilities"}
{"execute": "blockdev-reopen",
  "arguments": {
    "options": [{
      "node-name": "vmdk",
      "driver": "vmdk",
      "file": "raw"}]}}
{"execute": "quit"}
EOF
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 6}, 
"package": "v6.2.0-428-g572068bc87"}, "capabilities": ["oob"]}}
{"return": {}}
{"return": {}}
{"return": {}}
{"timestamp": {"seconds": 1641223501, "microseconds": 865524}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
[1]    21585 segmentation fault (core dumped)  ./qemu-system-x86_64 -qmp 
stdio -blockdev  -blockdev  -blockdev  <<<''


I don’t quite know what’s wrong, seems to me like some strong reference 
may be missing:

(gdb) bt
#0  0x00005643a784bb93 in bdrv_unset_inherits_from 
(root=root@entry=0x5643a898d990, child=child@entry=0x5643a89856c0, 
tran=tran@entry=0x0) at ../block.c:3172
#1  0x00005643a78524d0 in bdrv_unref_child (child=0x5643a89856c0, 
parent=0x5643a898d990) at ../block.c:3199
#2  bdrv_unref_child (parent=parent@entry=0x5643a898d990, 
child=0x5643a89856c0) at ../block.c:3193
#3  0x00005643a78b0c31 in vmdk_free_extents (bs=0x5643a898d990) at 
../block/vmdk.c:277
#4  0x00005643a78b0c5a in vmdk_close (bs=<optimized out>) at 
../block/vmdk.c:2810
#5  0x00005643a785260c in bdrv_close (bs=0x5643a898d990) at ../block.c:4790
#6  bdrv_delete (bs=<optimized out>) at ../block.c:5248
#7  bdrv_unref (bs=bs@entry=0x5643a898d990) at ../block.c:6675
#8  0x00005643a78436c5 in blockdev_close_all_bdrv_states () at 
../blockdev.c:675
#9  0x00005643a784f24f in bdrv_close_all () at ../block.c:4846
#10 0x00005643a77032de in qemu_cleanup () at ../softmmu/runstate.c:816
#11 0x00005643a7412823 in main (argc=<optimized out>, argv=<optimized 
out>, envp=<optimized out>) at ../softmmu/main.c:51
(gdb) p *child->bs
Cannot access memory at address 0x5646ccad4005


Valgrind points to vmdk_free_extents(), which makes me believe that the 
problem is that changing vmdk’s file child would need some cooperation 
from it to update its s->extents[...].file pointers:

==23055== Invalid read of size 8
==23055==    at 0x194CD0: bdrv_unset_inherits_from (block.c:3172)
==23055==    by 0x19B60F: bdrv_unref_child (block.c:3199)
==23055==    by 0x19B60F: bdrv_unref_child (block.c:3193)
==23055==    by 0x1F9D70: vmdk_free_extents (vmdk.c:277)
==23055==    by 0x1F9D99: vmdk_close (vmdk.c:2810)
==23055==    by 0x19B74B: bdrv_close (block.c:4790)
==23055==    by 0x19B74B: bdrv_delete (block.c:5248)
==23055==    by 0x19B74B: bdrv_unref (block.c:6675)
==23055==    by 0x18C804: blockdev_close_all_bdrv_states (blockdev.c:675)
==23055==    by 0x19838E: bdrv_close_all (block.c:4846)
==23055==    by 0x17F235: main (qemu-storage-daemon.c:354)
==23055==  Address 0x11259770 is 0 bytes inside a block of size 96 free'd
==23055==    at 0x48440E4: free (vg_replace_malloc.c:872)
==23055==    by 0x4AE0DAC: g_free (gmem.c:199)
==23055==    by 0x2ACE21: tran_commit (transactions.c:87)
==23055==    by 0x19D109: bdrv_reopen_multiple (block.c:4325)
==23055==    by 0x18FCB7: qmp_blockdev_reopen (blockdev.c:3599)
==23055==    by 0x272B0D: qmp_marshal_blockdev_reopen 
(qapi-commands-block-core.c:1086)
==23055==    by 0x295238: do_qmp_dispatch_bh (qmp-dispatch.c:129)
==23055==    by 0x2AD8A3: aio_bh_call (async.c:141)
==23055==    by 0x2AD8A3: aio_bh_poll (async.c:169)
==23055==    by 0x29E61D: aio_dispatch (aio-posix.c:381)
==23055==    by 0x2AD4ED: aio_ctx_dispatch (async.c:311)
==23055==    by 0x4ADF12F: UnknownInlinedFun (gmain.c:3381)
==23055==    by 0x4ADF12F: g_main_context_dispatch (gmain.c:4099)
==23055==    by 0x2B8E07: glib_pollfds_poll (main-loop.c:232)
==23055==    by 0x2B8E07: os_host_main_loop_wait (main-loop.c:255)
==23055==    by 0x2B8E07: main_loop_wait (main-loop.c:531)
==23055==  Block was alloc'd at
==23055==    at 0x484186F: malloc (vg_replace_malloc.c:381)
==23055==    by 0x4AE44A8: g_malloc (gmem.c:106)
==23055==    by 0x19A52F: bdrv_attach_child_common (block.c:2912)
==23055==    by 0x19A878: bdrv_attach_child_noperm (block.c:3002)
==23055==    by 0x19BC90: bdrv_attach_child (block.c:3095)
==23055==    by 0x1FC72E: vmdk_open (vmdk.c:1265)
==23055==    by 0x19B1D9: bdrv_open_driver (block.c:1566)
==23055==    by 0x19EAE3: bdrv_open_common (block.c:1858)
==23055==    by 0x19EAE3: bdrv_open_inherit (block.c:3865)
==23055==    by 0x19FB02: bdrv_open (block.c:3958)
==23055==    by 0x18FA9D: qmp_blockdev_add (blockdev.c:3540)
==23055==    by 0x17F1CA: process_options (qemu-storage-daemon.c:216)
==23055==    by 0x17F1CA: main (qemu-storage-daemon.c:338)


So we can’t use blockdev-reopen until this is fixed™, and because this 
kind of is a blocker for your series, I think I’ll just take the patch 
as-is then.

Hanna