On 03/07/2017 11:03 AM, Dr. David Alan Gilbert wrote: > * Halil Pasic (pa...@linux.vnet.ibm.com) 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
