| Message ID | 20191212222033.1026-1-quintela@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Migration Arguments cleanup | expand |
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/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 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