* [Qemu-devel] [PULL 0/2] Block layer patches @ 2018-12-03 16:58 Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 1/2] mirror: fix dead-lock Kevin Wolf ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Kevin Wolf @ 2018-12-03 16:58 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel The following changes since commit 83ea23cd207a03c5736be0231acbf7f8b05dbf52: i386: hvf: Fix overrun of _decode_tbl1 (2018-12-03 15:09:55 +0000) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to db5e8210adbafe9c6383d8364345a8c545d38e62: iotests: simple mirror test with kvm on 1G image (2018-12-03 16:51:53 +0100) ---------------------------------------------------------------- Block layer patches: - mirror: Fix deadlock ---------------------------------------------------------------- Vladimir Sementsov-Ogievskiy (2): mirror: fix dead-lock iotests: simple mirror test with kvm on 1G image block/mirror.c | 13 +++----- tests/qemu-iotests/235 | 76 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/235.out | 3 ++ tests/qemu-iotests/group | 1 + 4 files changed, 85 insertions(+), 8 deletions(-) create mode 100755 tests/qemu-iotests/235 create mode 100644 tests/qemu-iotests/235.out ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 1/2] mirror: fix dead-lock 2018-12-03 16:58 [Qemu-devel] [PULL 0/2] Block layer patches Kevin Wolf @ 2018-12-03 16:58 ` Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 2/2] iotests: simple mirror test with kvm on 1G image Kevin Wolf 2018-12-03 17:43 ` [Qemu-devel] [PULL 0/2] Block layer patches Peter Maydell 2 siblings, 0 replies; 15+ messages in thread From: Kevin Wolf @ 2018-12-03 16:58 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Let start from the beginning: Commit b9e413dd375 (in 2.9) "block: explicitly acquire aiocontext in aio callbacks that need it" added pairs of aio_context_acquire/release to mirror_write_complete and mirror_read_complete, when they were aio callbacks for blk_aio_* calls. Then, commit 2e1990b26e5 (in 3.0) "block/mirror: Convert to coroutines" dropped these blk_aio_* calls, than mirror_write_complete and mirror_read_complete are not callbacks more, and don't need additional aiocontext acquiring. Furthermore, mirror_read_complete calls blk_co_pwritev inside these pair of aio_context_acquire/release, which leads to the following dead-lock with mirror: (gdb) info thr Id Target Id Frame 3 Thread (LWP 145412) "qemu-system-x86" syscall () 2 Thread (LWP 145416) "qemu-system-x86" __lll_lock_wait () * 1 Thread (LWP 145411) "qemu-system-x86" __lll_lock_wait () (gdb) bt #0 __lll_lock_wait () #1 _L_lock_812 () #2 __GI___pthread_mutex_lock #3 qemu_mutex_lock_impl (mutex=0x561032dce420 <qemu_global_mutex>, file=0x5610327d8654 "util/main-loop.c", line=236) at util/qemu-thread-posix.c:66 #4 qemu_mutex_lock_iothread_impl #5 os_host_main_loop_wait (timeout=480116000) at util/main-loop.c:236 #6 main_loop_wait (nonblocking=0) at util/main-loop.c:497 #7 main_loop () at vl.c:1892 #8 main Printing contents of qemu_global_mutex, I see that "__owner = 145416", so, thr1 is main loop, and now it wants BQL, which is owned by thr2. (gdb) thr 2 (gdb) bt #0 __lll_lock_wait () #1 _L_lock_870 () #2 __GI___pthread_mutex_lock #3 qemu_mutex_lock_impl (mutex=0x561034d25dc0, ... #4 aio_context_acquire (ctx=0x561034d25d60) #5 dma_blk_cb #6 dma_blk_io #7 dma_blk_read #8 ide_dma_cb #9 bmdma_cmd_writeb #10 bmdma_write #11 memory_region_write_accessor #12 access_with_adjusted_size #15 flatview_write #16 address_space_write #17 address_space_rw #18 kvm_handle_io #19 kvm_cpu_exec #20 qemu_kvm_cpu_thread_fn #21 qemu_thread_start #22 start_thread #23 clone () Printing mutex in fr 2, I see "__owner = 145411", so thr2 wants aio context mutex, which is owned by thr1. Classic dead-lock. Then, let's check that aio context is hold by mirror coroutine: just print coroutine stack of first tracked request in mirror job target: (gdb) [...] (gdb) qemu coroutine 0x561035dd0860 #0 qemu_coroutine_switch #1 qemu_coroutine_yield #2 qemu_co_mutex_lock_slowpath #3 qemu_co_mutex_lock #4 qcow2_co_pwritev #5 bdrv_driver_pwritev #6 bdrv_aligned_pwritev #7 bdrv_co_pwritev #8 blk_co_pwritev #9 mirror_read_complete () at block/mirror.c:232 #10 mirror_co_read () at block/mirror.c:370 #11 coroutine_trampoline #12 __start_context Yes it is mirror_read_complete calling blk_co_pwritev after acquiring aio context. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/mirror.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 56d9ef7474..8f52c6215d 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -199,7 +199,6 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) { MirrorBlockJob *s = op->s; - aio_context_acquire(blk_get_aio_context(s->common.blk)); if (ret < 0) { BlockErrorAction action; @@ -209,15 +208,14 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret) s->ret = ret; } } + mirror_iteration_done(op, ret); - aio_context_release(blk_get_aio_context(s->common.blk)); } static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) { MirrorBlockJob *s = op->s; - aio_context_acquire(blk_get_aio_context(s->common.blk)); if (ret < 0) { BlockErrorAction action; @@ -228,12 +226,11 @@ static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret) } mirror_iteration_done(op, ret); - } else { - ret = blk_co_pwritev(s->target, op->offset, - op->qiov.size, &op->qiov, 0); - mirror_write_complete(op, ret); + return; } - aio_context_release(blk_get_aio_context(s->common.blk)); + + ret = blk_co_pwritev(s->target, op->offset, op->qiov.size, &op->qiov, 0); + mirror_write_complete(op, ret); } /* Clip bytes relative to offset to not exceed end-of-file */ -- 2.19.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-03 16:58 [Qemu-devel] [PULL 0/2] Block layer patches Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 1/2] mirror: fix dead-lock Kevin Wolf @ 2018-12-03 16:58 ` Kevin Wolf [not found] ` <dc86835f-65d9-cada-dc9b-1f2c708521f9@de.ibm.com> 2018-12-03 17:43 ` [Qemu-devel] [PULL 0/2] Block layer patches Peter Maydell 2 siblings, 1 reply; 15+ messages in thread From: Kevin Wolf @ 2018-12-03 16:58 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> This test is broken without previous commit fixing dead-lock in mirror. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Acked-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/235 | 76 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/235.out | 3 ++ tests/qemu-iotests/group | 1 + 3 files changed, 80 insertions(+) create mode 100755 tests/qemu-iotests/235 create mode 100644 tests/qemu-iotests/235.out diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 new file mode 100755 index 0000000000..da044ed34e --- /dev/null +++ b/tests/qemu-iotests/235 @@ -0,0 +1,76 @@ +#!/usr/bin/env python +# +# Simple mirror test +# +# Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +import sys +import os +import iotests +from iotests import qemu_img_create, qemu_io, file_path, log + +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts')) + +from qemu import QEMUMachine + +# Note: +# This test was added to check that mirror dead-lock was fixed (see previous +# commit before this test addition). +# And it didn't reproduce if at least one of the following: +# 1. use small image size +# 2. use raw format (not qcow2) +# 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still +# reproduces, if just drop kvm, but gdb failed to produce full backtraces +# for me) +# 4. add iothread + +size = 1 * 1024 * 1024 * 1024 + +iotests.verify_image_format(supported_fmts=['qcow2']) + +disk = file_path('disk') + +# prepare source image +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, + str(size)) + +vm = QEMUMachine(iotests.qemu_prog) +vm.add_args('-machine', 'pc,accel=kvm') +vm.add_args('-drive', 'id=src,file=' + disk) +vm.launch() + +log(vm.qmp('object-add', qom_type='throttle-group', id='tg0', + props={ 'x-bps-total': size })) + +log(vm.qmp('blockdev-add', + **{ 'node-name': 'target', + 'driver': 'throttle', + 'throttle-group': 'tg0', + 'file': { + 'driver': 'null-co', + 'size': size + } })) + +log(vm.qmp('blockdev-mirror', device='src', target='target', sync='full')) + +try: + vm.event_wait('BLOCK_JOB_READY', timeout=10.0) +except: + vm.shutdown() + raise + +vm.shutdown() diff --git a/tests/qemu-iotests/235.out b/tests/qemu-iotests/235.out new file mode 100644 index 0000000000..39db621e04 --- /dev/null +++ b/tests/qemu-iotests/235.out @@ -0,0 +1,3 @@ +{"return": {}} +{"return": {}} +{"return": {}} diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 8c56a0ad11..61a6d98ebd 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -232,3 +232,4 @@ 232 auto quick 233 auto quick 234 auto quick migration +235 auto quick -- 2.19.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <dc86835f-65d9-cada-dc9b-1f2c708521f9@de.ibm.com>]
[parent not found: <75f7e3cc-bd46-c743-84ab-cd68bcb1dcfb@de.ibm.com>]
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image [not found] ` <75f7e3cc-bd46-c743-84ab-cd68bcb1dcfb@de.ibm.com> @ 2018-12-05 8:23 ` Christian Borntraeger 2018-12-05 8:46 ` Kevin Wolf 0 siblings, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2018-12-05 8:23 UTC (permalink / raw) To: Kevin Wolf, qemu-block Cc: peter.maydell, qemu-s390x, Vladimir Sementsov-Ogievskiy, Max Reitz, qemu-devel On 04.12.2018 14:49, Christian Borntraeger wrote: > > > On 04.12.2018 14:46, Christian Borntraeger wrote: >> FWIW, this testcase fails with current qemu master on s390: >> >> QEMU -- "/home/cborntra/REPOS/qemu/build/tests/qemu-iotests/../../s390x-softmmu/qemu-system-s390x" -nodefaults -machine accel=qtest >> QEMU_IMG -- "/home/cborntra/REPOS/qemu/build/tests/qemu-iotests/../../qemu-img" >> QEMU_IO -- "/home/cborntra/REPOS/qemu/build/tests/qemu-iotests/../../qemu-io" --cache writeback -f qcow2 >> QEMU_NBD -- "/home/cborntra/REPOS/qemu/build/tests/qemu-iotests/../../qemu-nbd" >> IMGFMT -- qcow2 (compat=1.1) >> IMGPROTO -- file >> PLATFORM -- Linux/s390x s38lp08 4.19.0+ >> TEST_DIR -- /home/cborntra/REPOS/qemu/build/tests/qemu-iotests/scratch >> SOCKET_SCM_HELPER -- /home/cborntra/REPOS/qemu/build/tests/qemu-iotests/socket_scm_helper >> 235 >> [failed, exit status 1] - output mismatch (see 235.out.bad) >> --- /home/cborntra/REPOS/qemu/tests/qemu-iotests/235.out 2018-12-04 14:44:27.913714608 +0100 >> +++ /home/cborntra/REPOS/qemu/build/tests/qemu-iotests/235.out.bad 2018-12-04 14:44:51.512958864 +0100 >> @@ -1,3 +1,14 @@ >> -{"return": {}} >> -{"return": {}} >> -{"return": {}} >> +Traceback (most recent call last): >> + File "235", line 54, in <module> >> + vm.launch() >> + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 295, in launch >> + self._launch() >> + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 321, in _launch >> + self._post_launch() >> + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 266, in _post_launch >> + self._qmp.accept() >> + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 155, in accept >> + self.__sock, _ = self.__sock.accept() >> + File "/usr/lib64/python2.7/socket.py", line 206, in accept >> + sock, addr = self._sock.accept() >> +socket.timeout: timed out >> On 03.12.2018 17:58, Kevin Wolf wrote: >>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> >>> This test is broken without previous commit fixing dead-lock in mirror. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> Acked-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> tests/qemu-iotests/235 | 76 ++++++++++++++++++++++++++++++++++++++ >>> tests/qemu-iotests/235.out | 3 ++ >>> tests/qemu-iotests/group | 1 + >>> 3 files changed, 80 insertions(+) >>> create mode 100755 tests/qemu-iotests/235 >>> create mode 100644 tests/qemu-iotests/235.out >>> >>> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >>> new file mode 100755 >>> index 0000000000..da044ed34e >>> --- /dev/null >>> +++ b/tests/qemu-iotests/235 [...] >>> +# prepare source image >>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>> + str(size)) >>> + >>> +vm = QEMUMachine(iotests.qemu_prog) >>> +vm.add_args('-machine', 'pc,accel=kvm') This (pc) clearly does not work on other architectures. In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) This hack makes it work for me. diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 index da044ed34e..05aa641a74 100755 --- a/tests/qemu-iotests/235 +++ b/tests/qemu-iotests/235 @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, str(size)) vm = QEMUMachine(iotests.qemu_prog) -vm.add_args('-machine', 'pc,accel=kvm') +vm.add_args('-machine', 'accel=kvm') +vm.add_args('-no-shutdown') vm.add_args('-drive', 'id=src,file=' + disk) vm.launch() ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 8:23 ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger @ 2018-12-05 8:46 ` Kevin Wolf 2018-12-05 9:01 ` Christian Borntraeger 0 siblings, 1 reply; 15+ messages in thread From: Kevin Wolf @ 2018-12-05 8:46 UTC (permalink / raw) To: Christian Borntraeger Cc: qemu-block, peter.maydell, qemu-s390x, Vladimir Sementsov-Ogievskiy, Max Reitz, qemu-devel Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: > >>> +# prepare source image > >>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > >>> + str(size)) > >>> + > >>> +vm = QEMUMachine(iotests.qemu_prog) > >>> +vm.add_args('-machine', 'pc,accel=kvm') > > This (pc) clearly does not work on other architectures. > In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) Leaving out pc definitely makes sense, and the bug still reproduces for me without it. I don't understand the -no-shutdown, though. Already for 068, neither the code nor the commit message when it was added explain why this is needed. Can you turn this into a proper patch and add a comment why -no-shutdown is needed? Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 8:46 ` Kevin Wolf @ 2018-12-05 9:01 ` Christian Borntraeger 2018-12-05 12:00 ` Vladimir Sementsov-Ogievskiy 0 siblings, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2018-12-05 9:01 UTC (permalink / raw) To: Kevin Wolf Cc: peter.maydell, Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel, Max Reitz, qemu-s390x On 05.12.2018 09:46, Kevin Wolf wrote: > Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>> +# prepare source image >>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>> + str(size)) >>>>> + >>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>> +vm.add_args('-machine', 'pc,accel=kvm') >> >> This (pc) clearly does not work on other architectures. >> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) > > Leaving out pc definitely makes sense, and the bug still reproduces for > me without it. > > I don't understand the -no-shutdown, though. Already for 068, neither > the code nor the commit message when it was added explain why this is > needed. > > Can you turn this into a proper patch and add a comment why -no-shutdown > is needed? I already sent this patch. The reason is that there is no BIOS in a classical sense on s390x. If no bootable image (external kernel or from disk) is found, the small boot bios loads a disabled wait PSW. The default action for that is then shutdown. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 9:01 ` Christian Borntraeger @ 2018-12-05 12:00 ` Vladimir Sementsov-Ogievskiy 2018-12-05 12:35 ` Christian Borntraeger 0 siblings, 1 reply; 15+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-12-05 12:00 UTC (permalink / raw) To: Christian Borntraeger, Kevin Wolf Cc: peter.maydell, qemu-block, qemu-devel, Max Reitz, qemu-s390x 05.12.2018 12:01, Christian Borntraeger wrote: > > > On 05.12.2018 09:46, Kevin Wolf wrote: >> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>> +# prepare source image >>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>> + str(size)) >>>>>> + >>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>> >>> This (pc) clearly does not work on other architectures. >>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >> >> Leaving out pc definitely makes sense, and the bug still reproduces for >> me without it. >> >> I don't understand the -no-shutdown, though. Already for 068, neither >> the code nor the commit message when it was added explain why this is >> needed. >> >> Can you turn this into a proper patch and add a comment why -no-shutdown >> is needed? > > I already sent this patch. The reason is that there is no BIOS in a classical sense > on s390x. If no bootable image (external kernel or from disk) is found, the small boot > bios loads a disabled wait PSW. The default action for that is then shutdown. > Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? The problem without it for me was that gdb failed to produce full and nice backtrace, but test worked anyway -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 12:00 ` Vladimir Sementsov-Ogievskiy @ 2018-12-05 12:35 ` Christian Borntraeger 2018-12-05 13:39 ` Vladimir Sementsov-Ogievskiy 0 siblings, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2018-12-05 12:35 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, Kevin Wolf Cc: peter.maydell, qemu-block, qemu-devel, Max Reitz, qemu-s390x On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: > 05.12.2018 12:01, Christian Borntraeger wrote: >> >> >> On 05.12.2018 09:46, Kevin Wolf wrote: >>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>> +# prepare source image >>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>> + str(size)) >>>>>>> + >>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>> >>>> This (pc) clearly does not work on other architectures. >>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>> >>> Leaving out pc definitely makes sense, and the bug still reproduces for >>> me without it. >>> >>> I don't understand the -no-shutdown, though. Already for 068, neither >>> the code nor the commit message when it was added explain why this is >>> needed. >>> >>> Can you turn this into a proper patch and add a comment why -no-shutdown >>> is needed? >> >> I already sent this patch. The reason is that there is no BIOS in a classical sense >> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >> bios loads a disabled wait PSW. The default action for that is then shutdown. >> > > Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? > The problem without it for me was that gdb failed to produce full and nice backtrace, but > test worked anyway In the commid message Vladimir said that kvm is necessary to trigger the problem. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 12:35 ` Christian Borntraeger @ 2018-12-05 13:39 ` Vladimir Sementsov-Ogievskiy 2018-12-05 15:52 ` Christian Borntraeger 0 siblings, 1 reply; 15+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-12-05 13:39 UTC (permalink / raw) To: Christian Borntraeger, Kevin Wolf Cc: peter.maydell, qemu-block, qemu-devel, Max Reitz, qemu-s390x 05.12.2018 15:35, Christian Borntraeger wrote: > > > On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: >> 05.12.2018 12:01, Christian Borntraeger wrote: >>> >>> >>> On 05.12.2018 09:46, Kevin Wolf wrote: >>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>>> +# prepare source image >>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>>> + str(size)) >>>>>>>> + >>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>>> >>>>> This (pc) clearly does not work on other architectures. >>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>>> >>>> Leaving out pc definitely makes sense, and the bug still reproduces for >>>> me without it. >>>> >>>> I don't understand the -no-shutdown, though. Already for 068, neither >>>> the code nor the commit message when it was added explain why this is >>>> needed. >>>> >>>> Can you turn this into a proper patch and add a comment why -no-shutdown >>>> is needed? >>> >>> I already sent this patch. The reason is that there is no BIOS in a classical sense >>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >>> bios loads a disabled wait PSW. The default action for that is then shutdown. >>> >> >> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? >> The problem without it for me was that gdb failed to produce full and nice backtrace, but >> test worked anyway > > In the commid message Vladimir said that kvm is necessary to trigger the problem. > No, I didn't) and it's in the comment: # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still # reproduces, if just drop kvm, but gdb failed to produce full backtraces # for me) -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 13:39 ` Vladimir Sementsov-Ogievskiy @ 2018-12-05 15:52 ` Christian Borntraeger 2018-12-05 16:09 ` Vladimir Sementsov-Ogievskiy 0 siblings, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2018-12-05 15:52 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, Kevin Wolf Cc: peter.maydell, qemu-s390x, qemu-devel, qemu-block, Max Reitz, Eric Blake On 05.12.2018 14:39, Vladimir Sementsov-Ogievskiy wrote: > 05.12.2018 15:35, Christian Borntraeger wrote: >> >> >> On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: >>> 05.12.2018 12:01, Christian Borntraeger wrote: >>>> >>>> >>>> On 05.12.2018 09:46, Kevin Wolf wrote: >>>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>>>> +# prepare source image >>>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>>>> + str(size)) >>>>>>>>> + >>>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>>>> >>>>>> This (pc) clearly does not work on other architectures. >>>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>>>> >>>>> Leaving out pc definitely makes sense, and the bug still reproduces for >>>>> me without it. >>>>> >>>>> I don't understand the -no-shutdown, though. Already for 068, neither >>>>> the code nor the commit message when it was added explain why this is >>>>> needed. >>>>> >>>>> Can you turn this into a proper patch and add a comment why -no-shutdown >>>>> is needed? >>>> >>>> I already sent this patch. The reason is that there is no BIOS in a classical sense >>>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >>>> bios loads a disabled wait PSW. The default action for that is then shutdown. >>>> >>> >>> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? >>> The problem without it for me was that gdb failed to produce full and nice backtrace, but >>> test worked anyway >> >> In the commid message Vladimir said that kvm is necessary to trigger the problem. >> > > No, I didn't) > > and it's in the comment: > # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still > # reproduces, if just drop kvm, but gdb failed to produce full backtraces > # for me) Ok, so I would be fine with completely dropping that line. the patch would then be "-machine pc" will not work all architectures. Lets fall back to the default machine by not specifying anything for the machine. In addition we also need to specify -no-shutdown on s390 as qemu will exit on guest shutdown. This happens when there is no kernel or bootable disk on s390. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- tests/qemu-iotests/235 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 index da044ed34e..329da8f0c2 100755 --- a/tests/qemu-iotests/235 +++ b/tests/qemu-iotests/235 @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, str(size)) vm = QEMUMachine(iotests.qemu_prog) -vm.add_args('-machine', 'pc,accel=kvm') +if iotests.qemu_default_machine == 's390-ccw-virtio': + vm.add_args('-no-shutdown') vm.add_args('-drive', 'id=src,file=' + disk) vm.launch() Shall I resend a v2? ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 15:52 ` Christian Borntraeger @ 2018-12-05 16:09 ` Vladimir Sementsov-Ogievskiy 2018-12-05 16:23 ` Christian Borntraeger 2018-12-06 11:05 ` Christian Borntraeger 0 siblings, 2 replies; 15+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2018-12-05 16:09 UTC (permalink / raw) To: Christian Borntraeger, Kevin Wolf Cc: peter.maydell, qemu-s390x, qemu-devel, qemu-block, Max Reitz, Eric Blake 05.12.2018 18:52, Christian Borntraeger wrote: > > > On 05.12.2018 14:39, Vladimir Sementsov-Ogievskiy wrote: >> 05.12.2018 15:35, Christian Borntraeger wrote: >>> >>> >>> On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: >>>> 05.12.2018 12:01, Christian Borntraeger wrote: >>>>> >>>>> >>>>> On 05.12.2018 09:46, Kevin Wolf wrote: >>>>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>>>>> +# prepare source image >>>>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>>>>> + str(size)) >>>>>>>>>> + >>>>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>>>>> >>>>>>> This (pc) clearly does not work on other architectures. >>>>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>>>>> >>>>>> Leaving out pc definitely makes sense, and the bug still reproduces for >>>>>> me without it. >>>>>> >>>>>> I don't understand the -no-shutdown, though. Already for 068, neither >>>>>> the code nor the commit message when it was added explain why this is >>>>>> needed. >>>>>> >>>>>> Can you turn this into a proper patch and add a comment why -no-shutdown >>>>>> is needed? >>>>> >>>>> I already sent this patch. The reason is that there is no BIOS in a classical sense >>>>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >>>>> bios loads a disabled wait PSW. The default action for that is then shutdown. >>>>> >>>> >>>> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? >>>> The problem without it for me was that gdb failed to produce full and nice backtrace, but >>>> test worked anyway >>> >>> In the commid message Vladimir said that kvm is necessary to trigger the problem. >>> >> >> No, I didn't) >> >> and it's in the comment: >> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >> # for me) > > Ok, so I would be fine with completely dropping that line. > > the patch would then be > > > > "-machine pc" will not work all architectures. Lets fall back to the > default machine by not specifying anything for the machine. > > In addition we also need to specify -no-shutdown on s390 as qemu will > exit on guest shutdown. This happens when there is no kernel or bootable > disk on s390. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > tests/qemu-iotests/235 | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 > index da044ed34e..329da8f0c2 100755 > --- a/tests/qemu-iotests/235 > +++ b/tests/qemu-iotests/235 > @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > str(size)) > > vm = QEMUMachine(iotests.qemu_prog) > -vm.add_args('-machine', 'pc,accel=kvm') > +if iotests.qemu_default_machine == 's390-ccw-virtio': > + vm.add_args('-no-shutdown') > vm.add_args('-drive', 'id=src,file=' + disk) > vm.launch() > > > > Shall I resend a v2? > so, we need -no-shutdown even if we drop kvm? I hoped that not.. Hmm. grep points only to one iotest doing the same about no-shutdown - 068.. -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 16:09 ` Vladimir Sementsov-Ogievskiy @ 2018-12-05 16:23 ` Christian Borntraeger 2018-12-06 11:05 ` Christian Borntraeger 1 sibling, 0 replies; 15+ messages in thread From: Christian Borntraeger @ 2018-12-05 16:23 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, Kevin Wolf Cc: peter.maydell, qemu-block, qemu-devel, Max Reitz, qemu-s390x, Eric Blake On 05.12.2018 17:09, Vladimir Sementsov-Ogievskiy wrote: > 05.12.2018 18:52, Christian Borntraeger wrote: >> >> >> On 05.12.2018 14:39, Vladimir Sementsov-Ogievskiy wrote: >>> 05.12.2018 15:35, Christian Borntraeger wrote: >>>> >>>> >>>> On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: >>>>> 05.12.2018 12:01, Christian Borntraeger wrote: >>>>>> >>>>>> >>>>>> On 05.12.2018 09:46, Kevin Wolf wrote: >>>>>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>>>>>> +# prepare source image >>>>>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>>>>>> + str(size)) >>>>>>>>>>> + >>>>>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>>>>>> >>>>>>>> This (pc) clearly does not work on other architectures. >>>>>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>>>>>> >>>>>>> Leaving out pc definitely makes sense, and the bug still reproduces for >>>>>>> me without it. >>>>>>> >>>>>>> I don't understand the -no-shutdown, though. Already for 068, neither >>>>>>> the code nor the commit message when it was added explain why this is >>>>>>> needed. >>>>>>> >>>>>>> Can you turn this into a proper patch and add a comment why -no-shutdown >>>>>>> is needed? >>>>>> >>>>>> I already sent this patch. The reason is that there is no BIOS in a classical sense >>>>>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >>>>>> bios loads a disabled wait PSW. The default action for that is then shutdown. >>>>>> >>>>> >>>>> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? >>>>> The problem without it for me was that gdb failed to produce full and nice backtrace, but >>>>> test worked anyway >>>> >>>> In the commid message Vladimir said that kvm is necessary to trigger the problem. >>>> >>> >>> No, I didn't) >>> >>> and it's in the comment: >>> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >>> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >>> # for me) >> >> Ok, so I would be fine with completely dropping that line. >> >> the patch would then be >> >> >> >> "-machine pc" will not work all architectures. Lets fall back to the >> default machine by not specifying anything for the machine. >> >> In addition we also need to specify -no-shutdown on s390 as qemu will >> exit on guest shutdown. This happens when there is no kernel or bootable >> disk on s390. >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> tests/qemu-iotests/235 | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >> index da044ed34e..329da8f0c2 100755 >> --- a/tests/qemu-iotests/235 >> +++ b/tests/qemu-iotests/235 >> @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >> str(size)) >> >> vm = QEMUMachine(iotests.qemu_prog) >> -vm.add_args('-machine', 'pc,accel=kvm') >> +if iotests.qemu_default_machine == 's390-ccw-virtio': >> + vm.add_args('-no-shutdown') >> vm.add_args('-drive', 'id=src,file=' + disk) >> vm.launch() >> >> >> >> Shall I resend a v2? >> > > so, we need -no-shutdown even if we drop kvm? I hoped that not.. Hmm. grep points only to one iotest doing the same about no-shutdown - 068.. Yes, without that we fail with --- /home/cborntra/REPOS/qemu/tests/qemu-iotests/235.out 2018-12-04 14:44:27.913714608 +0100 +++ /home/cborntra/REPOS/qemu/build/tests/qemu-iotests/235.out.bad 2018-12-05 17:23:05.601827490 +0100 @@ -1,3 +1,13 @@ {"return": {}} {"return": {}} {"return": {}} +Traceback (most recent call last): + File "235", line 70, in <module> + vm.event_wait('BLOCK_JOB_READY', timeout=10.0) + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 436, in event_wait + event = self._qmp.pull_event(wait=timeout) + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 216, in pull_event + self.__get_events(wait) + File "/home/cborntra/REPOS/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 128, in __get_events + raise QMPConnectError("Error while reading from socket") +qmp.qmp.QMPConnectError: Error while reading from socket Failures: 235 Failed 1 of 1 tests ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-05 16:09 ` Vladimir Sementsov-Ogievskiy 2018-12-05 16:23 ` Christian Borntraeger @ 2018-12-06 11:05 ` Christian Borntraeger 2018-12-07 12:14 ` Kevin Wolf 1 sibling, 1 reply; 15+ messages in thread From: Christian Borntraeger @ 2018-12-06 11:05 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, Kevin Wolf Cc: peter.maydell, qemu-s390x, qemu-devel, qemu-block, Max Reitz, Eric Blake On 05.12.2018 17:09, Vladimir Sementsov-Ogievskiy wrote: > 05.12.2018 18:52, Christian Borntraeger wrote: >> >> >> On 05.12.2018 14:39, Vladimir Sementsov-Ogievskiy wrote: >>> 05.12.2018 15:35, Christian Borntraeger wrote: >>>> >>>> >>>> On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: >>>>> 05.12.2018 12:01, Christian Borntraeger wrote: >>>>>> >>>>>> >>>>>> On 05.12.2018 09:46, Kevin Wolf wrote: >>>>>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: >>>>>>>>>>> +# prepare source image >>>>>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >>>>>>>>>>> + str(size)) >>>>>>>>>>> + >>>>>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) >>>>>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') >>>>>>>> >>>>>>>> This (pc) clearly does not work on other architectures. >>>>>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) >>>>>>> >>>>>>> Leaving out pc definitely makes sense, and the bug still reproduces for >>>>>>> me without it. >>>>>>> >>>>>>> I don't understand the -no-shutdown, though. Already for 068, neither >>>>>>> the code nor the commit message when it was added explain why this is >>>>>>> needed. >>>>>>> >>>>>>> Can you turn this into a proper patch and add a comment why -no-shutdown >>>>>>> is needed? >>>>>> >>>>>> I already sent this patch. The reason is that there is no BIOS in a classical sense >>>>>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot >>>>>> bios loads a disabled wait PSW. The default action for that is then shutdown. >>>>>> >>>>> >>>>> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? >>>>> The problem without it for me was that gdb failed to produce full and nice backtrace, but >>>>> test worked anyway >>>> >>>> In the commid message Vladimir said that kvm is necessary to trigger the problem. >>>> >>> >>> No, I didn't) >>> >>> and it's in the comment: >>> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still >>> # reproduces, if just drop kvm, but gdb failed to produce full backtraces >>> # for me) >> >> Ok, so I would be fine with completely dropping that line. >> >> the patch would then be >> >> >> >> "-machine pc" will not work all architectures. Lets fall back to the >> default machine by not specifying anything for the machine. >> >> In addition we also need to specify -no-shutdown on s390 as qemu will >> exit on guest shutdown. This happens when there is no kernel or bootable >> disk on s390. >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> tests/qemu-iotests/235 | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 >> index da044ed34e..329da8f0c2 100755 >> --- a/tests/qemu-iotests/235 >> +++ b/tests/qemu-iotests/235 >> @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, >> str(size)) >> >> vm = QEMUMachine(iotests.qemu_prog) >> -vm.add_args('-machine', 'pc,accel=kvm') >> +if iotests.qemu_default_machine == 's390-ccw-virtio': >> + vm.add_args('-no-shutdown') >> vm.add_args('-drive', 'id=src,file=' + disk) >> vm.launch() >> >> >> >> Shall I resend a v2? >> > > so, we need -no-shutdown even if we drop kvm? I hoped that not.. Hmm. grep points only to one iotest doing the same about no-shutdown - 068.. Kevin, shall I send the above patch as v2? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image 2018-12-06 11:05 ` Christian Borntraeger @ 2018-12-07 12:14 ` Kevin Wolf 0 siblings, 0 replies; 15+ messages in thread From: Kevin Wolf @ 2018-12-07 12:14 UTC (permalink / raw) To: Christian Borntraeger Cc: Vladimir Sementsov-Ogievskiy, peter.maydell, qemu-s390x, qemu-devel, qemu-block, Max Reitz, Eric Blake Am 06.12.2018 um 12:05 hat Christian Borntraeger geschrieben: > > > On 05.12.2018 17:09, Vladimir Sementsov-Ogievskiy wrote: > > 05.12.2018 18:52, Christian Borntraeger wrote: > >> > >> > >> On 05.12.2018 14:39, Vladimir Sementsov-Ogievskiy wrote: > >>> 05.12.2018 15:35, Christian Borntraeger wrote: > >>>> > >>>> > >>>> On 05.12.2018 13:00, Vladimir Sementsov-Ogievskiy wrote: > >>>>> 05.12.2018 12:01, Christian Borntraeger wrote: > >>>>>> > >>>>>> > >>>>>> On 05.12.2018 09:46, Kevin Wolf wrote: > >>>>>>> Am 05.12.2018 um 09:23 hat Christian Borntraeger geschrieben: > >>>>>>>>>>> +# prepare source image > >>>>>>>>>>> +qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > >>>>>>>>>>> + str(size)) > >>>>>>>>>>> + > >>>>>>>>>>> +vm = QEMUMachine(iotests.qemu_prog) > >>>>>>>>>>> +vm.add_args('-machine', 'pc,accel=kvm') > >>>>>>>> > >>>>>>>> This (pc) clearly does not work on other architectures. > >>>>>>>> In addition to that, I also need to add -no-shutdown on s390 (see 068 for a similar case) > >>>>>>> > >>>>>>> Leaving out pc definitely makes sense, and the bug still reproduces for > >>>>>>> me without it. > >>>>>>> > >>>>>>> I don't understand the -no-shutdown, though. Already for 068, neither > >>>>>>> the code nor the commit message when it was added explain why this is > >>>>>>> needed. > >>>>>>> > >>>>>>> Can you turn this into a proper patch and add a comment why -no-shutdown > >>>>>>> is needed? > >>>>>> > >>>>>> I already sent this patch. The reason is that there is no BIOS in a classical sense > >>>>>> on s390x. If no bootable image (external kernel or from disk) is found, the small boot > >>>>>> bios loads a disabled wait PSW. The default action for that is then shutdown. > >>>>>> > >>>>> > >>>>> Is it an option for you just drop the whole line "vm.add_args('-machine', 'pc,accel=kvm')"? > >>>>> The problem without it for me was that gdb failed to produce full and nice backtrace, but > >>>>> test worked anyway > >>>> > >>>> In the commid message Vladimir said that kvm is necessary to trigger the problem. > >>>> > >>> > >>> No, I didn't) > >>> > >>> and it's in the comment: > >>> # 3. drop kvm and use iotests.VM() (maybe, because of qtest) (however, it still > >>> # reproduces, if just drop kvm, but gdb failed to produce full backtraces > >>> # for me) > >> > >> Ok, so I would be fine with completely dropping that line. > >> > >> the patch would then be > >> > >> > >> > >> "-machine pc" will not work all architectures. Lets fall back to the > >> default machine by not specifying anything for the machine. > >> > >> In addition we also need to specify -no-shutdown on s390 as qemu will > >> exit on guest shutdown. This happens when there is no kernel or bootable > >> disk on s390. > >> > >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > >> --- > >> tests/qemu-iotests/235 | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 > >> index da044ed34e..329da8f0c2 100755 > >> --- a/tests/qemu-iotests/235 > >> +++ b/tests/qemu-iotests/235 > >> @@ -49,7 +49,8 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > >> str(size)) > >> > >> vm = QEMUMachine(iotests.qemu_prog) > >> -vm.add_args('-machine', 'pc,accel=kvm') > >> +if iotests.qemu_default_machine == 's390-ccw-virtio': > >> + vm.add_args('-no-shutdown') > >> vm.add_args('-drive', 'id=src,file=' + disk) > >> vm.launch() > >> > >> > >> > >> Shall I resend a v2? > >> > > > > so, we need -no-shutdown even if we drop kvm? I hoped that not.. > > Hmm. grep points only to one iotest doing the same about no-shutdown > > - 068.. > > Kevin, shall I send the above patch as v2? Don't bother, I think v1 is fine. By specifying kvm explicitly, it's at least clear that we're not using qtest like in other tests. Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 0/2] Block layer patches 2018-12-03 16:58 [Qemu-devel] [PULL 0/2] Block layer patches Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 1/2] mirror: fix dead-lock Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 2/2] iotests: simple mirror test with kvm on 1G image Kevin Wolf @ 2018-12-03 17:43 ` Peter Maydell 2 siblings, 0 replies; 15+ messages in thread From: Peter Maydell @ 2018-12-03 17:43 UTC (permalink / raw) To: Kevin Wolf; +Cc: Qemu-block, QEMU Developers On Mon, 3 Dec 2018 at 16:58, Kevin Wolf <kwolf@redhat.com> wrote: > > The following changes since commit 83ea23cd207a03c5736be0231acbf7f8b05dbf52: > > i386: hvf: Fix overrun of _decode_tbl1 (2018-12-03 15:09:55 +0000) > > are available in the Git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to db5e8210adbafe9c6383d8364345a8c545d38e62: > > iotests: simple mirror test with kvm on 1G image (2018-12-03 16:51:53 +0100) > > ---------------------------------------------------------------- > Block layer patches: > > - mirror: Fix deadlock > Applied, thanks. -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-12-07 12:14 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-03 16:58 [Qemu-devel] [PULL 0/2] Block layer patches Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 1/2] mirror: fix dead-lock Kevin Wolf 2018-12-03 16:58 ` [Qemu-devel] [PULL 2/2] iotests: simple mirror test with kvm on 1G image Kevin Wolf [not found] ` <dc86835f-65d9-cada-dc9b-1f2c708521f9@de.ibm.com> [not found] ` <75f7e3cc-bd46-c743-84ab-cd68bcb1dcfb@de.ibm.com> 2018-12-05 8:23 ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger 2018-12-05 8:46 ` Kevin Wolf 2018-12-05 9:01 ` Christian Borntraeger 2018-12-05 12:00 ` Vladimir Sementsov-Ogievskiy 2018-12-05 12:35 ` Christian Borntraeger 2018-12-05 13:39 ` Vladimir Sementsov-Ogievskiy 2018-12-05 15:52 ` Christian Borntraeger 2018-12-05 16:09 ` Vladimir Sementsov-Ogievskiy 2018-12-05 16:23 ` Christian Borntraeger 2018-12-06 11:05 ` Christian Borntraeger 2018-12-07 12:14 ` Kevin Wolf 2018-12-03 17:43 ` [Qemu-devel] [PULL 0/2] Block layer patches Peter Maydell
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.