Fabiano Rosas <[email protected]> wrote:
> Juan Quintela <[email protected]> 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.
