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

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

2023-10-17 Thread Juan Quintela

Fabiano Rosas  wrote:
> Juan Quintela  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.

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

2023-10-17 Thread Fabiano Rosas

Juan Quintela  writes:

> Fabiano Rosas  wrote:
>> Juan Quintela  writes:
>>
>>> Fabiano Rosas  wrote:
>>> D> Juan Quintela  writes:

> From: Fabiano Rosas 
>
> 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 
> Reviewed-by: Juan Quintela 
> Signed-off-by: Fabiano Rosas 
> Signed-off-by: Juan Quintela 
> Message-ID: <20230712190742.22294-7-faro...@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 
 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.

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.

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

2023-10-17 Thread Juan Quintela

Fabiano Rosas  wrote:
> Juan Quintela  writes:
>
>> Fabiano Rosas  wrote:
>> D> Juan Quintela  writes:
>>>
 From: Fabiano Rosas 

 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 
 Reviewed-by: Juan Quintela 
 Signed-off-by: Fabiano Rosas 
 Signed-off-by: Juan Quintela 
 Message-ID: <20230712190742.22294-7-faro...@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 
>>> 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.

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

2023-10-17 Thread Fabiano Rosas

Juan Quintela  writes:

> Fabiano Rosas  wrote:
> D> Juan Quintela  writes:
>>
>>> From: Fabiano Rosas 
>>>
>>> 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 
>>> Reviewed-by: Juan Quintela 
>>> Signed-off-by: Fabiano Rosas 
>>> Signed-off-by: Juan Quintela 
>>> Message-ID: <20230712190742.22294-7-faro...@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 
>> 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?

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

2023-10-17 Thread Juan Quintela

From: Fabiano Rosas 

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 
Reviewed-by: Juan Quintela 
Signed-off-by: Fabiano Rosas 
Signed-off-by: Juan Quintela 
Message-ID: <20230712190742.22294-7-faro...@suse.de>
---
 tests/qtest/migration-test.c | 147 +++
 1 file changed, 147 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cef5081f8c..e1c110537b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -68,6 +68,10 @@ static bool got_dst_resume;
 
 #define ANALYZE_SCRIPT "scripts/analyze-migration.py"
 
+#define QEMU_VM_FILE_MAGIC 0x5145564d
+#define FILE_TEST_FILENAME "migfile"
+#define FILE_TEST_OFFSET 0x1000
+
 #if defined(__linux__)
 #include 
 #include 
@@ -884,6 +888,7 @@ static void test_migrate_end(QTestState *from, QTestState 
*to, bool test_dest)
 cleanup("migsocket");
 cleanup("src_serial");
 cleanup("dest_serial");
+cleanup(FILE_TEST_FILENAME);
 }
 
 #ifdef CONFIG_GNUTLS
@@ -1667,6 +1672,70 @@ finish:
 test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
 }
 
+static void test_file_common(MigrateCommon *args, bool stop_src)
+{
+QTestState *from, *to;
+void *data_hook = NULL;
+g_autofree char *connect_uri = g_strdup(args->connect_uri);
+
+if (test_migrate_start(, , args->listen_uri, >start)) {
+return;
+}
+
+/*
+ * File migration is never live. We can keep the source VM running
+ * during migration, but the destination will not be running
+ * concurrently.
+ */
+g_assert_false(args->live);
+
+if (args->start_hook) {
+data_hook = args->start_hook(from, to);
+}
+
+migrate_ensure_converge(from);
+wait_for_serial("src_serial");
+
+if (stop_src) {
+qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
+if (!got_src_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+}
+
+if (args->result == MIG_TEST_QMP_ERROR) {
+migrate_qmp_fail(from, connect_uri, "{}");
+goto finish;
+}
+
+migrate_qmp(from, connect_uri, "{}");
+wait_for_migration_complete(from);
+
+/*
+ * We need to wait for the source to finish before starting the
+ * destination.
+ */
+migrate_incoming_qmp(to, connect_uri, "{}");
+wait_for_migration_complete(to);
+
+if (stop_src) {
+qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
+}
+
+if (!got_dst_resume) {
+qtest_qmp_eventwait(to, "RESUME");
+}
+
+wait_for_serial("dest_serial");
+
+finish:
+if (args->finish_hook) {
+args->finish_hook(from, to, data_hook);
+}
+
+test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
+}
+
 static void test_precopy_unix_plain(void)
 {
 g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1862,6 +1931,76 @@ static void test_precopy_unix_compress_nowait(void)
 test_precopy_common();
 }
 
+static void test_precopy_file(void)
+{
+g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
+   FILE_TEST_FILENAME);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = "defer",
+};
+
+test_file_common(, true);
+}
+
+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_be64(*p) >> 32, ==, QEMU_VM_FILE_MAGIC);
+
+munmap(addr, size);
+close(fd);
+#endif
+}
+
+static void test_precopy_file_offset(void)
+{
+g_autofree char *uri = g_strdup_printf("file:%s/%s,offset=%d", tmpfs,
+   FILE_TEST_FILENAME,
+   FILE_TEST_OFFSET);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = "defer",
+.finish_hook = file_offset_finish_hook,
+};
+
+test_file_common(, false);
+}
+
+static void

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

