[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH RFC 0/5] New VMState table based load/save infra
From: |
Juan Quintela |
Subject: |
[Qemu-devel] Re: [PATCH RFC 0/5] New VMState table based load/save infrastructure |
Date: |
Wed, 19 Aug 2009 12:15:03 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Reply-to: address@hidden
Gerd Hoffmann <address@hidden> wrote:
>> 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.
>
> Yes, we want. And I think the right approach to do that is to simply
> use the old, existing code and switch based on version_id. To stick
> with the apic example: When loadvm finds a v2 (or older) apic
> section, go call apic_load. When loadvm finds a v3 (or newer) apic
> section, call the new generic load code with the apic vmstatefield
> list.
Liked it, will implement it in next round.
>> 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.
>
> Yes, we want. PCI devices call a generic function to save pci
> state. We want a common pci vmstatefield list too and have some way to
> refer to them from the device tables.
That is the reason that I wanted to maintain the ->info field. See the
other email.
>> 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?
>
> No. We want fixed-sized sections, bonus points for a 'size' field in
> the header. Just save everything unconditionally. The current
> save/load functions do way too much stuff conditionally. Saving a few
> bytes simply isn't worth the complexity of getting that right.
Problem with this is when things can be NULL :p
>From msix.c
if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
return;
}
void msix_save(PCIDevice *dev, QEMUFile *f)
{
unsigned n = dev->msix_entries_nr;
qemu_put_buffer(f, dev->msix_table_page, n * MSIX_ENTRY_SIZE);
qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
}
msix_table_page is not NULL only if QEMU_PCI_CAP_MSIX is present.
I think that optional fields are needed, or a better way of dealing with
things like this.
>> 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?
>
> Handle compatibility by keeping the old load functions and start over
> with a clean sheet.
Liked this idea, makes things way, way easier. I was looking at
virtio_net_load() and had a head-ache. With your approach, things
become really easy.
Thaks for the review, Juan
- [Qemu-devel] [PATCH 5/5] Port apic to new VMState design, (continued)
Re: [Qemu-devel] [PATCH RFC 0/5] New VMState table based load/save infrastructure, Gerd Hoffmann, 2009/08/19
- [Qemu-devel] Re: [PATCH RFC 0/5] New VMState table based load/save infrastructure,
Juan Quintela <=