* [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled
@ 2023-01-19 14:58 Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array Philippe Mathieu-Daudé
` (10 more replies)
0 siblings, 11 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Two test were failing on Darwin when testing Fabiano's series
which allows building ARM targets without TCG accelerator:
https://lore.kernel.org/qemu-devel/20230118193518.26433-1-farosas@suse.de/
These patches allow boot-serial / migration tests to run without
TCG / KVM.
Since v1:
- Split GString API conversion in multiple trivial patches (David)
- Use KVM first in migration test (David)
- Dropped HVF changes for now.
Philippe Mathieu-Daudé (11):
tests/qtest/boot-serial-test: Constify tests[] array
tests/qtest/boot-serial-test: Simplify test_machine() a bit
tests/qtest/boot-serial-test: Build command line using GString API
tests/qtest/boot-serial-test: Only use available accelerators
tests/qtest/migration-test: Inverse #ifdef'ry ladders
tests/qtest/migration-test: Reduce 'cmd_source' string scope
tests/qtest/migration-test: Build command line using GString API (1/4)
tests/qtest/migration-test: Build command line using GString API (2/4)
tests/qtest/migration-test: Build command line using GString API (3/4)
tests/qtest/migration-test: Build command line using GString API (4/4)
tests/qtest/migration-test: Only use available accelerators
tests/qtest/boot-serial-test.c | 46 +++++++++-------
tests/qtest/migration-test.c | 95 ++++++++++++++++++----------------
2 files changed, 79 insertions(+), 62 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 20:54 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit Philippe Mathieu-Daudé
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index b216519b62..3aef3a97a9 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -139,7 +139,7 @@ typedef struct testdef {
const uint8_t *bios; /* Set in case we use our own mini bios */
} testdef_t;
-static testdef_t tests[] = {
+static const testdef_t tests[] = {
{ "alpha", "clipper", "", "PCI:" },
{ "avr", "arduino-duemilanove", "", "T", sizeof(bios_avr), NULL, bios_avr },
{ "avr", "arduino-mega-2560-v3", "", "T", sizeof(bios_avr), NULL, bios_avr},
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 20:59 ` Richard Henderson
2023-01-20 8:54 ` Thomas Huth
2023-01-19 14:58 ` [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API Philippe Mathieu-Daudé
` (8 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Slighly modify test_machine() to simplify next commit review.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 3aef3a97a9..3a854b0174 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -227,7 +227,6 @@ static void test_machine(const void *data)
g_autofree char *serialtmp = NULL;
g_autofree char *codetmp = NULL;
const char *codeparam = "";
- const uint8_t *code = NULL;
QTestState *qts;
int ser_fd;
@@ -235,21 +234,13 @@ static void test_machine(const void *data)
g_assert(ser_fd != -1);
close(ser_fd);
- if (test->kernel) {
- code = test->kernel;
- codeparam = "-kernel";
- } else if (test->bios) {
- code = test->bios;
- codeparam = "-bios";
- }
-
- if (code) {
+ if (test->kernel || test->bios) {
ssize_t wlen;
int code_fd;
code_fd = g_file_open_tmp("qtest-boot-serial-cXXXXXX", &codetmp, NULL);
g_assert(code_fd != -1);
- wlen = write(code_fd, code, test->codesize);
+ wlen = write(code_fd, test->kernel ? : test->bios, test->codesize);
g_assert(wlen == test->codesize);
close(code_fd);
}
@@ -258,12 +249,14 @@ static void test_machine(const void *data)
* Make sure that this test uses tcg if available: It is used as a
* fast-enough smoketest for that.
*/
- qts = qtest_initf("%s %s -M %s -no-shutdown "
+ qts = qtest_initf("%s %s %s -M %s -no-shutdown "
"-chardev file,id=serial0,path=%s "
"-serial chardev:serial0 -accel tcg -accel kvm %s",
- codeparam, code ? codetmp : "", test->machine,
+ codeparam,
+ test->kernel ? "-kernel " : test->bios ? "-bios " : "",
+ codetmp ? : "", test->machine,
serialtmp, test->extra);
- if (code) {
+ if (codetmp) {
unlink(codetmp);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:03 ` Richard Henderson
2023-01-20 8:59 ` Thomas Huth
2023-01-19 14:58 ` [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators Philippe Mathieu-Daudé
` (7 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 3a854b0174..92890b409d 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -226,14 +226,17 @@ static void test_machine(const void *data)
const testdef_t *test = data;
g_autofree char *serialtmp = NULL;
g_autofree char *codetmp = NULL;
- const char *codeparam = "";
QTestState *qts;
int ser_fd;
+ g_autoptr(GString) cmd = g_string_new("");
ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX", &serialtmp, NULL);
g_assert(ser_fd != -1);
close(ser_fd);
+ g_string_append_printf(cmd, "-M %s ", test->machine);
+ g_string_append(cmd, "-no-shutdown ");
+
if (test->kernel || test->bios) {
ssize_t wlen;
int code_fd;
@@ -243,19 +246,23 @@ static void test_machine(const void *data)
wlen = write(code_fd, test->kernel ? : test->bios, test->codesize);
g_assert(wlen == test->codesize);
close(code_fd);
+ g_string_append_printf(cmd, "%s %s ",
+ test->kernel ? "-kernel " : "-bios ", codetmp);
}
+ g_string_append_printf(cmd, "-chardev file,id=serial0,path=%s "
+ "-serial chardev:serial0 ", serialtmp);
+
/*
* Make sure that this test uses tcg if available: It is used as a
* fast-enough smoketest for that.
*/
- qts = qtest_initf("%s %s %s -M %s -no-shutdown "
- "-chardev file,id=serial0,path=%s "
- "-serial chardev:serial0 -accel tcg -accel kvm %s",
- codeparam,
- test->kernel ? "-kernel " : test->bios ? "-bios " : "",
- codetmp ? : "", test->machine,
- serialtmp, test->extra);
+ g_string_append(cmd, "-accel tcg ");
+ g_string_append(cmd, "-accel kvm ");
+ g_string_append(cmd, test->extra);
+
+ qts = qtest_init(cmd->str);
+
if (codetmp) {
unlink(codetmp);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 15:18 ` Fabiano Rosas
2023-01-19 21:05 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders Philippe Mathieu-Daudé
` (6 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
For example, avoid when TCG is disabled:
$ make check-qtest-aarch64
...
18/20 qemu:qtest+qtest-aarch64 / qtest-aarch64/boot-serial-test
qemu-system-aarch64: -accel tcg: invalid accelerator tcg
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/boot-serial-test.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 92890b409d..f375b16dae 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -17,6 +17,9 @@
#include "libqtest.h"
#include "libqos/libqos-spapr.h"
+static bool has_tcg;
+static bool has_kvm;
+
static const uint8_t bios_avr[] = {
0x88, 0xe0, /* ldi r24, 0x08 */
0x80, 0x93, 0xc1, 0x00, /* sts 0x00C1, r24 ; Enable tx */
@@ -257,8 +260,12 @@ static void test_machine(const void *data)
* Make sure that this test uses tcg if available: It is used as a
* fast-enough smoketest for that.
*/
- g_string_append(cmd, "-accel tcg ");
- g_string_append(cmd, "-accel kvm ");
+ if (has_tcg) {
+ g_string_append(cmd, "-accel tcg ");
+ }
+ if (has_kvm) {
+ g_string_append(cmd, "-accel kvm ");
+ }
g_string_append(cmd, test->extra);
qts = qtest_init(cmd->str);
@@ -285,6 +292,9 @@ int main(int argc, char *argv[])
const char *arch = qtest_get_arch();
int i;
+ has_tcg = qtest_has_accel("tcg");
+ has_kvm = qtest_has_accel("kvm");
+
g_test_init(&argc, &argv, NULL);
for (i = 0; tests[i].arch != NULL; i++) {
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:06 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope Philippe Mathieu-Daudé
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
This slighly simplify the logic, and eases the following conversion.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index dbde726adf..8beeda4686 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -647,15 +647,15 @@ static int test_migrate_start(QTestState **from, QTestState **to,
}
if (!getenv("QTEST_LOG") && args->hide_stderr) {
-#ifndef _WIN32
- ignore_stderr = "2>/dev/null";
-#else
+#ifdef _WIN32
/*
* On Windows the QEMU executable is created via CreateProcess() and
* IO redirection does not work, so don't bother adding IO redirection
* to the command line.
*/
ignore_stderr = "";
+#else
+ ignore_stderr = "2>/dev/null";
#endif
} else {
ignore_stderr = "";
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:08 ` Richard Henderson
2023-01-20 9:02 ` Thomas Huth
2023-01-19 14:58 ` [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4) Philippe Mathieu-Daudé
` (4 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 8beeda4686..6c3db95113 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -584,7 +584,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
{
g_autofree gchar *arch_source = NULL;
g_autofree gchar *arch_target = NULL;
- g_autofree gchar *cmd_source = NULL;
g_autofree gchar *cmd_target = NULL;
const gchar *ignore_stderr;
g_autofree char *bootpath = NULL;
@@ -672,20 +671,22 @@ static int test_migrate_start(QTestState **from, QTestState **to,
shmem_opts = g_strdup("");
}
- cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
- "-name source,debug-threads=on "
- "-m %s "
- "-serial file:%s/src_serial "
- "%s %s %s %s",
- args->use_dirty_ring ?
- ",dirty-ring-size=4096" : "",
- machine_opts ? " -machine " : "",
- machine_opts ? machine_opts : "",
- memory_size, tmpfs,
- arch_source, shmem_opts,
- args->opts_source ? args->opts_source : "",
- ignore_stderr);
if (!args->only_target) {
+ g_autofree gchar *cmd_source = NULL;
+
+ cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
+ "-name source,debug-threads=on "
+ "-m %s "
+ "-serial file:%s/src_serial "
+ "%s %s %s %s",
+ args->use_dirty_ring ?
+ ",dirty-ring-size=4096" : "",
+ machine_opts ? " -machine " : "",
+ machine_opts ? machine_opts : "",
+ memory_size, tmpfs,
+ arch_source, shmem_opts,
+ args->opts_source ? args->opts_source : "",
+ ignore_stderr);
*from = qtest_init(cmd_source);
}
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4)
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:10 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4) Philippe Mathieu-Daudé
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Part 1/4: Convert memory & machine options.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6c3db95113..7aa323a7a7 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -582,6 +582,7 @@ typedef struct {
static int test_migrate_start(QTestState **from, QTestState **to,
const char *uri, MigrateStart *args)
{
+ g_autoptr(GString) cmd_common = NULL;
g_autofree gchar *arch_source = NULL;
g_autofree gchar *arch_target = NULL;
g_autofree gchar *cmd_target = NULL;
@@ -601,6 +602,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
}
got_stop = false;
+
+ cmd_common = g_string_new("");
+
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
/* the assembled x86 boot sector should be exactly one sector large */
@@ -644,6 +648,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
} else {
g_assert_not_reached();
}
+ if (machine_opts) {
+ g_string_append_printf(cmd_common, " -machine %s ", machine_opts);
+ }
+ g_string_append_printf(cmd_common, "-m %s ", memory_size);
if (!getenv("QTEST_LOG") && args->hide_stderr) {
#ifdef _WIN32
@@ -674,33 +682,27 @@ static int test_migrate_start(QTestState **from, QTestState **to,
if (!args->only_target) {
g_autofree gchar *cmd_source = NULL;
- cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
+ cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s "
"-name source,debug-threads=on "
- "-m %s "
"-serial file:%s/src_serial "
"%s %s %s %s",
args->use_dirty_ring ?
",dirty-ring-size=4096" : "",
- machine_opts ? " -machine " : "",
- machine_opts ? machine_opts : "",
- memory_size, tmpfs,
+ cmd_common->str, tmpfs,
arch_source, shmem_opts,
args->opts_source ? args->opts_source : "",
ignore_stderr);
*from = qtest_init(cmd_source);
}
- cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
+ cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s "
"-name target,debug-threads=on "
- "-m %s "
"-serial file:%s/dest_serial "
"-incoming %s "
"%s %s %s %s",
args->use_dirty_ring ?
",dirty-ring-size=4096" : "",
- machine_opts ? " -machine " : "",
- machine_opts ? machine_opts : "",
- memory_size, tmpfs, uri,
+ cmd_common->str, tmpfs, uri,
arch_target, shmem_opts,
args->opts_target ? args->opts_target : "",
ignore_stderr);
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4)
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4) Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:13 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4) Philippe Mathieu-Daudé
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Part 2/4: Convert shmem option.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 7aa323a7a7..8377b3976a 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -588,7 +588,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
g_autofree gchar *cmd_target = NULL;
const gchar *ignore_stderr;
g_autofree char *bootpath = NULL;
- g_autofree char *shmem_opts = NULL;
g_autofree char *shmem_path = NULL;
const char *arch = qtest_get_arch();
const char *machine_opts = NULL;
@@ -670,13 +669,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
if (args->use_shmem) {
shmem_path = g_strdup_printf("/dev/shm/qemu-%d", getpid());
- shmem_opts = g_strdup_printf(
+ g_string_append_printf(cmd_common,
"-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("");
}
if (!args->only_target) {
@@ -685,11 +681,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s "
"-name source,debug-threads=on "
"-serial file:%s/src_serial "
- "%s %s %s %s",
+ "%s %s %s",
args->use_dirty_ring ?
",dirty-ring-size=4096" : "",
- cmd_common->str, tmpfs,
- arch_source, shmem_opts,
+ cmd_common->str, tmpfs, arch_source,
args->opts_source ? args->opts_source : "",
ignore_stderr);
*from = qtest_init(cmd_source);
@@ -699,11 +694,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
"-name target,debug-threads=on "
"-serial file:%s/dest_serial "
"-incoming %s "
- "%s %s %s %s",
+ "%s %s %s",
args->use_dirty_ring ?
",dirty-ring-size=4096" : "",
- cmd_common->str, tmpfs, uri,
- arch_target, shmem_opts,
+ cmd_common->str, tmpfs, uri, arch_target,
args->opts_target ? args->opts_target : "",
ignore_stderr);
*to = qtest_init(cmd_target);
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4)
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4) Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:17 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4) Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators Philippe Mathieu-Daudé
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Part 3/4: Convert accelerator options.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 8377b3976a..015b774a9e 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -603,6 +603,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
got_stop = false;
cmd_common = g_string_new("");
+ g_string_append_printf(cmd_common, "-accel kvm%s ",
+ args->use_dirty_ring ? ",dirty-ring-size=4096" : "");
+ g_string_append(cmd_common, "-accel tcg ");
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
@@ -678,25 +681,21 @@ static int test_migrate_start(QTestState **from, QTestState **to,
if (!args->only_target) {
g_autofree gchar *cmd_source = NULL;
- cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s "
+ cmd_source = g_strdup_printf("%s "
"-name source,debug-threads=on "
"-serial file:%s/src_serial "
"%s %s %s",
- args->use_dirty_ring ?
- ",dirty-ring-size=4096" : "",
cmd_common->str, tmpfs, arch_source,
args->opts_source ? args->opts_source : "",
ignore_stderr);
*from = qtest_init(cmd_source);
}
- cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s "
+ cmd_target = g_strdup_printf("%s "
"-name target,debug-threads=on "
"-serial file:%s/dest_serial "
"-incoming %s "
"%s %s %s",
- args->use_dirty_ring ?
- ",dirty-ring-size=4096" : "",
cmd_common->str, tmpfs, uri, arch_target,
args->opts_target ? args->opts_target : "",
ignore_stderr);
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4)
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4) Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:19 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators Philippe Mathieu-Daudé
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
Part 4/4: Convert rest of options.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 50 +++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 015b774a9e..a930964268 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -585,8 +585,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
g_autoptr(GString) cmd_common = NULL;
g_autofree gchar *arch_source = NULL;
g_autofree gchar *arch_target = NULL;
- g_autofree gchar *cmd_target = NULL;
- const gchar *ignore_stderr;
+ g_autoptr(GString) cmd_target = NULL;
+ const gchar *ignore_stderr = NULL;
g_autofree char *bootpath = NULL;
g_autofree char *shmem_path = NULL;
const char *arch = qtest_get_arch();
@@ -662,12 +662,9 @@ 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 = "";
#else
ignore_stderr = "2>/dev/null";
#endif
- } else {
- ignore_stderr = "";
}
if (args->use_shmem) {
@@ -679,27 +676,32 @@ static int test_migrate_start(QTestState **from, QTestState **to,
}
if (!args->only_target) {
- g_autofree gchar *cmd_source = NULL;
-
- cmd_source = g_strdup_printf("%s "
- "-name source,debug-threads=on "
- "-serial file:%s/src_serial "
- "%s %s %s",
- cmd_common->str, tmpfs, arch_source,
- args->opts_source ? args->opts_source : "",
- ignore_stderr);
- *from = qtest_init(cmd_source);
+ g_autoptr(GString) cmd_source = g_string_new(cmd_common->str);
+ g_string_append(cmd_source, "-name source,debug-threads=on ");
+ g_string_append_printf(cmd_source, "-serial file:%s/src_serial ",
+ tmpfs);
+ g_string_append_printf(cmd_source, "%s ", arch_source);
+ if (args->opts_source) {
+ g_string_append_printf(cmd_source, "%s ", args->opts_source);
+ }
+ if (ignore_stderr) {
+ g_string_append(cmd_source, ignore_stderr); /* last string */
+ }
+ *from = qtest_init(cmd_source->str);
}
- cmd_target = g_strdup_printf("%s "
- "-name target,debug-threads=on "
- "-serial file:%s/dest_serial "
- "-incoming %s "
- "%s %s %s",
- cmd_common->str, tmpfs, uri, arch_target,
- args->opts_target ? args->opts_target : "",
- ignore_stderr);
- *to = qtest_init(cmd_target);
+ cmd_target = g_string_new(cmd_common->str);
+ g_string_append(cmd_target, "-name target,debug-threads=on ");
+ g_string_append_printf(cmd_target, "-serial file:%s/dest_serial ", tmpfs);
+ g_string_append_printf(cmd_target, "-incoming %s ", uri);
+ g_string_append_printf(cmd_target, "%s ", arch_target);
+ if (args->opts_target) {
+ g_string_append_printf(cmd_target, "%s ", args->opts_target);
+ }
+ if (ignore_stderr) {
+ g_string_append(cmd_target, ignore_stderr); /* last string */
+ }
+ *to = qtest_init(cmd_target->str);
/*
* Remove shmem file immediately to avoid memory leak in test failed case.
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2023-01-19 14:58 ` [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4) Philippe Mathieu-Daudé
@ 2023-01-19 14:58 ` Philippe Mathieu-Daudé
2023-01-19 21:19 ` Richard Henderson
10 siblings, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-19 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth,
Philippe Mathieu-Daudé
For example, avoid when TCG is disabled:
$ make check-qtest-aarch64
...
20/20 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test
qemu-system-aarch64: -accel tcg: invalid accelerator tcg
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/qtest/migration-test.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index a930964268..17783d7334 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -45,6 +45,8 @@
unsigned start_address;
unsigned end_address;
+static bool has_tcg;
+static bool has_kvm;
static bool uffd_feature_thread_id;
/*
@@ -603,9 +605,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
got_stop = false;
cmd_common = g_string_new("");
- g_string_append_printf(cmd_common, "-accel kvm%s ",
- args->use_dirty_ring ? ",dirty-ring-size=4096" : "");
- g_string_append(cmd_common, "-accel tcg ");
+ if (has_kvm) { /* KVM first */
+ g_string_append_printf(cmd_common, "-accel kvm%s ",
+ args->use_dirty_ring
+ ? ",dirty-ring-size=4096" : "");
+ }
+ if (has_tcg) {
+ g_string_append(cmd_common, "-accel tcg ");
+ }
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
@@ -2457,12 +2464,14 @@ static bool kvm_dirty_ring_supported(void)
int main(int argc, char **argv)
{
- const bool has_kvm = qtest_has_accel("kvm");
const bool has_uffd = ufd_version_check();
const char *arch = qtest_get_arch();
g_autoptr(GError) err = NULL;
int ret;
+ has_tcg = qtest_has_accel("tcg");
+ has_kvm = qtest_has_accel("kvm");
+
g_test_init(&argc, &argv, NULL);
/*
--
2.38.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators
2023-01-19 14:58 ` [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators Philippe Mathieu-Daudé
@ 2023-01-19 15:18 ` Fabiano Rosas
2023-01-19 21:05 ` Richard Henderson
1 sibling, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2023-01-19 15:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Thomas Huth,
Philippe Mathieu-Daudé
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> For example, avoid when TCG is disabled:
>
> $ make check-qtest-aarch64
> ...
> 18/20 qemu:qtest+qtest-aarch64 / qtest-aarch64/boot-serial-test
> qemu-system-aarch64: -accel tcg: invalid accelerator tcg
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array
2023-01-19 14:58 ` [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array Philippe Mathieu-Daudé
@ 2023-01-19 20:54 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 20:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit
2023-01-19 14:58 ` [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit Philippe Mathieu-Daudé
@ 2023-01-19 20:59 ` Richard Henderson
2023-01-20 8:54 ` Thomas Huth
1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 20:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Slighly modify test_machine() to simplify next commit review.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API
2023-01-19 14:58 ` [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API Philippe Mathieu-Daudé
@ 2023-01-19 21:03 ` Richard Henderson
2023-01-20 8:59 ` Thomas Huth
1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators
2023-01-19 14:58 ` [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators Philippe Mathieu-Daudé
2023-01-19 15:18 ` Fabiano Rosas
@ 2023-01-19 21:05 ` Richard Henderson
1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> For example, avoid when TCG is disabled:
>
> $ make check-qtest-aarch64
> ...
> 18/20 qemu:qtest+qtest-aarch64 / qtest-aarch64/boot-serial-test
> qemu-system-aarch64: -accel tcg: invalid accelerator tcg
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders
2023-01-19 14:58 ` [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders Philippe Mathieu-Daudé
@ 2023-01-19 21:06 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> This slighly simplify the logic, and eases the following conversion.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope
2023-01-19 14:58 ` [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope Philippe Mathieu-Daudé
@ 2023-01-19 21:08 ` Richard Henderson
2023-01-20 9:02 ` Thomas Huth
1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4)
2023-01-19 14:58 ` [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4) Philippe Mathieu-Daudé
@ 2023-01-19 21:10 ` Richard Henderson
2023-01-20 7:32 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Part 1/4: Convert memory & machine options.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 6c3db95113..7aa323a7a7 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -582,6 +582,7 @@ typedef struct {
> static int test_migrate_start(QTestState **from, QTestState **to,
> const char *uri, MigrateStart *args)
> {
> + g_autoptr(GString) cmd_common = NULL;
> g_autofree gchar *arch_source = NULL;
> g_autofree gchar *arch_target = NULL;
> g_autofree gchar *cmd_target = NULL;
> @@ -601,6 +602,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> }
>
> got_stop = false;
> +
> + cmd_common = g_string_new("");
> +
> bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> /* the assembled x86 boot sector should be exactly one sector large */
> @@ -644,6 +648,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> } else {
> g_assert_not_reached();
> }
> + if (machine_opts) {
> + g_string_append_printf(cmd_common, " -machine %s ", machine_opts);
> + }
> + g_string_append_printf(cmd_common, "-m %s ", memory_size);
>
> if (!getenv("QTEST_LOG") && args->hide_stderr) {
> #ifdef _WIN32
> @@ -674,33 +682,27 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> if (!args->only_target) {
> g_autofree gchar *cmd_source = NULL;
>
> - cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> + cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s "
> "-name source,debug-threads=on "
> - "-m %s "
> "-serial file:%s/src_serial "
> "%s %s %s %s",
> args->use_dirty_ring ?
> ",dirty-ring-size=4096" : "",
> - machine_opts ? " -machine " : "",
> - machine_opts ? machine_opts : "",
You removed two strings here, but only one %s above.
r~
> - memory_size, tmpfs,
> + cmd_common->str, tmpfs,
> arch_source, shmem_opts,
> args->opts_source ? args->opts_source : "",
> ignore_stderr);
> *from = qtest_init(cmd_source);
> }
>
> - cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
> + cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s "
> "-name target,debug-threads=on "
> - "-m %s "
> "-serial file:%s/dest_serial "
> "-incoming %s "
> "%s %s %s %s",
> args->use_dirty_ring ?
> ",dirty-ring-size=4096" : "",
> - machine_opts ? " -machine " : "",
> - machine_opts ? machine_opts : "",
> - memory_size, tmpfs, uri,
> + cmd_common->str, tmpfs, uri,
> arch_target, shmem_opts,
> args->opts_target ? args->opts_target : "",
> ignore_stderr);
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4)
2023-01-19 14:58 ` [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4) Philippe Mathieu-Daudé
@ 2023-01-19 21:13 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Part 2/4: Convert shmem option.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 16 +++++-----------
> 1 file changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4)
2023-01-19 14:58 ` [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4) Philippe Mathieu-Daudé
@ 2023-01-19 21:17 ` Richard Henderson
2023-01-20 7:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Part 3/4: Convert accelerator options.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 8377b3976a..015b774a9e 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -603,6 +603,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> got_stop = false;
>
> cmd_common = g_string_new("");
> + g_string_append_printf(cmd_common, "-accel kvm%s ",
> + args->use_dirty_ring ? ",dirty-ring-size=4096" : "");
> + g_string_append(cmd_common, "-accel tcg ");
Maybe clearer as
if (args->use_dirty_ring) {
g_string_append(s, "-accel kvm,dirty-ring-size=4096 ");
} else {
g_string_append(s, "-accel kvm ");
}
but what you have works, so,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4)
2023-01-19 14:58 ` [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4) Philippe Mathieu-Daudé
@ 2023-01-19 21:19 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> Part 4/4: Convert rest of options.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 50 +++++++++++++++++++-----------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators
2023-01-19 14:58 ` [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators Philippe Mathieu-Daudé
@ 2023-01-19 21:19 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2023-01-19 21:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
> For example, avoid when TCG is disabled:
>
> $ make check-qtest-aarch64
> ...
> 20/20 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test
> qemu-system-aarch64: -accel tcg: invalid accelerator tcg
>
> Reviewed-by: Dr. David Alan Gilbert<dgilbert@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4)
2023-01-19 21:17 ` Richard Henderson
@ 2023-01-20 7:05 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-20 7:05 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 19/1/23 22:17, Richard Henderson wrote:
> On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
>> Part 3/4: Convert accelerator options.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/qtest/migration-test.c | 11 +++++------
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 8377b3976a..015b774a9e 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -603,6 +603,9 @@ static int test_migrate_start(QTestState **from,
>> QTestState **to,
>> got_stop = false;
>> cmd_common = g_string_new("");
>> + g_string_append_printf(cmd_common, "-accel kvm%s ",
>> + args->use_dirty_ring ?
>> ",dirty-ring-size=4096" : "");
>> + g_string_append(cmd_common, "-accel tcg ");
>
> Maybe clearer as
>
> if (args->use_dirty_ring) {
> g_string_append(s, "-accel kvm,dirty-ring-size=4096 ");
> } else {
> g_string_append(s, "-accel kvm ");
> }
Agreed, I first did that change, then went back to have simpler "one
big patch" in v1. Now since v2 splits the changes I'll do that.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4)
2023-01-19 21:10 ` Richard Henderson
@ 2023-01-20 7:32 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-20 7:32 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas, Thomas Huth
On 19/1/23 22:10, Richard Henderson wrote:
> On 1/19/23 04:58, Philippe Mathieu-Daudé wrote:
>> Part 1/4: Convert memory & machine options.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/qtest/migration-test.c | 22 ++++++++++++----------
>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 6c3db95113..7aa323a7a7 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -582,6 +582,7 @@ typedef struct {
>> static int test_migrate_start(QTestState **from, QTestState **to,
>> const char *uri, MigrateStart *args)
>> {
>> + g_autoptr(GString) cmd_common = NULL;
>> g_autofree gchar *arch_source = NULL;
>> g_autofree gchar *arch_target = NULL;
>> g_autofree gchar *cmd_target = NULL;
>> @@ -601,6 +602,9 @@ static int test_migrate_start(QTestState **from,
>> QTestState **to,
>> }
>> got_stop = false;
>> +
>> + cmd_common = g_string_new("");
>> +
>> bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> /* the assembled x86 boot sector should be exactly one
>> sector large */
>> @@ -644,6 +648,10 @@ static int test_migrate_start(QTestState **from,
>> QTestState **to,
>> } else {
>> g_assert_not_reached();
>> }
>> + if (machine_opts) {
>> + g_string_append_printf(cmd_common, " -machine %s ",
>> machine_opts);
>> + }
>> + g_string_append_printf(cmd_common, "-m %s ", memory_size);
>> if (!getenv("QTEST_LOG") && args->hide_stderr) {
>> #ifdef _WIN32
>> @@ -674,33 +682,27 @@ static int test_migrate_start(QTestState **from,
>> QTestState **to,
>> if (!args->only_target) {
>> g_autofree gchar *cmd_source = NULL;
>> - cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>> + cmd_source = g_strdup_printf("-accel kvm%s -accel tcg%s "
>> "-name source,debug-threads=on "
>> - "-m %s "
>> "-serial file:%s/src_serial "
>> "%s %s %s %s",
>> args->use_dirty_ring ?
>> ",dirty-ring-size=4096" : "",
>> - machine_opts ? " -machine " : "",
>> - machine_opts ? machine_opts : "",
>
> You removed two strings here, but only one %s above.
>> - memory_size, tmpfs,
>> + cmd_common->str, tmpfs,
One new string is added here ^^^^^^^^^^^^^^^.
>> arch_source, shmem_opts,
>> args->opts_source ?
>> args->opts_source : "",
>> ignore_stderr);
>> *from = qtest_init(cmd_source);
>> }
>> - cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
>> + cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s "
>> "-name target,debug-threads=on "
>> - "-m %s "
>> "-serial file:%s/dest_serial "
>> "-incoming %s "
>> "%s %s %s %s",
>> args->use_dirty_ring ?
>> ",dirty-ring-size=4096" : "",
>> - machine_opts ? " -machine " : "",
>> - machine_opts ? machine_opts : "",
>> - memory_size, tmpfs, uri,
>> + cmd_common->str, tmpfs, uri,
>> arch_target, shmem_opts,
>> args->opts_target ?
>> args->opts_target : "",
>> ignore_stderr);
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit
2023-01-19 14:58 ` [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit Philippe Mathieu-Daudé
2023-01-19 20:59 ` Richard Henderson
@ 2023-01-20 8:54 ` Thomas Huth
1 sibling, 0 replies; 31+ messages in thread
From: Thomas Huth @ 2023-01-20 8:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas
On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
> Slighly modify test_machine() to simplify next commit review.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 3aef3a97a9..3a854b0174 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -227,7 +227,6 @@ static void test_machine(const void *data)
> g_autofree char *serialtmp = NULL;
> g_autofree char *codetmp = NULL;
> const char *codeparam = "";
> - const uint8_t *code = NULL;
> QTestState *qts;
> int ser_fd;
>
> @@ -235,21 +234,13 @@ static void test_machine(const void *data)
> g_assert(ser_fd != -1);
> close(ser_fd);
>
> - if (test->kernel) {
> - code = test->kernel;
> - codeparam = "-kernel";
> - } else if (test->bios) {
> - code = test->bios;
> - codeparam = "-bios";
> - }
> -
> - if (code) {
> + if (test->kernel || test->bios) {
> ssize_t wlen;
> int code_fd;
>
> code_fd = g_file_open_tmp("qtest-boot-serial-cXXXXXX", &codetmp, NULL);
> g_assert(code_fd != -1);
> - wlen = write(code_fd, code, test->codesize);
> + wlen = write(code_fd, test->kernel ? : test->bios, test->codesize);
Just a matter of taste, but I prefer the Elvis operator without space in
between.
> g_assert(wlen == test->codesize);
> close(code_fd);
> }
> @@ -258,12 +249,14 @@ static void test_machine(const void *data)
> * Make sure that this test uses tcg if available: It is used as a
> * fast-enough smoketest for that.
> */
> - qts = qtest_initf("%s %s -M %s -no-shutdown "
> + qts = qtest_initf("%s %s %s -M %s -no-shutdown "
> "-chardev file,id=serial0,path=%s "
> "-serial chardev:serial0 -accel tcg -accel kvm %s",
> - codeparam, code ? codetmp : "", test->machine,
> + codeparam,
You removed the initialization of codeparam, so this is now always the empty
string, isn't it? ... please just remove it completely now.
> + test->kernel ? "-kernel " : test->bios ? "-bios " : "",
> + codetmp ? : "", test->machine,
> serialtmp, test->extra);
> - if (code) {
> + if (codetmp) {
> unlink(codetmp);
> }
>
Thomas
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API
2023-01-19 14:58 ` [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API Philippe Mathieu-Daudé
2023-01-19 21:03 ` Richard Henderson
@ 2023-01-20 8:59 ` Thomas Huth
2023-01-20 11:04 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2023-01-20 8:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas
On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/boot-serial-test.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 3a854b0174..92890b409d 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -226,14 +226,17 @@ static void test_machine(const void *data)
> const testdef_t *test = data;
> g_autofree char *serialtmp = NULL;
> g_autofree char *codetmp = NULL;
> - const char *codeparam = "";
> QTestState *qts;
> int ser_fd;
> + g_autoptr(GString) cmd = g_string_new("");
You could already start with the "-no-shutdown" here.
> ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX", &serialtmp, NULL);
> g_assert(ser_fd != -1);
> close(ser_fd);
>
> + g_string_append_printf(cmd, "-M %s ", test->machine);
> + g_string_append(cmd, "-no-shutdown ");
> +
> if (test->kernel || test->bios) {
> ssize_t wlen;
> int code_fd;
> @@ -243,19 +246,23 @@ static void test_machine(const void *data)
> wlen = write(code_fd, test->kernel ? : test->bios, test->codesize);
> g_assert(wlen == test->codesize);
> close(code_fd);
> + g_string_append_printf(cmd, "%s %s ",
> + test->kernel ? "-kernel " : "-bios ", codetmp);
> }
>
> + g_string_append_printf(cmd, "-chardev file,id=serial0,path=%s "
> + "-serial chardev:serial0 ", serialtmp);
Why not include this in the initial g_string_append_printf statement already?
> /*
> * Make sure that this test uses tcg if available: It is used as a
> * fast-enough smoketest for that.
> */
> - qts = qtest_initf("%s %s %s -M %s -no-shutdown "
> - "-chardev file,id=serial0,path=%s "
> - "-serial chardev:serial0 -accel tcg -accel kvm %s",
> - codeparam,
> - test->kernel ? "-kernel " : test->bios ? "-bios " : "",
> - codetmp ? : "", test->machine,
> - serialtmp, test->extra);
> + g_string_append(cmd, "-accel tcg ");
> + g_string_append(cmd, "-accel kvm ");
> + g_string_append(cmd, test->extra);
> +
> + qts = qtest_init(cmd->str);
... and I have to say that this is already quite a lot of code churn, just
to get the -accel parameters tweaked in the end.
Why don't you simply do a single small patch that just replaces the "-accel
tcg -accel" part with "%s %s" and add two parameters like this:
has_tcg ? "-accel tcg" : "", has_kvm ? "-accel kvm" : ""
?
Thomas
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope
2023-01-19 14:58 ` [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope Philippe Mathieu-Daudé
2023-01-19 21:08 ` Richard Henderson
@ 2023-01-20 9:02 ` Thomas Huth
2023-01-20 10:58 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2023-01-20 9:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas
On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> tests/qtest/migration-test.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
Missing explanation in the commit description. What's the benefit of doing this?
Thomas
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope
2023-01-20 9:02 ` Thomas Huth
@ 2023-01-20 10:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-20 10:58 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas
On 20/1/23 10:02, Thomas Huth wrote:
> On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/qtest/migration-test.c | 29 +++++++++++++++--------------
>> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> Missing explanation in the commit description. What's the benefit of
> doing this?
Reduce the changes in the next patches to ease David's review.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API
2023-01-20 8:59 ` Thomas Huth
@ 2023-01-20 11:04 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-20 11:04 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Juan Quintela, Laurent Vivier, Paolo Bonzini,
Dr. David Alan Gilbert, qemu-arm, Fabiano Rosas
On 20/1/23 09:59, Thomas Huth wrote:
> On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> tests/qtest/boot-serial-test.c | 23 +++++++++++++++--------
>> 1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/qtest/boot-serial-test.c
>> b/tests/qtest/boot-serial-test.c
>> index 3a854b0174..92890b409d 100644
>> --- a/tests/qtest/boot-serial-test.c
>> +++ b/tests/qtest/boot-serial-test.c
>> @@ -226,14 +226,17 @@ static void test_machine(const void *data)
>> const testdef_t *test = data;
>> g_autofree char *serialtmp = NULL;
>> g_autofree char *codetmp = NULL;
>> - const char *codeparam = "";
>> QTestState *qts;
>> int ser_fd;
>> + g_autoptr(GString) cmd = g_string_new("");
>
> You could already start with the "-no-shutdown" here.
It is just the matter of knowing the style of the maintainer :)
IIRC Alex prefers starting with an empty "" to avoid future churn
when adding new params (just add new line instead of modify).
I'll use your suggestion if I respin.
>
>> ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX",
>> &serialtmp, NULL);
>> g_assert(ser_fd != -1);
>> close(ser_fd);
>> + g_string_append_printf(cmd, "-M %s ", test->machine);
>> + g_string_append(cmd, "-no-shutdown ");
>> +
>> if (test->kernel || test->bios) {
>> ssize_t wlen;
>> int code_fd;
>> @@ -243,19 +246,23 @@ static void test_machine(const void *data)
>> wlen = write(code_fd, test->kernel ? : test->bios,
>> test->codesize);
>> g_assert(wlen == test->codesize);
>> close(code_fd);
>> + g_string_append_printf(cmd, "%s %s ",
>> + test->kernel ? "-kernel " : "-bios ",
>> codetmp);
>> }
>> + g_string_append_printf(cmd, "-chardev file,id=serial0,path=%s "
>> + "-serial chardev:serial0 ", serialtmp);
>
> Why not include this in the initial g_string_append_printf statement
> already?
>
>> /*
>> * Make sure that this test uses tcg if available: It is used as a
>> * fast-enough smoketest for that.
>> */
>> - qts = qtest_initf("%s %s %s -M %s -no-shutdown "
>> - "-chardev file,id=serial0,path=%s "
>> - "-serial chardev:serial0 -accel tcg -accel kvm
>> %s",
>> - codeparam,
>> - test->kernel ? "-kernel " : test->bios ? "-bios
>> " : "",
>> - codetmp ? : "", test->machine,
>> - serialtmp, test->extra);
>> + g_string_append(cmd, "-accel tcg ");
>> + g_string_append(cmd, "-accel kvm ");
>> + g_string_append(cmd, test->extra);
>> +
>> + qts = qtest_init(cmd->str);
>
> ... and I have to say that this is already quite a lot of code churn,
> just to get the -accel parameters tweaked in the end.
>
> Why don't you simply do a single small patch that just replaces the
> "-accel tcg -accel" part with "%s %s" and add two parameters like this:
>
> has_tcg ? "-accel tcg" : "", has_kvm ? "-accel kvm" : ""
I thought it would be simpler when adding HVF support later, KISS,
but I don't mind to change.
Thanks,
Phil.
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-01-20 11:05 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-19 14:58 [PATCH v2 00/11] tests/qtest: Allow running boot-serial / migration with TCG disabled Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 01/11] tests/qtest/boot-serial-test: Constify tests[] array Philippe Mathieu-Daudé
2023-01-19 20:54 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit Philippe Mathieu-Daudé
2023-01-19 20:59 ` Richard Henderson
2023-01-20 8:54 ` Thomas Huth
2023-01-19 14:58 ` [PATCH v2 03/11] tests/qtest/boot-serial-test: Build command line using GString API Philippe Mathieu-Daudé
2023-01-19 21:03 ` Richard Henderson
2023-01-20 8:59 ` Thomas Huth
2023-01-20 11:04 ` Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 04/11] tests/qtest/boot-serial-test: Only use available accelerators Philippe Mathieu-Daudé
2023-01-19 15:18 ` Fabiano Rosas
2023-01-19 21:05 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 05/11] tests/qtest/migration-test: Inverse #ifdef'ry ladders Philippe Mathieu-Daudé
2023-01-19 21:06 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 06/11] tests/qtest/migration-test: Reduce 'cmd_source' string scope Philippe Mathieu-Daudé
2023-01-19 21:08 ` Richard Henderson
2023-01-20 9:02 ` Thomas Huth
2023-01-20 10:58 ` Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 07/11] tests/qtest/migration-test: Build command line using GString API (1/4) Philippe Mathieu-Daudé
2023-01-19 21:10 ` Richard Henderson
2023-01-20 7:32 ` Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 08/11] tests/qtest/migration-test: Build command line using GString API (2/4) Philippe Mathieu-Daudé
2023-01-19 21:13 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 09/11] tests/qtest/migration-test: Build command line using GString API (3/4) Philippe Mathieu-Daudé
2023-01-19 21:17 ` Richard Henderson
2023-01-20 7:05 ` Philippe Mathieu-Daudé
2023-01-19 14:58 ` [PATCH v2 10/11] tests/qtest/migration-test: Build command line using GString API (4/4) Philippe Mathieu-Daudé
2023-01-19 21:19 ` Richard Henderson
2023-01-19 14:58 ` [PATCH v2 11/11] tests/qtest/migration-test: Only use available accelerators Philippe Mathieu-Daudé
2023-01-19 21:19 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.