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