From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MdmHM-0006Lt-2M for qemu-devel@nongnu.org; Wed, 19 Aug 2009 10:36:44 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MdmHH-0006J8-CW for qemu-devel@nongnu.org; Wed, 19 Aug 2009 10:36:43 -0400 Received: from [199.232.76.173] (port=46911 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MdmHH-0006Iy-8E for qemu-devel@nongnu.org; Wed, 19 Aug 2009 10:36:39 -0400 Received: from [66.187.237.31] (port=37243 helo=mx2.redhat.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MdmHG-0003LI-Kb for qemu-devel@nongnu.org; Wed, 19 Aug 2009 10:36:39 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n7JEaTdS020106 for ; Wed, 19 Aug 2009 10:36:29 -0400 From: Juan Quintela Date: Wed, 19 Aug 2009 16:34:11 +0200 Message-Id: Subject: [Qemu-devel] [PATCH 0/3] New VMState table based load/save infrastructure List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org His this is 2nd version of the series, I changed: v2: * Add _V() constructors. We almost never need the version field. * Add const to VMStateDescription uses (BlueWirl suggestion) * Add const to VMStateInfo uses * Remove VMStateInfo parameter from get/put callbacks. Not needed. * Load of old versions is done with old foo_load function (Kraxel suggestion) * Add struct support * Move i8254 to new infrastructure, to test struct support * I removed the autostart patches, updated version sent to list i8254 note: There is only one timer in the 1st channel, in the other two channels, the timer is not created ever, this is the reason why I sent the irq_timer not in the irq channels. ToDo: - still not optional test function. Notice that this can be done with a local VMStateInfo struct. - Start/end functions, not done waiting for user case (next goal is virtio and I think we need them there). - For structures, Kraxel suggested to use a VMSTATE_INCLUDE() instead of having to declare a new VMStateInfo struct. As I already have the new struct code working, I sent it with the struct. Thinking about how to implement the VMSTATE_INCLUDE() and which one is easier to use. Comments? v1: This series implement a new save/load approach for device state. I want to start a discussion on how is better to design it. This is the reason why only one device has been ported to this new approach. I increased the version_id of apic state, just to sent the 3 arrays (isr, tmr, irr) as arrays. I didn't want to introduce structure support before there is any discussion about this, not that it is imposible to do it in a compatible way. Problems of current approach: - have to maintain both load and save functions on sync - it is not type safe - you can't use easily the device save/load information to do more things. For instance, print the state of a device in the monitor. To do this, you basically have to copy foo_apic() function. - You can't examine the saved images in any meaningful way New approach: - you only have to declare the state once, in a table, save/load functions are generic for all devices. - it is type safe - it is qdev-like design (i.e. you don't have to learn yet a completely different appearch for a similar problem) - it is trivial to add new operations to VMStateInfo, and then you can use them anywhere (for instance printing in the monitor, etc). Problems of new approach: - it is less flexible, you can't progamatically save in any order that you want. It is not clear to me that we _want_ that flexibility. Look for instance and hw/msix.c and think if you can be sure that save/load are on sync, and as soon as we get new versions, that things could be tested to be wright. What do I want to discuss: a- do we want to be able to load state form old versions? I am not sure that we are going to get this right for complex devices, and I don't completely see how we can have any kind of testing here. There was already a discussion about this on the list. b- Is this the right approach? What more do we want/need? For instance, implementing struct save support, and calling other "sub-descriptions" is not difficult, we just have to decide if we want it. c- In the current approach, we have loops to send arrays, I think that one got already done better on new approarch. But we don't have support for ifs (see hw/ide.c if (s->identify_set) { qemu_get_buffer(f, (uint8_t *)s->identify_data, 512); } Do we want support for things like that? d- how aggresive should the new design be? i.e. be able to be compatible with old design is good, or can we start with a clean sheet and just remove the gotchas of the previous design? Juan Quintela (3): New VMstate save/load infrastructure Port apic to new VMState design Port i8254 to new VMState design hw/apic.c | 67 ++++++------- hw/hw.h | 153 ++++++++++++++++++++++++++++ hw/i8254.c | 72 ++++++++------ savevm.c | 325 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 548 insertions(+), 69 deletions(-)