Re: migration: broken snapshot saves appear on s390 when small fields in migration stream removed

[Claudio Fontana]

Hi Paolo,

On 7/12/20 6:11 PM, Paolo Bonzini wrote:
> On 12/07/20 12:00, Claudio Fontana wrote:
>> Note: only the === -blockdev with a backing file === part of test 267 fails. 
>> -blockdev with NBD is ok, like all the rest.
>>
>>
>> Interesting facts about s390 in particular: its save/load code includes the 
>> transfer of "storage keys",
>> which include a buffer of 32768 bytes of keydata in the stream.
>>
>> The code (hw/s390x/s390-skeys.c),
>> is modeled similarly to RAM transfer (like in migration/ram.c), with an EOS 
>> (end of stream) marker.
>>
>> Countrary to RAM transfer code though, after qemu_put_be64(f, EOS), the s390 
>> code does not qemu_fflush(f).
> 
> 1) Are there unexpected differences in the migration stream?  That is,
> you could modify qcow2.c to fopen/fwrite/fclose the bytes as they're
> written and read, and see if something does not match.

I hooked qcow2_co_pwritev_task and qcow2_co_preadv_task to also write to an 
external file using your suggestion.

I am seeing very interesting differences with and without the reproducer patch 
(ie, forcing icount_state_needed to 0 or not):

* without the reproducer patch, everything past the s390-skeys data field is in 
order: there is the EOS, and then the next idstr follows ("cpu_common").

* with the reproducer patch, every single byte past the s390-skeys data field 
is ZERO. There is no EOS, there is no next idstr "cpu_common", there is 
absolutely nothing else than zeroes until the end of the file.


> 
> 2) If it matches, are there unexpected differences other than the lack
> of icount section when you apply the reproducer patch?

they do not match at all.


> 
> The fflush part makes me put more hope in the first, but both could help
> you debug it.
> 
> Thanks,
> 
> Paolo
> 

Thanks,

Claudio