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

[Juan Quintela]

Anthony Liguori <address@hidden> wrote:
> 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));
> ...

I don't doubt that you are right.  I don't understand that bit of the
code.  active is created in other way, but I don't know if they are
equivalent or not.


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

Malc?

Later, Juan.