diff mbox series

[V2,6/6] tests/qtest: migration: add reboot mode test

Message ID 1698263069-406971-7-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series Live Update reboot mode | expand

Commit Message

Steven Sistare Oct. 25, 2023, 7:44 p.m. UTC
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Juan Quintela Oct. 31, 2023, 1:19 p.m. UTC | #1
Steve Sistare <steven.sistare@oracle.com> wrote:
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
Fabiano Rosas Nov. 1, 2023, 1:34 p.m. UTC | #2
Steve Sistare <steven.sistare@oracle.com> writes:

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index e1c1105..de29fc5 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2001,6 +2001,31 @@ static void test_precopy_file_offset_bad(void)
>      test_file_common(&args, false);
>  }
>  
> +static void *test_mode_reboot_start(QTestState *from, QTestState *to)
> +{
> +    migrate_set_parameter_str(from, "mode", "cpr-reboot");
> +    migrate_set_parameter_str(to, "mode", "cpr-reboot");
> +
> +    migrate_set_capability(from, "x-ignore-shared", true);
> +    migrate_set_capability(to, "x-ignore-shared", true);
> +
> +    return NULL;
> +}
> +
> +static void test_mode_reboot(void)
> +{
> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
> +                                           FILE_TEST_FILENAME);
> +    MigrateCommon args = {
> +        .start.use_shmem = true,
> +        .connect_uri = uri,
> +        .listen_uri = "defer",
> +        .start_hook = test_mode_reboot_start
> +    };
> +
> +    test_file_common(&args, true);
> +}
> +
>  static void test_precopy_tcp_plain(void)
>  {
>      MigrateCommon args = {
> @@ -3056,6 +3081,8 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/file/offset/bad",
>                     test_precopy_file_offset_bad);
>  
> +    qtest_add_func("/migration/mode/reboot", test_mode_reboot);
> +
>  #ifdef CONFIG_GNUTLS
>      qtest_add_func("/migration/precopy/unix/tls/psk",
>                     test_precopy_unix_tls_psk);

We have an issue with this test on CI:

$ df -h /dev/shm
Filesystem      Size  Used Avail Use% Mounted on
shm              64M     0   64M   0% /dev/shm

These are shared CI runners, so AFAICT there's no way to increase the
shared memory size.

Reducing the memory for this single test also wouldn't work because we
can run migration-test for different archs in parallel + there's the
ivshmem_test which uses 4M.

Maybe just leave it out of CI? Laptops will probably have enough shared
memory to not hit this. If we add a warning comment to the test, might
be enough.
Steven Sistare Nov. 1, 2023, 1:57 p.m. UTC | #3
On 11/1/2023 9:34 AM, Fabiano Rosas wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index e1c1105..de29fc5 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -2001,6 +2001,31 @@ static void test_precopy_file_offset_bad(void)
>>      test_file_common(&args, false);
>>  }
>>  
>> +static void *test_mode_reboot_start(QTestState *from, QTestState *to)
>> +{
>> +    migrate_set_parameter_str(from, "mode", "cpr-reboot");
>> +    migrate_set_parameter_str(to, "mode", "cpr-reboot");
>> +
>> +    migrate_set_capability(from, "x-ignore-shared", true);
>> +    migrate_set_capability(to, "x-ignore-shared", true);
>> +
>> +    return NULL;
>> +}
>> +
>> +static void test_mode_reboot(void)
>> +{
>> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
>> +                                           FILE_TEST_FILENAME);
>> +    MigrateCommon args = {
>> +        .start.use_shmem = true,
>> +        .connect_uri = uri,
>> +        .listen_uri = "defer",
>> +        .start_hook = test_mode_reboot_start
>> +    };
>> +
>> +    test_file_common(&args, true);
>> +}
>> +
>>  static void test_precopy_tcp_plain(void)
>>  {
>>      MigrateCommon args = {
>> @@ -3056,6 +3081,8 @@ int main(int argc, char **argv)
>>      qtest_add_func("/migration/precopy/file/offset/bad",
>>                     test_precopy_file_offset_bad);
>>  
>> +    qtest_add_func("/migration/mode/reboot", test_mode_reboot);
>> +
>>  #ifdef CONFIG_GNUTLS
>>      qtest_add_func("/migration/precopy/unix/tls/psk",
>>                     test_precopy_unix_tls_psk);
> 
> We have an issue with this test on CI:
> 
> $ df -h /dev/shm
> Filesystem      Size  Used Avail Use% Mounted on
> shm              64M     0   64M   0% /dev/shm
> 
> These are shared CI runners, so AFAICT there's no way to increase the
> shared memory size.
> 
> Reducing the memory for this single test also wouldn't work because we
> can run migration-test for different archs in parallel + there's the
> ivshmem_test which uses 4M.
> 
> Maybe just leave it out of CI? Laptops will probably have enough shared
> memory to not hit this. If we add a warning comment to the test, might
> be enough.

in test_migrate_start, I could set memory_size very small if use_shmem, and adjust 
start_address and end_address. Can you suggest a safe size?

- Steve
Steven Sistare Nov. 1, 2023, 4:26 p.m. UTC | #4
On 11/1/2023 9:57 AM, Steven Sistare wrote:
> On 11/1/2023 9:34 AM, Fabiano Rosas wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>  tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>>
>>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>>> index e1c1105..de29fc5 100644
>>> --- a/tests/qtest/migration-test.c
>>> +++ b/tests/qtest/migration-test.c
>>> @@ -2001,6 +2001,31 @@ static void test_precopy_file_offset_bad(void)
>>>      test_file_common(&args, false);
>>>  }
>>>  
>>> +static void *test_mode_reboot_start(QTestState *from, QTestState *to)
>>> +{
>>> +    migrate_set_parameter_str(from, "mode", "cpr-reboot");
>>> +    migrate_set_parameter_str(to, "mode", "cpr-reboot");
>>> +
>>> +    migrate_set_capability(from, "x-ignore-shared", true);
>>> +    migrate_set_capability(to, "x-ignore-shared", true);
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +static void test_mode_reboot(void)
>>> +{
>>> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
>>> +                                           FILE_TEST_FILENAME);
>>> +    MigrateCommon args = {
>>> +        .start.use_shmem = true,
>>> +        .connect_uri = uri,
>>> +        .listen_uri = "defer",
>>> +        .start_hook = test_mode_reboot_start
>>> +    };
>>> +
>>> +    test_file_common(&args, true);
>>> +}
>>> +
>>>  static void test_precopy_tcp_plain(void)
>>>  {
>>>      MigrateCommon args = {
>>> @@ -3056,6 +3081,8 @@ int main(int argc, char **argv)
>>>      qtest_add_func("/migration/precopy/file/offset/bad",
>>>                     test_precopy_file_offset_bad);
>>>  
>>> +    qtest_add_func("/migration/mode/reboot", test_mode_reboot);
>>> +
>>>  #ifdef CONFIG_GNUTLS
>>>      qtest_add_func("/migration/precopy/unix/tls/psk",
>>>                     test_precopy_unix_tls_psk);
>>
>> We have an issue with this test on CI:
>>
>> $ df -h /dev/shm
>> Filesystem      Size  Used Avail Use% Mounted on
>> shm              64M     0   64M   0% /dev/shm
>>
>> These are shared CI runners, so AFAICT there's no way to increase the
>> shared memory size.
>>
>> Reducing the memory for this single test also wouldn't work because we
>> can run migration-test for different archs in parallel + there's the
>> ivshmem_test which uses 4M.
>>
>> Maybe just leave it out of CI? Laptops will probably have enough shared
>> memory to not hit this. If we add a warning comment to the test, might
>> be enough.
> 
> in test_migrate_start, I could set memory_size very small if use_shmem, and adjust 
> start_address and end_address. Can you suggest a safe size?

Ugh, I would also need to dynamically change TEST_MEM_END and ARM_TEST_MEM_END in
a-b-bootblock.S and a-b-kernel.S, like I do for the suspend_me variable in my
work-in-progress patch "tests/qtest: option to suspend during migration".

- Steve
Fabiano Rosas Nov. 1, 2023, 4:52 p.m. UTC | #5
Steven Sistare <steven.sistare@oracle.com> writes:

> On 11/1/2023 9:57 AM, Steven Sistare wrote:
>> On 11/1/2023 9:34 AM, Fabiano Rosas wrote:
>>> Steve Sistare <steven.sistare@oracle.com> writes:
>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>> ---
>>>>  tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
>>>>  1 file changed, 27 insertions(+)
>>>>
>>>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>>>> index e1c1105..de29fc5 100644
>>>> --- a/tests/qtest/migration-test.c
>>>> +++ b/tests/qtest/migration-test.c
>>>> @@ -2001,6 +2001,31 @@ static void test_precopy_file_offset_bad(void)
>>>>      test_file_common(&args, false);
>>>>  }
>>>>  
>>>> +static void *test_mode_reboot_start(QTestState *from, QTestState *to)
>>>> +{
>>>> +    migrate_set_parameter_str(from, "mode", "cpr-reboot");
>>>> +    migrate_set_parameter_str(to, "mode", "cpr-reboot");
>>>> +
>>>> +    migrate_set_capability(from, "x-ignore-shared", true);
>>>> +    migrate_set_capability(to, "x-ignore-shared", true);
>>>> +
>>>> +    return NULL;
>>>> +}
>>>> +
>>>> +static void test_mode_reboot(void)
>>>> +{
>>>> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
>>>> +                                           FILE_TEST_FILENAME);
>>>> +    MigrateCommon args = {
>>>> +        .start.use_shmem = true,
>>>> +        .connect_uri = uri,
>>>> +        .listen_uri = "defer",
>>>> +        .start_hook = test_mode_reboot_start
>>>> +    };
>>>> +
>>>> +    test_file_common(&args, true);
>>>> +}
>>>> +
>>>>  static void test_precopy_tcp_plain(void)
>>>>  {
>>>>      MigrateCommon args = {
>>>> @@ -3056,6 +3081,8 @@ int main(int argc, char **argv)
>>>>      qtest_add_func("/migration/precopy/file/offset/bad",
>>>>                     test_precopy_file_offset_bad);
>>>>  
>>>> +    qtest_add_func("/migration/mode/reboot", test_mode_reboot);
>>>> +
>>>>  #ifdef CONFIG_GNUTLS
>>>>      qtest_add_func("/migration/precopy/unix/tls/psk",
>>>>                     test_precopy_unix_tls_psk);
>>>
>>> We have an issue with this test on CI:
>>>
>>> $ df -h /dev/shm
>>> Filesystem      Size  Used Avail Use% Mounted on
>>> shm              64M     0   64M   0% /dev/shm
>>>
>>> These are shared CI runners, so AFAICT there's no way to increase the
>>> shared memory size.
>>>
>>> Reducing the memory for this single test also wouldn't work because we
>>> can run migration-test for different archs in parallel + there's the
>>> ivshmem_test which uses 4M.
>>>
>>> Maybe just leave it out of CI? Laptops will probably have enough shared
>>> memory to not hit this. If we add a warning comment to the test, might
>>> be enough.
>> 
>> in test_migrate_start, I could set memory_size very small if use_shmem, and adjust 
>> start_address and end_address. Can you suggest a safe size?

We need at least ~4M for the MAGIC_MARKER logic to work. Then each
architecture will have a minimum memory size it needs to even be able to
boot. Then we'll need ~2M (to be safe) for the a-b code itself.

One issue I can see already is that ppc needs at least 256M to boot. We
currently only test ppc with KVM, but we need to make sure the custom
runners (not even sure we have ppc baremetal) would have enough shared
memory. Another issue is that there's not much stopping us from testing
ppc with TCG as well. In fact, I'm highly in favor of bringing back
s390x and ppc64le with TCG for migration-test. It's just a pain to test
those otherwise.

x86 seems like it could go as low as ~6M.

aarch64 only needs 512k for the kernel, but I'm not having success with
less than 110M. Probably some firmware + device tree taking up space,
I'm not sure.

>
> Ugh, I would also need to dynamically change TEST_MEM_END and ARM_TEST_MEM_END in
> a-b-bootblock.S and a-b-kernel.S, like I do for the suspend_me variable in my
> work-in-progress patch "tests/qtest: option to suspend during migration".

I'm leaning towards just checking the GITLAB_CI environment variable and
skipping the test on CI.

Juan, any preference?
diff mbox series

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index e1c1105..de29fc5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2001,6 +2001,31 @@  static void test_precopy_file_offset_bad(void)
     test_file_common(&args, false);
 }
 
+static void *test_mode_reboot_start(QTestState *from, QTestState *to)
+{
+    migrate_set_parameter_str(from, "mode", "cpr-reboot");
+    migrate_set_parameter_str(to, "mode", "cpr-reboot");
+
+    migrate_set_capability(from, "x-ignore-shared", true);
+    migrate_set_capability(to, "x-ignore-shared", true);
+
+    return NULL;
+}
+
+static void test_mode_reboot(void)
+{
+    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
+                                           FILE_TEST_FILENAME);
+    MigrateCommon args = {
+        .start.use_shmem = true,
+        .connect_uri = uri,
+        .listen_uri = "defer",
+        .start_hook = test_mode_reboot_start
+    };
+
+    test_file_common(&args, true);
+}
+
 static void test_precopy_tcp_plain(void)
 {
     MigrateCommon args = {
@@ -3056,6 +3081,8 @@  int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/file/offset/bad",
                    test_precopy_file_offset_bad);
 
+    qtest_add_func("/migration/mode/reboot", test_mode_reboot);
+
 #ifdef CONFIG_GNUTLS
     qtest_add_func("/migration/precopy/unix/tls/psk",
                    test_precopy_unix_tls_psk);