| Message ID | 20181203165810.14509-3-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,1/2] mirror: fix dead-lock | expand |
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()
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
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.
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
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.
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)
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?
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..
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
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?
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
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