[Qemu-devel] Re: [PATCH 27/49] ac97: add active to the state

[malc]

On Wed, 30 Sep 2009, Juan Quintela wrote:

> malc <address@hidden> wrote:
> > On Wed, 30 Sep 2009, Juan Quintela wrote:
> >
> >> malc <address@hidden> wrote:
> >> > On Wed, 30 Sep 2009, Juan Quintela wrote:
> >> 
> >> > active doesn't belong in the above structure, it's not used for anything
> >> > other than save/loadvm.
> >> 
> >> It is used for reset, for instance.  We can discuss if it belongs there
> >> or not, but will let that for another day.
> >> 
> >> > If this "design" doesn't allow this, either find
> >> > another way to accomplish the same or fix the "design".
> >> 
> >> VMStateDescription is a list of offsets from a known address.  Address
> >> is the one from the state.  That is the design.
> >> 
> >> Now, back to ac97.  Rest of ac97 don't need the active field, because it
> >> is stored anywhere else (basically where AUD_set_active* store it).
> >> 
> >> Clearly it is part of ac97 state, becase it is needed for load/save/reset 
> >> to
> >> work properly.  It just happens that it is stored anywhere else for the
> >> design of the audio system.
> >> 
> >> At load/save time we don't want to call malloc(), and now we have
> >> descriptions of the state, not functions that save/load the state.
> >> Only sane way of storing this kind of variables is storing them
> >> into the state.
> >> 
> >> Sorry, there is no way that we are going to have a declarative
> >> description of the state and retain the old functions to do save/load.
> >> You can only get only declarative description or functions, not both.
> >
> > I don't want to have declarative description to begin with, i was and
> > sitll am perfectly happy with how it was done before, the monstricity
> > of the new system is frightening and things are kept being added all
> > the time.
> 
> I can't answer to that.
> 
> >> 
> >> There are more temporary variables in other devices, and this was the
> >> way it was done there.  Normally I would have called it active_vmstate
> >> to make that explicit, but here, it was also used for reset, that is the
> >> way I named it with the _vmstate suffix.
> >
> > In any case the while i can understand the fear of malloc, nobody forces
> > you to do that you can have a scratch space on the stack, with an some
> > upper cap, that's the way it's done now anyhow, only the cap is stack size
> > reserved for the process and not something you have to choose.
> 
> You call new way a monstrosity, and now propose a solution that makes
> the monster bigger?
> 
> I still stand behind the patch, and I still want it applied.
> 

And it wont be, not this part of it, not in this state.

-- 
mailto:address@hidden