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

[Jianjun Duan]

On 10/13/2016 09:32 AM, Halil Pasic wrote:
> 
> 
> On 10/13/2016 06:23 PM, Jianjun Duan wrote:
>>
>>
>> On 10/13/2016 03:48 AM, Halil Pasic wrote:
>>>
>>>
>>> On 10/13/2016 10:22 AM, Paolo Bonzini wrote:
>>>>>> 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.
>>>> Code moves.  We can decide how much customization to allow of VMStateInfo.
>>>>
>>>> You said it: "Not all of its fields are used all time".  Likewise, not
>>>> all arguments are used all time for get/put, but it's not an issue if they
>>>> are still there!  So this patch is good, but at the same time VMS_LINKED is
>>>> pointless.
>>>>
>>>> Paolo
>>>>
>>>
>>> I'm fine with this. I just think, it would be nice if the contract between
>>> the vmstate-core and the client code implementing VMStateInfo callbacks
>>> could be documented, including when are certain parameters valid, what
>>> they stand for, and how are they supposed to be used for the next version of
>>> the patch. Just to improve readability. Would this be OK with everybody?
>>>
>>> By the way the flag VMS_SINGLE is documented as ignored. Should we drop
>>> it?
>>>
>> I will prepare the VMStateInfo and QTAIL stuff as a separated series. If
>> indeed VMS_SINGLE is ignored, I can drop it here. It is similar to
>> VMS_LINKED situation.
>>
>> Thanks,
>> Jianjun
> 
> I think it's completely unrelated, so I would not lump it together with
> the QTAILQ stuff.
> 
> How do you feel about the apidoc part?
> 
I will add some comments inside vmstate_save/load_state about it.

Thanks,
Jianjun

> Cheers,
> Halil
>