From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mt0nR-0007Qv-T5 for qemu-devel@nongnu.org; Wed, 30 Sep 2009 11:08:49 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mt0nM-0007Ku-TX for qemu-devel@nongnu.org; Wed, 30 Sep 2009 11:08:48 -0400 Received: from [199.232.76.173] (port=59323 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mt0nM-0007KY-EM for qemu-devel@nongnu.org; Wed, 30 Sep 2009 11:08:44 -0400 Received: from mail-bw0-f211.google.com ([209.85.218.211]:57137) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mt0nL-0001kZ-W9 for qemu-devel@nongnu.org; Wed, 30 Sep 2009 11:08:44 -0400 Received: by bwz7 with SMTP id 7so569992bwz.34 for ; Wed, 30 Sep 2009 08:08:43 -0700 (PDT) Message-ID: <4AC37476.60007@codemonkey.ws> Date: Wed, 30 Sep 2009 10:08:38 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH 27/49] ac97: add active to the state References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc Cc: qemu-devel@nongnu.org, Juan Quintela 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