| Message ID | 20231006123910.17759-8-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | tests/migration-test: Allow testing older machine types | expand |
Fabiano Rosas <farosas@suse.de> wrote: > Stop relying on defaults and select a machine explicitly for every > architecture. > > This is a prerequisite for being able to select machine types for > migration using different QEMU binaries for source and destination. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/migration-test.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 46f1c275a2..7c10ac925b 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, > const char *kvm_opts = NULL; > const char *arch = qtest_get_arch(); > const char *memory_size; > + const char *machine; > > if (args->use_shmem) { > if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { > @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, > got_dst_resume = false; > if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > memory_size = "150M"; > + machine = "pc"; I would suggest: if (strcmp(arch, "i386")) { machine = "pc"; } else { machine = "q35"; } New development is happening in q35, so I think this should be the more tested. > @@ -774,10 +777,12 @@ static int test_migrate_start(QTestState **from, QTestState **to, > "'nvramrc=hex .\" _\" begin %x %x " > "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " > "until'", end_address, start_address); > + machine = "pseries"; > arch_opts = g_strdup("-nodefaults -machine vsmt=8"); > } else if (strcmp(arch, "aarch64") == 0) { > memory_size = "150M"; > - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " > + machine = "virt"; > + arch_opts = g_strdup_printf("-machine gic-version=max -cpu max " Does this double -machine command line works? I expect yes, but who knows. Later, Juan.
Juan Quintela <quintela@redhat.com> writes: > Fabiano Rosas <farosas@suse.de> wrote: >> Stop relying on defaults and select a machine explicitly for every >> architecture. >> >> This is a prerequisite for being able to select machine types for >> migration using different QEMU binaries for source and destination. >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/migration-test.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> index 46f1c275a2..7c10ac925b 100644 >> --- a/tests/qtest/migration-test.c >> +++ b/tests/qtest/migration-test.c >> @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> const char *kvm_opts = NULL; >> const char *arch = qtest_get_arch(); >> const char *memory_size; >> + const char *machine; >> >> if (args->use_shmem) { >> if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { >> @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> got_dst_resume = false; >> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> memory_size = "150M"; >> + machine = "pc"; > > I would suggest: > > if (strcmp(arch, "i386")) { > machine = "pc"; > } else { > machine = "q35"; > } > > New development is happening in q35, so I think this should be the more tested. > Ok, I'll change it. >> @@ -774,10 +777,12 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> "'nvramrc=hex .\" _\" begin %x %x " >> "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " >> "until'", end_address, start_address); >> + machine = "pseries"; >> arch_opts = g_strdup("-nodefaults -machine vsmt=8"); >> } else if (strcmp(arch, "aarch64") == 0) { >> memory_size = "150M"; >> - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " >> + machine = "virt"; >> + arch_opts = g_strdup_printf("-machine gic-version=max -cpu max " > > Does this double -machine command line works? > > I expect yes, but who knows. I remember it did. But I'll double check just in case.
On Wed, Oct 11, 2023 at 04:28:41PM +0200, Juan Quintela wrote: > Fabiano Rosas <farosas@suse.de> wrote: > > Stop relying on defaults and select a machine explicitly for every > > architecture. > > > > This is a prerequisite for being able to select machine types for > > migration using different QEMU binaries for source and destination. > > > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > --- > > tests/qtest/migration-test.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > > index 46f1c275a2..7c10ac925b 100644 > > --- a/tests/qtest/migration-test.c > > +++ b/tests/qtest/migration-test.c > > @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, > > const char *kvm_opts = NULL; > > const char *arch = qtest_get_arch(); > > const char *memory_size; > > + const char *machine; > > > > if (args->use_shmem) { > > if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { > > @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, > > got_dst_resume = false; > > if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > > memory_size = "150M"; > > + machine = "pc"; > > I would suggest: > > if (strcmp(arch, "i386")) { > machine = "pc"; > } else { > machine = "q35"; > } > > New development is happening in q35, so I think this should be the more tested. > > > @@ -774,10 +777,12 @@ static int test_migrate_start(QTestState **from, QTestState **to, > > "'nvramrc=hex .\" _\" begin %x %x " > > "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " > > "until'", end_address, start_address); > > + machine = "pseries"; > > arch_opts = g_strdup("-nodefaults -machine vsmt=8"); > > } else if (strcmp(arch, "aarch64") == 0) { > > memory_size = "150M"; > > - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " > > + machine = "virt"; > > + arch_opts = g_strdup_printf("-machine gic-version=max -cpu max " > > Does this double -machine command line works? Why not just call the variable 'machine_opts' and here you can do - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " + machine_opts = "virt,gic-version=max"; + arch_opts = g_strdup_printf("-cpu max " With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Wed, Oct 11, 2023 at 04:28:41PM +0200, Juan Quintela wrote: >> Fabiano Rosas <farosas@suse.de> wrote: >> > Stop relying on defaults and select a machine explicitly for every >> > architecture. >> > >> > This is a prerequisite for being able to select machine types for >> > migration using different QEMU binaries for source and destination. >> > >> > Signed-off-by: Fabiano Rosas <farosas@suse.de> >> > --- >> > tests/qtest/migration-test.c | 11 ++++++++++- >> > 1 file changed, 10 insertions(+), 1 deletion(-) >> > >> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> > index 46f1c275a2..7c10ac925b 100644 >> > --- a/tests/qtest/migration-test.c >> > +++ b/tests/qtest/migration-test.c >> > @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> > const char *kvm_opts = NULL; >> > const char *arch = qtest_get_arch(); >> > const char *memory_size; >> > + const char *machine; >> > >> > if (args->use_shmem) { >> > if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { >> > @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> > got_dst_resume = false; >> > if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> > memory_size = "150M"; >> > + machine = "pc"; >> >> I would suggest: >> >> if (strcmp(arch, "i386")) { >> machine = "pc"; >> } else { >> machine = "q35"; >> } >> >> New development is happening in q35, so I think this should be the more tested. >> >> > @@ -774,10 +777,12 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> > "'nvramrc=hex .\" _\" begin %x %x " >> > "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " >> > "until'", end_address, start_address); >> > + machine = "pseries"; >> > arch_opts = g_strdup("-nodefaults -machine vsmt=8"); >> > } else if (strcmp(arch, "aarch64") == 0) { >> > memory_size = "150M"; >> > - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " >> > + machine = "virt"; >> > + arch_opts = g_strdup_printf("-machine gic-version=max -cpu max " >> >> Does this double -machine command line works? > > Why not just call the variable 'machine_opts' and here you can > do > > - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " > + machine_opts = "virt,gic-version=max"; > + arch_opts = g_strdup_printf("-cpu max " The machine name needs to be standalone so it can be overridden in the next patch when we compute the common machine type. Maybe I could add the machine_opts anyway just to make it more explicit.
On 06/10/2023 14.39, Fabiano Rosas wrote: > Stop relying on defaults and select a machine explicitly for every > architecture. > > This is a prerequisite for being able to select machine types for > migration using different QEMU binaries for source and destination. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/migration-test.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 46f1c275a2..7c10ac925b 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, > const char *kvm_opts = NULL; > const char *arch = qtest_get_arch(); > const char *memory_size; > + const char *machine; I'd maybe call this machine_alias to avoid confusion with the variable that is added in the next patch. Thomas
Juan Quintela <quintela@redhat.com> writes: > Fabiano Rosas <farosas@suse.de> wrote: >> Stop relying on defaults and select a machine explicitly for every >> architecture. >> >> This is a prerequisite for being able to select machine types for >> migration using different QEMU binaries for source and destination. >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/migration-test.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> index 46f1c275a2..7c10ac925b 100644 >> --- a/tests/qtest/migration-test.c >> +++ b/tests/qtest/migration-test.c >> @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> const char *kvm_opts = NULL; >> const char *arch = qtest_get_arch(); >> const char *memory_size; >> + const char *machine; >> >> if (args->use_shmem) { >> if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { >> @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, >> got_dst_resume = false; >> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> memory_size = "150M"; >> + machine = "pc"; > > I would suggest: > > if (strcmp(arch, "i386")) { > machine = "pc"; > } else { > machine = "q35"; > } Turns out we cannot run the tests with the q35 currently. It seems the bootsector we use to print the A and Bs is not recognized by seabios on that machine. I'm investigating.
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 46f1c275a2..7c10ac925b 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -746,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to, const char *kvm_opts = NULL; const char *arch = qtest_get_arch(); const char *memory_size; + const char *machine; if (args->use_shmem) { if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { @@ -758,11 +759,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, got_dst_resume = false; if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { memory_size = "150M"; + machine = "pc"; arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath); start_address = X86_TEST_MEM_START; end_address = X86_TEST_MEM_END; } else if (g_str_equal(arch, "s390x")) { memory_size = "128M"; + machine = "s390-ccw-virtio"; arch_opts = g_strdup_printf("-bios %s", bootpath); start_address = S390_TEST_MEM_START; end_address = S390_TEST_MEM_END; @@ -774,10 +777,12 @@ static int test_migrate_start(QTestState **from, QTestState **to, "'nvramrc=hex .\" _\" begin %x %x " "do i c@ 1 + i c! 1000 +loop .\" B\" 0 " "until'", end_address, start_address); + machine = "pseries"; arch_opts = g_strdup("-nodefaults -machine vsmt=8"); } else if (strcmp(arch, "aarch64") == 0) { memory_size = "150M"; - arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max " + machine = "virt"; + arch_opts = g_strdup_printf("-machine gic-version=max -cpu max " "-kernel %s", bootpath); start_address = ARM_TEST_MEM_START; end_address = ARM_TEST_MEM_END; @@ -813,11 +818,13 @@ static int test_migrate_start(QTestState **from, QTestState **to, } cmd_source = g_strdup_printf("-accel kvm%s -accel tcg " + "-machine %s " "-name source,debug-threads=on " "-m %s " "-serial file:%s/src_serial " "%s %s %s %s %s", kvm_opts ? kvm_opts : "", + machine, memory_size, tmpfs, arch_opts ? arch_opts : "", arch_source ? arch_source : "", @@ -832,12 +839,14 @@ static int test_migrate_start(QTestState **from, QTestState **to, } cmd_target = g_strdup_printf("-accel kvm%s -accel tcg " + "-machine %s " "-name target,debug-threads=on " "-m %s " "-serial file:%s/dest_serial " "-incoming %s " "%s %s %s %s %s", kvm_opts ? kvm_opts : "", + machine, memory_size, tmpfs, uri, arch_opts ? arch_opts : "", arch_target ? arch_target : "",
Stop relying on defaults and select a machine explicitly for every architecture. This is a prerequisite for being able to select machine types for migration using different QEMU binaries for source and destination. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/migration-test.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)