[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 14:55:34 +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:
>
>> Fabiano Rosas <farosas@suse.de> wrote:
>> D> Juan Quintela <quintela@redhat.com> writes:
>>>
>>>> From: Fabiano Rosas <farosas@suse.de>
>>>>
>>>> Add basic tests for file-based migration.
>>>>
>>>> Note that we cannot use test_precopy_common because that routine
>>>> expects it to be possible to run the migration live. With the file
>>>> transport there is no live migration because we must wait for the
>>>> source to finish writing the migration data to the file before the
>>>> destination can start reading. Add a new migration function
>>>> specifically to handle the file migration.
>>>>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>> Message-ID: <20230712190742.22294-7-farosas@suse.de>
>>
>>>> +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);
>>>> + uintptr_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 < addr + FILE_TEST_OFFSET / sizeof(uintptr_t)) {
>>>> + g_assert(*p == 0);
>>>> + p++;
>>>> + }
>>>> + g_assert_cmpint(cpu_to_be32(*p), ==, QEMU_VM_FILE_MAGIC);
>>>
>>> This truncates to 32-bits, so it breaks on a BE host. We need this:
>>>
>>> -->8--
>>> From ea0c2d1c988add48d9754891a9fc7f6854a9718a Mon Sep 17 00:00:00 2001
>>> From: Fabiano Rosas <farosas@suse.de>
>>> Date: Mon, 16 Oct 2023 15:21:49 -0300
>>> Subject: [PATCH] fixup! tests/qtest: migration-test: Add tests for
>>> file-based
>>> migration
>>>
>>> ---
>>> tests/qtest/migration-test.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>>> index da02b6d692..e1c110537b 100644
>>> --- a/tests/qtest/migration-test.c
>>> +++ b/tests/qtest/migration-test.c
>>> @@ -1966,7 +1966,7 @@ static void file_offset_finish_hook(QTestState *from,
>>> QTestState *to,
>>> g_assert(*p == 0);
>>> p++;
>>> }
>>> - g_assert_cmpint(cpu_to_be32(*p), ==, QEMU_VM_FILE_MAGIC);
>>> + g_assert_cmpint(cpu_to_be64(*p) >> 32, ==, QEMU_VM_FILE_MAGIC);
>>>
>>> munmap(addr, size);
>>> close(fd);
>>
>> 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.
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?
And yes, for migration, in case of doubt, we use 64bits. I know it is
unfair for 32 bits host architectures, but they basically don't exist
anymore.
Later, Juan.
- [PULL 08/38] migration: Fix analyze-migration.py when ignore-shared is used, (continued)
- [PULL 08/38] migration: Fix analyze-migration.py when ignore-shared is used, Juan Quintela, 2023/10/16
- [PULL 10/38] tests/qtest/migration: Add a test for the analyze-migration script, Juan Quintela, 2023/10/16
- [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