1. Patchew
  2. QEMU
  3. [PATCH 00/42] Migration test refactoring
  4. View patch

[PATCH 03/42] migration-test: simplify shmem_opts handling

Juan Quintela posted 42 patches 2 years, 8 months ago
[PATCH 03/42] migration-test: simplify shmem_opts handling
Posted by Juan Quintela 2 years, 8 months ago 
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/qtest/migration-test.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 73b2f01427..95ccc9bce7 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -681,9 +681,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
             "-object memory-backend-file,id=mem0,size=%s"
             ",mem-path=%s,share=on -numa node,memdev=mem0",
             memory_size, shmem_path);
-    } else {
-        shmem_path = NULL;
-        shmem_opts = g_strdup("");
     }
 
     cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
@@ -696,7 +693,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  machine_opts ? " -machine " : "",
                                  machine_opts ? machine_opts : "",
                                  memory_size, tmpfs,
-                                 arch_source, shmem_opts,
+                                 arch_source,
+                                 shmem_opts ? shmem_opts : "",
                                  args->opts_source ? args->opts_source : "",
                                  ignore_stderr ? ignore_stderr : "");
 
@@ -718,7 +716,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  machine_opts ? " -machine " : "",
                                  machine_opts ? machine_opts : "",
                                  memory_size, tmpfs, uri,
-                                 arch_target, shmem_opts,
+                                 arch_target,
+                                 shmem_opts ? shmem_opts : "",
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr ? ignore_stderr : "");
     *to = qtest_init(cmd_target);
-- 
2.40.1
Re: [PATCH 03/42] migration-test: simplify shmem_opts handling
Posted by Peter Xu 2 years, 7 months ago 
On Fri, Jun 09, 2023 at 12:49:04AM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/qtest/migration-test.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 73b2f01427..95ccc9bce7 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -681,9 +681,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>              "-object memory-backend-file,id=mem0,size=%s"
>              ",mem-path=%s,share=on -numa node,memdev=mem0",
>              memory_size, shmem_path);
> -    } else {
> -        shmem_path = NULL;
> -        shmem_opts = g_strdup("");
>      }
>  
>      cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> @@ -696,7 +693,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   machine_opts ? " -machine " : "",
>                                   machine_opts ? machine_opts : "",
>                                   memory_size, tmpfs,
> -                                 arch_source, shmem_opts,
> +                                 arch_source,
> +                                 shmem_opts ? shmem_opts : "",
>                                   args->opts_source ? args->opts_source : "",
>                                   ignore_stderr ? ignore_stderr : "");
>  
> @@ -718,7 +716,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                   machine_opts ? " -machine " : "",
>                                   machine_opts ? machine_opts : "",
>                                   memory_size, tmpfs, uri,
> -                                 arch_target, shmem_opts,
> +                                 arch_target,
> +                                 shmem_opts ? shmem_opts : "",

Isn't this adding duplications instead?

Meanwhile, shmem_opts right now is auto-free.  If we do this we don't need
it to be auto-free anymore..

>                                   args->opts_target ? args->opts_target : "",
>                                   ignore_stderr ? ignore_stderr : "");
>      *to = qtest_init(cmd_target);
> -- 
> 2.40.1
> 

-- 
Peter Xu
Re: [PATCH 03/42] migration-test: simplify shmem_opts handling
Posted by Juan Quintela 2 years, 7 months ago 
Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jun 09, 2023 at 12:49:04AM +0200, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/qtest/migration-test.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> 
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 73b2f01427..95ccc9bce7 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -681,9 +681,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>              "-object memory-backend-file,id=mem0,size=%s"
>>              ",mem-path=%s,share=on -numa node,memdev=mem0",
>>              memory_size, shmem_path);
>> -    } else {
>> -        shmem_path = NULL;
>> -        shmem_opts = g_strdup("");
>>      }
>>  
>>      cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>> @@ -696,7 +693,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>                                   machine_opts ? " -machine " : "",
>>                                   machine_opts ? machine_opts : "",
>>                                   memory_size, tmpfs,
>> -                                 arch_source, shmem_opts,
>> +                                 arch_source,
>> +                                 shmem_opts ? shmem_opts : "",
>>                                   args->opts_source ? args->opts_source : "",
>>                                   ignore_stderr ? ignore_stderr : "");
>>  
>> @@ -718,7 +716,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>>                                   machine_opts ? " -machine " : "",
>>                                   machine_opts ? machine_opts : "",
>>                                   memory_size, tmpfs, uri,
>> -                                 arch_target, shmem_opts,
>> +                                 arch_target,
>> +                                 shmem_opts ? shmem_opts : "",
>
> Isn't this adding duplications instead?

I don't follow.

> Meanwhile, shmem_opts right now is auto-free.  If we do this we don't need
> it to be auto-free anymore..

We need.
It can still be from g_strdup_printf().

What this patch change is that it will never be (again) "".

It is going to be NULL or a real string, like all the other options.
The real string is generated, so it needs to be auto_free.

Later, Juan.

>>                                   args->opts_target ? args->opts_target : "",
>>                                   ignore_stderr ? ignore_stderr : "");
>>      *to = qtest_init(cmd_target);
>> -- 
>> 2.40.1
>>
Re: [PATCH 03/42] migration-test: simplify shmem_opts handling
Posted by Peter Xu 2 years, 7 months ago 
On Wed, Jun 21, 2023 at 11:42:45AM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Jun 09, 2023 at 12:49:04AM +0200, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/qtest/migration-test.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> index 73b2f01427..95ccc9bce7 100644
> >> --- a/tests/qtest/migration-test.c
> >> +++ b/tests/qtest/migration-test.c
> >> @@ -681,9 +681,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> >>              "-object memory-backend-file,id=mem0,size=%s"
> >>              ",mem-path=%s,share=on -numa node,memdev=mem0",
> >>              memory_size, shmem_path);
> >> -    } else {
> >> -        shmem_path = NULL;
> >> -        shmem_opts = g_strdup("");
> >>      }
> >>  
> >>      cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> >> @@ -696,7 +693,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> >>                                   machine_opts ? " -machine " : "",
> >>                                   machine_opts ? machine_opts : "",
> >>                                   memory_size, tmpfs,
> >> -                                 arch_source, shmem_opts,
> >> +                                 arch_source,
> >> +                                 shmem_opts ? shmem_opts : "",
> >>                                   args->opts_source ? args->opts_source : "",
> >>                                   ignore_stderr ? ignore_stderr : "");
> >>  
> >> @@ -718,7 +716,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> >>                                   machine_opts ? " -machine " : "",
> >>                                   machine_opts ? machine_opts : "",
> >>                                   memory_size, tmpfs, uri,
> >> -                                 arch_target, shmem_opts,
> >> +                                 arch_target,
> >> +                                 shmem_opts ? shmem_opts : "",
> >
> > Isn't this adding duplications instead?
> 
> I don't follow.
> 
> > Meanwhile, shmem_opts right now is auto-free.  If we do this we don't need
> > it to be auto-free anymore..
> 
> We need.
> It can still be from g_strdup_printf().
> 
> What this patch change is that it will never be (again) "".
> 
> It is going to be NULL or a real string, like all the other options.
> The real string is generated, so it needs to be auto_free.

Ah ok.. after I read some other reply I think I see what you mean.

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.