[Top][All Lists]

[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),


> 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.


> Dave

reply via email to

[Prev in Thread] Current Thread [Next in Thread]