From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mnu8a-0002mE-NU for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:01:32 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mnu8V-0002i0-GE for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:01:31 -0400 Received: from [199.232.76.173] (port=54190 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mnu8V-0002hn-3i for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:01:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37863) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mnu8U-0006vK-Dr for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:01:26 -0400 From: Juan Quintela In-Reply-To: <20090916123710.GB4729@redhat.com> (Michael S. Tsirkin's message of "Wed, 16 Sep 2009 15:37:10 +0300") References: <20090916104620.GA4456@redhat.com> <20090916111845.GJ23157@redhat.com> <20090916115224.GA4628@redhat.com> <20090916121818.GA4710@redhat.com> <20090916123710.GB4729@redhat.com> Date: Wed, 16 Sep 2009 15:01:22 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: optional feature List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Gleb Natapov "Michael S. Tsirkin" wrote: > On Wed, Sep 16, 2009 at 02:26:58PM +0200, Juan Quintela wrote: >> "Michael S. Tsirkin" wrote: >> > On Wed, Sep 16, 2009 at 02:14:32PM +0200, Juan Quintela wrote: >> >> >> See below, we are changing the state to one table, and tables don't have >> >> >> neither if's or whiles (we have a limited for that just walks arrays). >> >> > >> >> > Let's just bite the bullet and add support for if's? It's not like it's >> >> > hard to invent 'struct vmstate_condition' or some such. >> >> >> >> I have to do it. The problem is not adding an optional field, is adding >> >> it conditionally on _what_, and that _what_ should also be ideally on vmstate. >> >> >> >> Later, Juan. >> > >> > pci config is on vmstate already, I don't see a problem here. >> >> vmstate don't understand pci config. > > How can it save it then? What's more, how can it load it sanely? E.g. > when loading we must make sure that device id etc match. We don't test it at all. We just load an ne2000 image into an ne2000 device, and things like that. VMState has zero knowledge of pci. It just saves some fields. >> I want to manipulate images with >> the informantion that you have given vmstate. Sending another byte >> meaning: >> >> msix_enabled >> >> and now depending on that value another field is ok with me. > > 1. this bit is there in config already. > sending it twice is redundant and so, wrong: > we will just have to add even more code > to check that these values match You need to write the code anyways. There is no way that VMState will learn PCI (or whatever else), it does'nt matter if you do: pre_save() .... s->msix_enabled_vmstate = foo(s); .... or we add a new feature to VMState that allows you to call a function (foo() in this case) to know if we send/don't send vmstate. You need to write foo anyways. > 2. it's also not backward compatible, is it? Optional fields were not allowed, really before. Format was abused and then we got problems that we have to live with them. If you mean that in the past devices used optional features, that is right. > >> You can >> calculate msix_enable at pre_save time whenever way that you see fit. > > Not everything has to fit in a global variable, we can have a table > per-device. Then each time msix_enable bit is changed, I can tell > vmstate about it. No. You only need to tell before saving. And there is nice function (pre_save()) that would be called before doing a save. You can put there any code to fill that variable. It is ok to create msix_enabled_vmstate variable, that is only used for vmstate, no problems with that. > >> What vmstate needs is a only to now if msix_enable is 0 or 1, how you >> calculate it, VMState don't care. > > Add a condition including offset into pci config and bitmask? No, defining a variable is easy for anybody. if I teach PCI to VMState, then the damn things at i2c will want me to teach it i2c, and then SunBus, and then ISA, and then ..... VMState understands all qemu. There is no problem at all doing in pre_save() msix_enabled = foo() And you can share that foo function for all msix devices if you want it. VMState don't have to learn msix, and efect is the same. Later, Juan.