Migration test refactoring

[PATCH 02/42] migration-test: Make ignore_stderr regular with other options

Posted by Juan Quintela 2 years, 8 months ago

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/qtest/migration-test.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index c5e0c69c6b..73b2f01427 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -602,7 +602,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     g_autofree gchar *arch_target = NULL;
     g_autofree gchar *cmd_source = NULL;
     g_autofree gchar *cmd_target = NULL;
-    const gchar *ignore_stderr;
+    const gchar *ignore_stderr = NULL;
     g_autofree char *bootpath = NULL;
     g_autofree char *shmem_opts = NULL;
     g_autofree char *shmem_path = NULL;
@@ -672,10 +672,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
          * IO redirection does not work, so don't bother adding IO redirection
          * to the command line.
          */
-        ignore_stderr = "";
 #endif
-    } else {
-        ignore_stderr = "";
     }
 
     if (args->use_shmem) {
@@ -701,7 +698,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  memory_size, tmpfs,
                                  arch_source, shmem_opts,
                                  args->opts_source ? args->opts_source : "",
-                                 ignore_stderr);
+                                 ignore_stderr ? ignore_stderr : "");
+
     if (!args->only_target) {
         *from = qtest_init(cmd_source);
         qtest_qmp_set_event_callback(*from,
@@ -722,7 +720,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  memory_size, tmpfs, uri,
                                  arch_target, shmem_opts,
                                  args->opts_target ? args->opts_target : "",
-                                 ignore_stderr);
+                                 ignore_stderr ? ignore_stderr : "");
     *to = qtest_init(cmd_target);
     qtest_qmp_set_event_callback(*to,
                                  migrate_watch_for_resume,
-- 
2.40.1

Re: [PATCH 02/42] migration-test: Make ignore_stderr regular with other options

Posted by Peter Xu 2 years, 7 months ago

On Fri, Jun 09, 2023 at 12:49:03AM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/qtest/migration-test.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index c5e0c69c6b..73b2f01427 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -602,7 +602,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>      g_autofree gchar *arch_target = NULL;
>      g_autofree gchar *cmd_source = NULL;
>      g_autofree gchar *cmd_target = NULL;
> -    const gchar *ignore_stderr;
> +    const gchar *ignore_stderr = NULL;
>      g_autofree char *bootpath = NULL;
>      g_autofree char *shmem_opts = NULL;
>      g_autofree char *shmem_path = NULL;
> @@ -672,10 +672,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>           * IO redirection does not work, so don't bother adding IO redirection
>           * to the command line.
>           */
> -        ignore_stderr = "";

Personally maybe I won't bother with such a patch as a whole.. but if we do
want to, we could also remove "#else" here and move comment above it.

>  #endif
> -    } else {
> -        ignore_stderr = "";
>      }
>  
>      if (args->use_shmem) {
> @@ -701,7 +698,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   memory_size, tmpfs,
>                                   arch_source, shmem_opts,
>                                   args->opts_source ? args->opts_source : "",
> -                                 ignore_stderr);
> +                                 ignore_stderr ? ignore_stderr : "");
> +
>      if (!args->only_target) {
>          *from = qtest_init(cmd_source);
>          qtest_qmp_set_event_callback(*from,
> @@ -722,7 +720,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   memory_size, tmpfs, uri,
>                                   arch_target, shmem_opts,
>                                   args->opts_target ? args->opts_target : "",
> -                                 ignore_stderr);
> +                                 ignore_stderr ? ignore_stderr : "");
>      *to = qtest_init(cmd_target);
>      qtest_qmp_set_event_callback(*to,
>                                   migrate_watch_for_resume,
> -- 
> 2.40.1
> 

-- 
Peter Xu

Re: [PATCH 02/42] migration-test: Make ignore_stderr regular with other options

Posted by Juan Quintela 2 years, 7 months ago

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jun 09, 2023 at 12:49:03AM +0200, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/qtest/migration-test.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>> 
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index c5e0c69c6b..73b2f01427 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -602,7 +602,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>      g_autofree gchar *arch_target = NULL;
>>      g_autofree gchar *cmd_source = NULL;
>>      g_autofree gchar *cmd_target = NULL;
>> -    const gchar *ignore_stderr;
>> +    const gchar *ignore_stderr = NULL;
>>      g_autofree char *bootpath = NULL;
>>      g_autofree char *shmem_opts = NULL;
>>      g_autofree char *shmem_path = NULL;
>> @@ -672,10 +672,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>           * IO redirection does not work, so don't bother adding IO redirection
>>           * to the command line.
>>           */
>> -        ignore_stderr = "";
>
> Personally maybe I won't bother with such a patch as a whole..

I am trying that all the options are handled the same.
The whole idea here is that I have to split guest definition and guest
creation.  See GuestState later on the series.

If all the options are handled the same, it is much easier than if I
have to do "magic" for one vs others.

One thing that I wonder is that migration-test can't be the only test
that needs to redirect stderr, but that is a completely different issue.

> but if we do
> want to, we could also remove "#else" here and move comment above it.

Done.