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

[Anthony Liguori]

malc wrote:
> > > > +
> > > >  typedef struct AC97LinkState {
> > > >      PCIDevice dev;
> > > >      QEMUSoundCard card;
> > > > @@ -162,6 +169,7 @@ typedef struct AC97LinkState {
> > > >      uint8_t silence[128];
> > > >      uint32_t base[2];
> > > >      int bup_flag;
> > > > +    uint8_t active[LAST_INDEX];
> > > >         
> > > This doesn't belong here, cause the only purpose i can see is to hack
> > > around defficiencies of the new load/savevm APIs.
> > >       
> > That was supposed to be one of the features, not deficiences.  You can't
> > sent stuff that it is not in the state. It is "by design" that you can't
> > sent arbitrary variables.
> >     
>
> active doesn't belong in the above structure, it's not used for anything
> other than save/loadvm. If this "design" doesn't allow this, either find
> another way to accomplish the same or fix the "design".
>   

Looking briefly at the code, it looks like the active[] array isn't 
technically needed in the savevm state.

I think you could basically do:

AUD_set_active_in(s->voice_pi, !!(s->bm_regs[PI_INDEX].cr & CR_RPBM));
...

Better yet, active[] can be dynamically built from the contents of 
bm_regs->cr so there would be little code change.

So I think we should bump the version of the ac97 format, remove the 
active[] array from the vmstate, and then generate it in a post function 
that can then be passed to reset_voices().

Regards,

Anthony Liguori