Re: [PATCH v4 4/4] iotests: Test node/bitmap aliases during migration

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5671 bytes --]

On 20.08.20 03:58, Eric Blake wrote:
> On 8/18/20 8:32 AM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/300     | 595 +++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/300.out |   5 +
> 
> Rather sparse output (I hate debugging those sorts of outputs when the
> test is failing).

Hm.  I don’t know, the stack trace usually gives a good idea and
./check -d gives QMP context.

The advantage of a sparse output is that we don’t need to adjust the
reference output every time some optional field is added somewhere.

>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 601 insertions(+)
>>   create mode 100755 tests/qemu-iotests/300
>>   create mode 100644 tests/qemu-iotests/300.out
>>
> 
>> +        # Dirty some random megabytes
>> +        for _ in range(9):
>> +            mb_ofs = random.randrange(1024)
>> +            self.vm_a.hmp_qemu_io(self.src_node_name, f'write
>> {mb_ofs}M 1M')
> 
> It turns out that the discard operation likewise dirties the bitmap, but
> slightly faster (see edb90bbd).  We could optimize it on top, but I'm
> not going to require a micro-optimizing to get it in.  The test takes
> about 12 seconds to run for me, but you didn't mark it as such in
> 'group', so that's good; but it turns up a problem:
> 
> 300      fail       [20:55:54] [20:56:06]                    output
> mismatch (see 300.out.bad)
> --- /home/eblake/qemu-tmp2/tests/qemu-iotests/300.out    2020-08-19
> 20:53:11.087791988 -0500
> +++ /home/eblake/qemu-tmp2/tests/qemu-iotests/300.out.bad    2020-08-19
> 20:56:06.092428756 -0500
> @@ -1,5 +1,41 @@
> -.....................................
> +WARNING:qemu.machine:qemu received signal 11; command:
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
> -display none -vga none -chardev
> socket,id=mon,path=/tmp/tmp.qT831UThme/qemu-b-798452-monitor.sock -mon
> chardev=mon,mode=control -qtest
> unix:path=/tmp/tmp.qT831UThme/qemu-b-798452-qtest.sock -accel qtest
> -nodefaults -display none -accel qtest -blockdev
> node-name=node0,driver=null-co -incoming unix:/tmp/tmp.qT831UThme/mig_sock"
> +.............FE.......................
> +======================================================================
> +ERROR: test_migratee_bitmap_is_not_mapped_on_dst
> (__main__.TestBlockBitmapMappingErrors)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/machine.py", line
> 435, in _do_shutdown
> +    self._soft_shutdown(timeout, has_quit)
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/machine.py", line
> 415, in _soft_shutdown
> +    self._qmp.cmd('quit')
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/qmp.py",
> line 266, in cmd
> +    return self.cmd_obj(qmp_cmd)
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/qmp.py",
> line 246, in cmd_obj
> +    self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
> +BrokenPipeError: [Errno 32] Broken pipe
> +
> +The above exception was the direct cause of the following exception:
> +
> +Traceback (most recent call last):
> +  File "300", line 76, in tearDown
> +    self.vm_b.shutdown()
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/machine.py", line
> 465, in shutdown
> +    self._do_shutdown(timeout, has_quit)
> +  File
> "/home/eblake/qemu-tmp2/tests/qemu-iotests/../../python/qemu/machine.py", line
> 438, in _do_shutdown
> +    raise AbnormalShutdown("Could not perform graceful shutdown") \
> +qemu.machine.AbnormalShutdown: Could not perform graceful shutdown
> +
> +======================================================================
> +FAIL: test_migratee_bitmap_is_not_mapped_on_dst
> (__main__.TestBlockBitmapMappingErrors)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "300", line 384, in test_migratee_bitmap_is_not_mapped_on_dst
> +    self.migrate(False)
> +  File "300", line 99, in migrate
> +    self.assertEqual(self.vm_a.wait_migration('postmigrate'),
> +AssertionError: False != True
> +
>  ----------------------------------------------------------------------
>  Ran 37 tests
> 
> -OK
> +FAILED (failures=1, errors=1)
> 
> I'm not sure why I'm seeing that, but it looks like you've got a bad
> deref somewhere in the alias code.

Ah, crap.

This should fix it:

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 89cb16b12c..d407dfefea 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -1091,7 +1091,9 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s,
             } else {
                 bitmap_name = s->bitmap_alias;
             }
+        }

+        if (!s->cancelled) {
             g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
             s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);


I had this originally, and then I decided to drop that hunk just before
sending v4 because I couldn’t see the point.  But we need it, because if
the bitmap alias is unknown, the migration is cancelled, so we need to
re-check s->cancalled after the alias lookup block.

Would you be OK with squashing that into patch 1?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]