From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mnubl-0007tL-Hu for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:31:41 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mnubg-0007pB-H5 for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:31:40 -0400 Received: from [199.232.76.173] (port=40786 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mnubg-0007ov-6g for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:31:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11622) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mnubf-0004qO-Pl for qemu-devel@nongnu.org; Wed, 16 Sep 2009 09:31:36 -0400 From: Juan Quintela In-Reply-To: <20090916122901.GA4729@redhat.com> (Michael S. Tsirkin's message of "Wed, 16 Sep 2009 15:29:01 +0300") References: <20090916104620.GA4456@redhat.com> <20090916114133.GA4567@redhat.com> <20090916122901.GA4729@redhat.com> Date: Wed, 16 Sep 2009 15:31:31 +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@redhat.com "Michael S. Tsirkin" wrote: > On Wed, Sep 16, 2009 at 02:13:06PM +0200, Juan Quintela wrote: >> VMState rules are simple: >> - Everything is explicit > > By the way, pci currently has cmask, > which performs checking on load, making sure > that load does not modify a constant field in config space, > which can't change as a result of guest actions. > If it does - migration fails. > > This is IMO much better and more robust than > simply hoping that there are no bugs or that > developers remember to increment a version > number each time they change some field. This one is going to be fixed. Some kind of checksum that assures you that you haven't added/removed any field of a vmstatedescription. It is not difficult to add, but no code/whatever is there. The only minimal check that it does today is that you put: VMSTATE_INT32_ARRAY_V(irq_state, PCIDevice, 4, 2), 4 is the length and 2 is the version. VMstate checks that PCIDevice has an irq_state field of type int32_t with lenght 4. I could have calculated the 4, but it is there just in case someone changes the size of the array in PCIDevice, it gets a compilation error. There is not more infrastructure yet to check for changes on the state. It should come once devices are ported to VMState. > I think it's pretty important to keep this > feature, and maybe add something similar > to other devices. > > How will VMState support this? This is the 1st request that I have. This is what the code does today (it is the same that was before): static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) { PCIDevice *s = container_of(pv, PCIDevice, config); uint8_t config[size]; int i; qemu_get_buffer(f, config, size); for (i = 0; i < size; ++i) if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i]) return -EINVAL; memcpy(s->config, config, size); pci_update_mappings(s); return 0; } /* just put buffer */ static void put_pci_config_device(QEMUFile *f, void *pv, size_t size) { const uint8_t *v = pv; qemu_put_buffer(f, v, size); } i.e. at save time, we save everything that we want to save. At load time, we only copy some things. I don't understand what cmask and wmask means, but I guess you understand this part better than me. If we need to add more checks on load, we can just hack on that function whatever you want to check/change/... VMstate don't really care (as it shouldn't) Later, Juan.