Re: [PULL 11/38] tests/qtest: migration-test: Add tests for file-based migration

[Juan Quintela <quintela@redhat.com>]

Fabiano Rosas <farosas@suse.de> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>

[..]

>>>> I am resubmitting with this change.
>>>>
>>>> But I think we need to change this:
>>>>
>>>>>> +    g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME);
>>>>>> +    size_t size = FILE_TEST_OFFSET + sizeof(QEMU_VM_FILE_MAGIC);
>>>>>> +    uintptr_t *addr, *p;
>>>>
>>>> I think we should change the test so the file is 64 bits on every
>>>> architecture.
>>>> Then we can cast to void * or uintptr_t as needed.
>>>
>>> Hm, I don't get what you mean here. What needs to be 64 bits?
>>
>> size_t is 32 bits on 32bits host, and 64 bits on 64 bits host.
>> uintprt_t is the same.
>
> Right, I have thought of that when writing this fix yesterday, but I
> dismissed it because I thought we were never have a 32bit host running
> these tests.
>
>> So using explicit sizes:
>>
>> static void file_offset_finish_hook(QTestState *from, QTestState *to,
>>                                     void *opaque)
>> {
>> #if defined(__linux__)
>>     g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME);
>>     size_t size = FILE_TEST_OFFSET + sizeof(QEMU_VM_FILE_MAGIC);
>>     uint64_t *addr, *p;
>>     int fd;
>>
>>     fd = open(path, O_RDONLY);
>>     g_assert(fd != -1);
>>     addr = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
>>     g_assert(addr != MAP_FAILED);
>>
>>     /*
>>      * Ensure the skipped offset contains zeros and the migration
>>      * stream starts at the right place.
>>      */
>>     p = addr;
>>     while (p < (uintprt_t)addr + FILE_TEST_OFFSET) {
>>         g_assert(*p == 0);
>>         p++;
>>     }
>>     g_assert_cmpint(cpu_to_be64(*p) >> 32, ==, QEMU_VM_FILE_MAGIC);
>>
>>     munmap(addr, size);
>>     close(fd);
>> #endif
>> }
>>
>> This is completely untested, but it should make sure that we are reading
>> 64bits integers in both 32 and 64 bits hosts, no?
>
> Looks like it. I can give it a try and send an update as a separate
> patch.

Send the update against migration-20231017 please.
That branch is on github and the patches are on the PULL request on the
list.

Thanks, Juan.