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

[Dr. David Alan Gilbert]

* Halil Pasic (address@hidden) wrote:
> 
> 
> On 10/12/2016 04:59 PM, Dr. David Alan Gilbert wrote:
> >> Paolo I agree on a theoretical level. It's just I do not see why this
> >> > particular change makes the API simpler. In my opinion this complicates
> >> > things because now all VMStateInfo's can do funky stuff. Having 
> >> > additional
> >> > state you can poke is rarely a simplification. Same goes for lots
> >> > of arguments especially if some of them are barely ever used. These
> >> > additional parameters contribute nothing for the large majority
> >> > of the cases (except maybe some head scratching when reading
> >> > the code).
> > I think it might depend how many VMStateInfo's we have.
> > My ideal rule would be there are no .get/.put implementations outside
> > of migration/ and we can trust that they would never do anything silly 
> > (right?);
> 
> Your statement about ideally no .get/.put implementations outside
> of migration/ is consistent with my initial understanding of VMStateInfo:
> It's there to take care of the marshaling between the on wire representation
> and the in memory representation of a single and preferably primitive
> vmstate field (not consisting of further fields). Complex stuff like
> arrays, structs, indirection via pointers and possibly allocation is
> preferably handled elsewhere. So VMStateInfo is the basic building stones,
> and the only place which should write to/read from the stream (in
> ideal vmstate).
> 
> So in a perfect vmstate world complete type of VMStateInfo is not part of the
> API (you do not care about how it's done outside vmstate/), but only the
> (possibly pointers to) VMStateInfo's supported by the vmstate API.
> 
> Of course this is not realistic, at least at the moment.
> 
> On the other hand if VMStateInfo is meant for complete customization,
> as Jianjun has put it, then it obviously has to be a full fledged member
> of the API, and I think then your ideal rule makes no sense to me.
> 
> I also do think we will always need something for handling special
> cases because we need to maintain compatibility -- see virtio migration
> for example.
> 
> > so the extra parameters aren't going to be misused too badly.
> > 
> 
> What would you consider bad misuse? I do not see this as a big concern
> at the moment.

I don't know; but the only justification for needing the VMS_LINKED flag
was that only those functions that were marked with VMS_LINKED would
be passed the new field.

Dave

> Cheers,
> Halil
> 
> > However, we're probably quite a way from pulling all of the weirder
> > .get/.put implementations back in.
> > 
> > Dave
> > 
> 



--
Dr. David Alan Gilbert / address@hidden / Manchester, UK