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

[malc <av1474@comtv.ru>]

On Wed, 30 Sep 2009, Juan Quintela wrote:

> malc <av1474@comtv.ru> 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.

> 
> 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.

-- 
mailto:av1474@comtv.ru