vl.c | 2 +- tests/qemu-iotests/check | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
Commit 6f6e1698a68ceb made these options incompatible, but it breaks
iotests:
-accel qtest comes from QEMUQtestMachine
and
-machine accel=qtest comes from QEMU_OPTIONS
After this patch, -accel may still be duplicated, but this is less
invasive.
Also, fix extra comma in comment, added by the same 6f6e1698a68ceb
Fixes: 6f6e1698a68ceb
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
vl.c | 2 +-
tests/qemu-iotests/check | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 86474a55c9..9fb859969c 100644
--- a/vl.c
+++ b/vl.c
@@ -2779,7 +2779,7 @@ static void configure_accelerators(const char *progname)
for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
/*
* Filter invalid accelerators here, to prevent obscenities
- * such as "-machine accel=tcg,,thread=single".
+ * such as "-machine accel=tcg,thread=single".
*/
if (accel_find(*tmp)) {
qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 90970b0549..2890785a10 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -587,13 +587,13 @@ export QEMU_PROG="$(type -p "$QEMU_PROG")"
case "$QEMU_PROG" in
*qemu-system-arm|*qemu-system-aarch64)
- export QEMU_OPTIONS="-nodefaults -display none -machine virt,accel=qtest"
+ export QEMU_OPTIONS="-nodefaults -display none -machine virt -accel qtest"
;;
*qemu-system-tricore)
- export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard,accel=qtest"
+ export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard -accel qtest"
;;
*)
- export QEMU_OPTIONS="-nodefaults -display none -machine accel=qtest"
+ export QEMU_OPTIONS="-nodefaults -display none -accel qtest"
;;
esac
--
2.21.0
On 23/12/19 08:43, Vladimir Sementsov-Ogievskiy wrote: > diff --git a/vl.c b/vl.c > index 86474a55c9..9fb859969c 100644 > --- a/vl.c > +++ b/vl.c > @@ -2779,7 +2779,7 @@ static void configure_accelerators(const char *progname) > for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) { > /* > * Filter invalid accelerators here, to prevent obscenities > - * such as "-machine accel=tcg,,thread=single". > + * such as "-machine accel=tcg,thread=single". The double comma is intentional. Without the "if" below, the comma would be escaped and parsed as "-accel tcg,thread=single". > */ > if (accel_find(*tmp)) { > qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true); > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 90970b0549..2890785a10 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check > @@ -587,13 +587,13 @@ export QEMU_PROG="$(type -p "$QEMU_PROG")" > > case "$QEMU_PROG" in > *qemu-system-arm|*qemu-system-aarch64) > - export QEMU_OPTIONS="-nodefaults -display none -machine virt,accel=qtest" > + export QEMU_OPTIONS="-nodefaults -display none -machine virt -accel qtest" > ;; > *qemu-system-tricore) > - export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard,accel=qtest" > + export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard -accel qtest" > ;; > *) > - export QEMU_OPTIONS="-nodefaults -display none -machine accel=qtest" > + export QEMU_OPTIONS="-nodefaults -display none -accel qtest" > ;; > esac > > This part is good, but what is the reproducer? Paolo
23.12.2019 11:39, Paolo Bonzini wrote: > On 23/12/19 08:43, Vladimir Sementsov-Ogievskiy wrote: >> diff --git a/vl.c b/vl.c >> index 86474a55c9..9fb859969c 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2779,7 +2779,7 @@ static void configure_accelerators(const char *progname) >> for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) { >> /* >> * Filter invalid accelerators here, to prevent obscenities >> - * such as "-machine accel=tcg,,thread=single". >> + * such as "-machine accel=tcg,thread=single". > > The double comma is intentional. Without the "if" below, the comma > would be escaped and parsed as "-accel tcg,thread=single". Ah, OK, than drop this chunk. > >> */ >> if (accel_find(*tmp)) { >> qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true); >> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >> index 90970b0549..2890785a10 100755 >> --- a/tests/qemu-iotests/check >> +++ b/tests/qemu-iotests/check >> @@ -587,13 +587,13 @@ export QEMU_PROG="$(type -p "$QEMU_PROG")" >> >> case "$QEMU_PROG" in >> *qemu-system-arm|*qemu-system-aarch64) >> - export QEMU_OPTIONS="-nodefaults -display none -machine virt,accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -machine virt -accel qtest" >> ;; >> *qemu-system-tricore) >> - export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard,accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard -accel qtest" >> ;; >> *) >> - export QEMU_OPTIONS="-nodefaults -display none -machine accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -accel qtest" >> ;; >> esac >> >> > > This part is good, but what is the reproducer? > For example, 30 iotest fails for me with a lot of +====================================================================== +ERROR: test_stream (__main__.TestSmallerBackingFile) +---------------------------------------------------------------------- +Traceback (most recent call last): + File "030", line 592, in setUp + self.vm.launch() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/machine.py", line 302, in launch + self._launch() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/machine.py", line 329, in _launch + self._post_launch() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/qtest.py", line 110, in _post_launch + super(QEMUQtestMachine, self)._post_launch() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/machine.py", line 274, in _post_launch + self._qmp.accept() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/qmp.py", line 157, in accept + return self.__negotiate_capabilities() + File "/work/src/qemu/master/tests/qemu-iotests/../../python/qemu/qmp.py", line 73, in __negotiate_capabilities + raise QMPConnectError +qemu.qmp.QMPConnectError and if I add -d, I see +DEBUG:qemu.machine:Error launching VM +DEBUG:qemu.machine:Command: '/work/src/qemu/master/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/tmp/tmp.ANcscZnFct/qemu-23220-monitor.sock -mon chardev=mon,mode=control -qtest unix:path=/tmp/tmp.ANcscZnFct/qemu-23220-qtest.sock -accel qtest -nodefaults -display none -machine accel=qtest -drive if=virtio,id=drive0,file=blkdebug::/ramdisk/x3/test.img,format=qcow2,cache=writeback,backing.node-name=mid,backing.backing.node-name=base' +DEBUG:qemu.machine:Output: 'qemu-system-x86_64: The -accel and "-machine accel=" options are incompatible\n' Try just random three python tests: 132 202 203: all fails with same symptoms. (interesting that -d doesn't help for 202 and 203, but still they are fixed after this patch). I expect that all python tests are broken.. Still I'm lazy to check all, let's just fix. -- Best regards, Vladimir
On 23/12/2019 09.39, Paolo Bonzini wrote: > On 23/12/19 08:43, Vladimir Sementsov-Ogievskiy wrote: >> diff --git a/vl.c b/vl.c >> index 86474a55c9..9fb859969c 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2779,7 +2779,7 @@ static void configure_accelerators(const char *progname) >> for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) { >> /* >> * Filter invalid accelerators here, to prevent obscenities >> - * such as "-machine accel=tcg,,thread=single". >> + * such as "-machine accel=tcg,thread=single". > > The double comma is intentional. Without the "if" below, the comma > would be escaped and parsed as "-accel tcg,thread=single". > >> */ >> if (accel_find(*tmp)) { >> qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true); >> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check >> index 90970b0549..2890785a10 100755 >> --- a/tests/qemu-iotests/check >> +++ b/tests/qemu-iotests/check >> @@ -587,13 +587,13 @@ export QEMU_PROG="$(type -p "$QEMU_PROG")" >> >> case "$QEMU_PROG" in >> *qemu-system-arm|*qemu-system-aarch64) >> - export QEMU_OPTIONS="-nodefaults -display none -machine virt,accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -machine virt -accel qtest" >> ;; >> *qemu-system-tricore) >> - export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard,accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -machine tricore_testboard -accel qtest" >> ;; >> *) >> - export QEMU_OPTIONS="-nodefaults -display none -machine accel=qtest" >> + export QEMU_OPTIONS="-nodefaults -display none -accel qtest" >> ;; >> esac >> >> > > This part is good, but what is the reproducer? Make the iotests run with either qemu-system-arm, qemu-system-aarch64 or qemu-system-tricore, e.g.: QEMU_PROG=aarch64-softmmu/qemu-system-aarch64 make check-block HTH, Thomas
© 2016 - 2024 Red Hat, Inc.