2023-10-17 Thread Juan Quintela

Fabiano Rosas  wrote:
D> Juan Quintela  writes:
>
>> From: Fabiano Rosas 
>>
>> 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 
>> Reviewed-by: Juan Quintela 
>> Signed-off-by: Fabiano Rosas 
>> Signed-off-by: Juan Quintela 
>> Message-ID: <20230712190742.22294-7-faro...@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 
> 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.

Later, Juan.

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

2023-10-16 Thread Fabiano Rosas

Juan Quintela  writes:

> From: Fabiano Rosas 
>
> 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 
> Reviewed-by: Juan Quintela 
> Signed-off-by: Fabiano Rosas 
> Signed-off-by: Juan Quintela 
> Message-ID: <20230712190742.22294-7-faro...@suse.de>
> ---
>  tests/qtest/migration-test.c | 147 +++
>  1 file changed, 147 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index cef5081f8c..da02b6d692 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -68,6 +68,10 @@ static bool got_dst_resume;
>  
>  #define ANALYZE_SCRIPT "scripts/analyze-migration.py"
>  
> +#define QEMU_VM_FILE_MAGIC 0x5145564d
> +#define FILE_TEST_FILENAME "migfile"
> +#define FILE_TEST_OFFSET 0x1000
> +
>  #if defined(__linux__)
>  #include 
>  #include 
> @@ -884,6 +888,7 @@ static void test_migrate_end(QTestState *from, QTestState 
> *to, bool test_dest)
>  cleanup("migsocket");
>  cleanup("src_serial");
>  cleanup("dest_serial");
> +cleanup(FILE_TEST_FILENAME);
>  }
>  
>  #ifdef CONFIG_GNUTLS
> @@ -1667,6 +1672,70 @@ finish:
>  test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
>  }
>  
> +static void test_file_common(MigrateCommon *args, bool stop_src)
> +{
> +QTestState *from, *to;
> +void *data_hook = NULL;
> +g_autofree char *connect_uri = g_strdup(args->connect_uri);
> +
> +if (test_migrate_start(, , args->listen_uri, >start)) {
> +return;
> +}
> +
> +/*
> + * File migration is never live. We can keep the source VM running
> + * during migration, but the destination will not be running
> + * concurrently.
> + */
> +g_assert_false(args->live);
> +
> +if (args->start_hook) {
> +data_hook = args->start_hook(from, to);
> +}
> +
> +migrate_ensure_converge(from);
> +wait_for_serial("src_serial");
> +
> +if (stop_src) {
> +qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
> +if (!got_src_stop) {
> +qtest_qmp_eventwait(from, "STOP");
> +}
> +}
> +
> +if (args->result == MIG_TEST_QMP_ERROR) {
> +migrate_qmp_fail(from, connect_uri, "{}");
> +goto finish;
> +}
> +
> +migrate_qmp(from, connect_uri, "{}");
> +wait_for_migration_complete(from);
> +
> +/*
> + * We need to wait for the source to finish before starting the
> + * destination.
> + */
> +migrate_incoming_qmp(to, connect_uri, "{}");
> +wait_for_migration_complete(to);
> +
> +if (stop_src) {
> +qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
> +}
> +
> +if (!got_dst_resume) {
> +qtest_qmp_eventwait(to, "RESUME");
> +}
> +
> +wait_for_serial("dest_serial");
> +
> +finish:
> +if (args->finish_hook) {
> +args->finish_hook(from, to, data_hook);
> +}
> +
> +test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
> +}
> +
>  static void test_precopy_unix_plain(void)
>  {
>  g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1862,6 +1931,76 @@ static void test_precopy_unix_compress_nowait(void)
>  test_precopy_common();
>  }
>  
> +static void test_precopy_file(void)
> +{
> +g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
> +   FILE_TEST_FILENAME);
> +MigrateCommon args = {
> +.connect_uri = uri,
> +.listen_uri = "defer",
> +};
> +
> +test_file_common(, true);
> +}
> +
> +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 
Date: Mon, 16 Oct

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

