[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests c
From: |
Halil Pasic |
Subject: |
Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91 |
Date: |
Tue, 7 Mar 2017 11:11:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 03/07/2017 11:03 AM, Dr. David Alan Gilbert wrote:
> * Halil Pasic (address@hidden) wrote:
>>
>>
>> On 03/07/2017 10:29 AM, Kevin Wolf wrote:
>>> Am 07.03.2017 um 03:53 hat QingFeng Hao geschrieben:
>>>> I am not very clear about the logic in vmstate.c, but from its context in
>>>> vmstate_save_state, it seems size should not be 0, otherwise the followed
>>>> for loop will keep working on the same element. So I just add a simple
>>>> check to pass that case, not sure if it's right but it can pass iotest
>>>> case 68 and 91 now.
[..]
>>
>> I have looked onto the issue. It affects s390x only if we
>> are running without KVM. Basically, S390CPU.irqstate is unused
>> if we do not use KVM, and thus no buffer is allocated.
>>
>> IMHO this is a missing field and the cleaner way to handle such
>> missing fields is exist. However this used to work, and I recommended
>> QuiFeng Hao to discuss the problem upstream.
>>
>> By the way, I think, if we want to go back to the old behavior
>> and support VMS_VBUFFER with size 0 and nullptr, a much
>> cleaner way to do the fix is to change the assert to:
>>
>> assert(first_elem || !n_elems || !size)
>>
>> Obviously the other clean way to fix is to implement exists.
>>
>> I think the migration maintainers (Juan and Dave) should make a
>> call regarding if the described usage of VMS_BUFFER is a legal
>> or an illegal one.
>
> So is it this code in target/s390x/machine.c ?
>
> VMSTATE_VBUFFER_UINT32(irqstate, S390CPU, 4, NULL,
> irqstate_saved_size),
>
Yes!
> it should be legal for that to be zero length.
> It also makes sense that if that's zero length it should be OK for
> the pointer to be NULL.
>
> I think it's best if you do change that assert.
>
Makes sense. I think QingFeng will agree too and send a
new version soon.
Regards,
Halil
> Dave
- [Qemu-devel] [PATCH RFC 0/1] vmstate: fix the failed iotests case 68 and 91, QingFeng Hao, 2017/03/06
- [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, QingFeng Hao, 2017/03/06
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, Kevin Wolf, 2017/03/07
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, Halil Pasic, 2017/03/07
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, QingFeng Hao, 2017/03/08
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, Halil Pasic, 2017/03/08
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, QingFeng Hao, 2017/03/08
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, Halil Pasic, 2017/03/09
- Re: [Qemu-devel] [PATCH RFC 1/1] vmstate: draft fix for failed iotests case 68 and 91, QingFeng Hao, 2017/03/09
Re: [Qemu-devel] [PATCH RFC 0/1] vmstate: fix the failed iotests case 68 and 91, Fam Zheng, 2017/03/07