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 >