2023-10-16 Thread Juan Quintela

From: Fabiano Rosas 

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 
Reviewed-by: Juan Quintela 
Signed-off-by: Fabiano Rosas 
Signed-off-by: Juan Quintela 
Message-ID: <20230712190742.22294-7-faro...@suse.de>
---
 tests/qtest/migration-test.c | 147 +++
 1 file changed, 147 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cef5081f8c..da02b6d692 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -68,6 +68,10 @@ static bool got_dst_resume;
 
 #define ANALYZE_SCRIPT "scripts/analyze-migration.py"
 
+#define QEMU_VM_FILE_MAGIC 0x5145564d
+#define FILE_TEST_FILENAME "migfile"
+#define FILE_TEST_OFFSET 0x1000
+
 #if defined(__linux__)
 #include 
 #include 
@@ -884,6 +888,7 @@ static void test_migrate_end(QTestState *from, QTestState 
*to, bool test_dest)
 cleanup("migsocket");
 cleanup("src_serial");
 cleanup("dest_serial");
+cleanup(FILE_TEST_FILENAME);
 }
 
 #ifdef CONFIG_GNUTLS
@@ -1667,6 +1672,70 @@ finish:
 test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
 }
 
+static void test_file_common(MigrateCommon *args, bool stop_src)
+{
+QTestState *from, *to;
+void *data_hook = NULL;
+g_autofree char *connect_uri = g_strdup(args->connect_uri);
+
+if (test_migrate_start(, , args->listen_uri, >start)) {
+return;
+}
+
+/*
+ * File migration is never live. We can keep the source VM running
+ * during migration, but the destination will not be running
+ * concurrently.
+ */
+g_assert_false(args->live);
+
+if (args->start_hook) {
+data_hook = args->start_hook(from, to);
+}
+
+migrate_ensure_converge(from);
+wait_for_serial("src_serial");
+
+if (stop_src) {
+qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
+if (!got_src_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+}
+
+if (args->result == MIG_TEST_QMP_ERROR) {
+migrate_qmp_fail(from, connect_uri, "{}");
+goto finish;
+}
+
+migrate_qmp(from, connect_uri, "{}");
+wait_for_migration_complete(from);
+
+/*
+ * We need to wait for the source to finish before starting the
+ * destination.
+ */
+migrate_incoming_qmp(to, connect_uri, "{}");
+wait_for_migration_complete(to);
+
+if (stop_src) {
+qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
+}
+
+if (!got_dst_resume) {
+qtest_qmp_eventwait(to, "RESUME");
+}
+
+wait_for_serial("dest_serial");
+
+finish:
+if (args->finish_hook) {
+args->finish_hook(from, to, data_hook);
+}
+
+test_migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
+}
+
 static void test_precopy_unix_plain(void)
 {
 g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1862,6 +1931,76 @@ static void test_precopy_unix_compress_nowait(void)
 test_precopy_common();
 }
 
+static void test_precopy_file(void)
+{
+g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
+   FILE_TEST_FILENAME);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = "defer",
+};
+
+test_file_common(, true);
+}
+
+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);
+
+munmap(addr, size);
+close(fd);
+#endif
+}
+
+static void test_precopy_file_offset(void)
+{
+g_autofree char *uri = g_strdup_printf("file:%s/%s,offset=%d", tmpfs,
+   FILE_TEST_FILENAME,
+   FILE_TEST_OFFSET);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = "defer",
+.finish_hook = file_offset_finish_hook,
+};
+
+test_file_common(, false);
+}
+
+static void

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

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

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

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

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

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

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

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

8 matches

Site Navigation

Mail list logo

Footer information