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

[Anthony Liguori <anthony@codemonkey.ws>]

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