Hi

This series simplify test_migrate_start() in two ways:
- simplify the command line creation, so everything that is common between
  architectures don't have to be repeated (DRY).
  Note that this bit remove lines of code.
- test_migrate_start() has two bools and two strings as arguments, it is very
  difficult to remmeber which is which and meaning.  And it is even worse to
  add new parameters.  Just pass them through one struct.

Please, review.

Juan Quintela (10):
  migration-test: Create cmd_soure and cmd_target
  migration-test: Move hide_stderr to common commandline
  migration-test: Move -machine to common commandline
  migration-test: Move memory size to common commandline
  migration-test: Move shmem handling to common commandline
  migration-test: Move -name handling to common commandline
  migration-test: Move -serial handling to common commandline
  migration-test: Move -incomming handling to common commandline
  migration-test: Rename cmd_src/dst to arch_source/arch_target
  migration-test: Use a struct for test_migrate_start parameters

 tests/migration-test.c | 265 +++++++++++++++++++++++------------------
 1 file changed, 147 insertions(+), 118 deletions(-)

-- 
2.21.0

On 12/12/2019 23:20, Juan Quintela wrote:
> Hi
> 
> This series simplify test_migrate_start() in two ways:
> - simplify the command line creation, so everything that is common between
>   architectures don't have to be repeated (DRY).
>   Note that this bit remove lines of code.
> - test_migrate_start() has two bools and two strings as arguments, it is very
>   difficult to remmeber which is which and meaning.  And it is even worse to
>   add new parameters.  Just pass them through one struct.
> 
> Please, review.
> 
> Juan Quintela (10):
>   migration-test: Create cmd_soure and cmd_target
>   migration-test: Move hide_stderr to common commandline
>   migration-test: Move -machine to common commandline
>   migration-test: Move memory size to common commandline
>   migration-test: Move shmem handling to common commandline
>   migration-test: Move -name handling to common commandline
>   migration-test: Move -serial handling to common commandline
>   migration-test: Move -incomming handling to common commandline
>   migration-test: Rename cmd_src/dst to arch_source/arch_target
>   migration-test: Use a struct for test_migrate_start parameters
> 
>  tests/migration-test.c | 265 +++++++++++++++++++++++------------------
>  1 file changed, 147 insertions(+), 118 deletions(-)
> 

For ppc64:

Tested-by: Laurent Vivier <lvivier@redhat.com>

On Thu, 12 Dec 2019 23:20:23 +0100
Juan Quintela <quintela@redhat.com> wrote:

> Hi
> 
> This series simplify test_migrate_start() in two ways:
> - simplify the command line creation, so everything that is common between
>   architectures don't have to be repeated (DRY).
>   Note that this bit remove lines of code.
> - test_migrate_start() has two bools and two strings as arguments, it is very
>   difficult to remmeber which is which and meaning.  And it is even worse to
>   add new parameters.  Just pass them through one struct.
> 
> Please, review.
> 
> Juan Quintela (10):
>   migration-test: Create cmd_soure and cmd_target
>   migration-test: Move hide_stderr to common commandline
>   migration-test: Move -machine to common commandline
>   migration-test: Move memory size to common commandline
>   migration-test: Move shmem handling to common commandline
>   migration-test: Move -name handling to common commandline
>   migration-test: Move -serial handling to common commandline
>   migration-test: Move -incomming handling to common commandline
>   migration-test: Rename cmd_src/dst to arch_source/arch_target
>   migration-test: Use a struct for test_migrate_start parameters
> 
>  tests/migration-test.c | 265 +++++++++++++++++++++++------------------
>  1 file changed, 147 insertions(+), 118 deletions(-)
> 

I gave this a go on s390x, and the migration test still seems to work
fine there. So, feel free to add

Tested-by: Cornelia Huck <cohuck@redhat.com> #s390x

On 12/12/19 23:20, Juan Quintela wrote:
> Hi
> 
> This series simplify test_migrate_start() in two ways:
> - simplify the command line creation, so everything that is common between
>   architectures don't have to be repeated (DRY).
>   Note that this bit remove lines of code.
> - test_migrate_start() has two bools and two strings as arguments, it is very
>   difficult to remmeber which is which and meaning.  And it is even worse to
>   add new parameters.  Just pass them through one struct.
> 
> Please, review.
> 
> Juan Quintela (10):
>   migration-test: Create cmd_soure and cmd_target
>   migration-test: Move hide_stderr to common commandline
>   migration-test: Move -machine to common commandline
>   migration-test: Move memory size to common commandline
>   migration-test: Move shmem handling to common commandline
>   migration-test: Move -name handling to common commandline
>   migration-test: Move -serial handling to common commandline
>   migration-test: Move -incomming handling to common commandline
>   migration-test: Rename cmd_src/dst to arch_source/arch_target
>   migration-test: Use a struct for test_migrate_start parameters
> 
>  tests/migration-test.c | 265 +++++++++++++++++++++++------------------
>  1 file changed, 147 insertions(+), 118 deletions(-)
> 

I have picked up this series and rebased the -accel changes on top.

Paolo

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/12/19 23:20, Juan Quintela wrote:
>> Hi
>> 
>> This series simplify test_migrate_start() in two ways:
>> - simplify the command line creation, so everything that is common between
>>   architectures don't have to be repeated (DRY).
>>   Note that this bit remove lines of code.
>> - test_migrate_start() has two bools and two strings as arguments, it is very
>>   difficult to remmeber which is which and meaning.  And it is even worse to
>>   add new parameters.  Just pass them through one struct.
>> 
>> Please, review.
>> 
>> Juan Quintela (10):
>>   migration-test: Create cmd_soure and cmd_target
>>   migration-test: Move hide_stderr to common commandline
>>   migration-test: Move -machine to common commandline
>>   migration-test: Move memory size to common commandline
>>   migration-test: Move shmem handling to common commandline
>>   migration-test: Move -name handling to common commandline
>>   migration-test: Move -serial handling to common commandline
>>   migration-test: Move -incomming handling to common commandline
>>   migration-test: Rename cmd_src/dst to arch_source/arch_target
>>   migration-test: Use a struct for test_migrate_start parameters
>> 
>>  tests/migration-test.c | 265 +++++++++++++++++++++++------------------
>>  1 file changed, 147 insertions(+), 118 deletions(-)
>> 
>
> I have picked up this series and rebased the -accel changes on top.

Thanks.

about the accel and the machine type, .... it feel so weird that we only
need to add a machine type for aarch64.

Later, Juan.

On 16/12/19 16:46, Juan Quintela wrote:
>> I have picked up this series and rebased the -accel changes on top.
> Thanks.
> 
> about the accel and the machine type, .... it feel so weird that we only
> need to add a machine type for aarch64.

Yes, it is.  For now I have resolved the conflict to something like

    ...
    machine_opts = "virt,gic_ver=max";

and then if machine_opts is NULL I don't need to add "-machine" at all, so

   "...%s%s " ...,
   machine_opts ? " -machine" : NULL,
   machine_opts ? machine_opts : NULL,
   ...

Paolo

Paolo