qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH RFC 0/5] New VMState table based load/save infrastructure
Date: Tue, 18 Aug 2009 15:34:05 +0200	[thread overview]
Message-ID: <cover.1250601335.git.quintela@redhat.com> (raw)

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?

Comments?

Later, Juan.

P.D. the 1st two patches just improve loadvm error messages.  I included then
     here because they make easier to play with the state/versions.  Thinking
     about proper solution that will be sent in a different patch.

Juan Quintela (5):
  loadvm already call vm_start()
  Don't call vm_start() if there was an error loading
  Don't ignore load_state() error return values.
  New VMstate save/load infrastructure
  Port apic to new VMState design

 hw/apic.c |   96 +++++-------------
 hw/hw.h   |  124 +++++++++++++++++++++++
 savevm.c  |  322 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 vl.c      |    4 +-
 4 files changed, 470 insertions(+), 76 deletions(-)

             reply	other threads:[~2009-08-18 13:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-18 13:34 Juan Quintela [this message]
2009-08-18 13:34 ` [Qemu-devel] [PATCH 1/5] loadvm already call vm_start() Juan Quintela
2009-08-18 13:34 ` [Qemu-devel] [PATCH 2/5] Don't call vm_start() if there was an error loading Juan Quintela
2009-08-18 13:34 ` [Qemu-devel] [PATCH 3/5] Don't ignore load_state() error return values Juan Quintela
2009-08-18 13:34 ` [Qemu-devel] [PATCH 4/5] New VMstate save/load infrastructure Juan Quintela
2009-08-18 17:13   ` Blue Swirl
2009-08-18 17:56     ` [Qemu-devel] " Juan Quintela
2009-08-19  7:49   ` [Qemu-devel] " Gerd Hoffmann
2009-08-19  9:38     ` [Qemu-devel] " Juan Quintela
2009-08-19 12:43       ` Gerd Hoffmann
2009-08-18 13:34 ` [Qemu-devel] [PATCH 5/5] Port apic to new VMState design Juan Quintela
2009-08-18 14:24   ` Reimar Döffinger
     [not found]   ` <20090818142405.GA16563@1und1.de>
     [not found]     ` <m37hx1tc9l.fsf@neno.mitica>
2009-08-18 15:21       ` [Qemu-devel] " Reimar Döffinger
2009-08-18 15:38         ` Juan Quintela
2009-08-18 16:06           ` Reimar Döffinger
2009-08-18 16:37             ` Juan Quintela
2009-08-19  8:00         ` Gerd Hoffmann
2009-08-19  9:10           ` Reimar Döffinger
     [not found]           ` <20090819085334.GA31062@1und1.de>
     [not found]             ` <4A8BC0C7.4010806@redhat.com>
2009-08-19  9:16               ` Reimar Döffinger
2009-08-19  7:41 ` [Qemu-devel] [PATCH RFC 0/5] New VMState table based load/save infrastructure Gerd Hoffmann
2009-08-19 10:15   ` [Qemu-devel] " Juan Quintela
2009-08-19 12:55     ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1250601335.git.quintela@redhat.com \
    --to=quintela@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).