Re: [Qemu-devel] [qemu-s390x] [PULL 2/2] iotests: simple mirror test with kvm on 1G image

[Vladimir Sementsov-Ogievskiy]

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 <address@hidden>
> ---
>   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