Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

[Jianjun Duan <duanj@linux.vnet.ibm.com>]

On 10/12/2016 05:07 AM, Paolo Bonzini wrote:
> 
> 
> On 12/10/2016 13:59, Halil Pasic wrote:
>> IMHO this would:
>> * allow us to keep the good old MVStateInfo objects unmodified and
>>   the semantic of VMStateInfo unchanged
>> * make clear that VMStateLinked does not care about the calculated size
>>   and provide a new anchor for documentation
>> * if overloading the semantic of VMStateField.start is not desired we
>>   could put it into  VMStateLinked, if needed we could also put more
>>   stuff in there
>> * have clearer separation between special handling for (linked/certain)
>>   data structures and the usual scenario with the DAG.
> 
> No, I disagree.  We shouldn't be worried about making intrusive changes
> to all invocations or declarations, if that leads to a simpler API.
> 
If VMStateInfo was meant for complete customization, then it was missing
some parts. I think the API is indeed simpler. Just like
definition for VMStateField. Not all of its fields are used all time.

> I agree that overloading .start can be a bit ugly but you can choose to
> overload .num_offset instead, which is already better.
>
I did considered num_offset. But it is associated with an actual field.
On the other hand, start means the start position of the q in the
structure. So it is not that far stretched.

>> I would also suggest unit tests in test/test-vmstate.c. Maybe with
>> that the vmstate migration of QTAILQ could be factored out as a separate
>> patch series.
> 
> Yes, definitely.
> 
This sounds a good idea. Will do it.

> Paolo
> 
Thanks,
Jianjun