[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 11/38] tests/qtest: migration-test: Add tests for file-based m
From: |
Juan Quintela |
Subject: |
Re: [PULL 11/38] tests/qtest: migration-test: Add tests for file-based migration |
Date: |
Tue, 17 Oct 2023 15:30:45 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.3 (gnu/linux) |
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.
- [PULL 14/38] migration: Create migrate_rdma(), (continued)
- [PULL 14/38] migration: Create migrate_rdma(), Juan Quintela, 2023/10/16
- [PULL 13/38] migration: Non multifd migration don't care about multifd flushes, Juan Quintela, 2023/10/16
- [PULL 15/38] migration/rdma: Unfold ram_control_before_iterate(), Juan Quintela, 2023/10/16
- [PULL 16/38] migration/rdma: Unfold ram_control_after_iterate(), Juan Quintela, 2023/10/16
- [PULL 11/38] tests/qtest: migration-test: Add tests for file-based migration, Juan Quintela, 2023/10/16
[PULL 18/38] migration/rdma: Unfold hook_ram_load(), Juan Quintela, 2023/10/16
[PULL 20/38] qemu-file: Remove QEMUFileHooks, Juan Quintela, 2023/10/16
[PULL 19/38] migration/rdma: Create rdma_control_save_page(), Juan Quintela, 2023/10/16
[PULL 17/38] migration/rdma: Remove all uses of RAM_CONTROL_HOOK, Juan Quintela, 2023/10/16
[PULL 23/38] migration/rdma: Check sooner if we are in postcopy for save_page(), Juan Quintela, 2023/10/16
[PULL 24/38] migration/rdma: Use i as for index instead of idx, Juan Quintela, 2023/10/16
[PULL 21/38] migration/rdma: Move rdma constants from qemu-file.h to rdma.h, Juan Quintela, 2023/10/16
[PULL 26/38] migration/rdma: Remove all "ret" variables that are used only once, Juan Quintela, 2023/10/16
[PULL 25/38] migration/rdma: Declare for index variables local, Juan Quintela, 2023/10/16
[PULL 22/38] migration/rdma: Remove qemu_ prefix from exported functions, Juan Quintela, 2023/10/16