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 02:07 PM, 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.
> 
> 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?);
so the extra parameters aren't going to be misused too badly.

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

Dave

> No strong opinion here, it's just that I don't understand. I think one
> trait of a simple API is that it is easy to document. Unfortunately
> the documentation is quite sparse and in the patch the signature
> change goes undocumented.
> 
> Well as I said, just an idea, motivated by how I understood he role of
> VMStateInfo form reading the code.
> 
> Cheers,
> Halil
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK