Patchwork [16/23] Port PCI Bus to VMState design

login
register
mail settings
Submitter Juan Quintela
Date Aug. 20, 2009, 5:42 p.m.
Message ID <101b7fd66dd59c7655b257eb8dd438175f5232b6.1250788880.git.quintela@redhat.com>
Download mbox | patch
Permalink /patch/31764/
State Superseded
Headers show

Comments

Juan Quintela - Aug. 20, 2009, 5:42 p.m.
This uses VARRAY and INT32_EQUAL values

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/pci.c |   41 +++++++++++------------------------------
 1 files changed, 11 insertions(+), 30 deletions(-)
Gerd Hoffmann - Aug. 21, 2009, 8:32 a.m.
On 08/20/09 19:42, Juan Quintela wrote:
> This uses VARRAY and INT32_EQUAL values

This is one place where I would change the code to reduce load/save 
complexity.

Quick grep shows that the max value for bus->nirqs is 32. 
s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation and 
allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed then.

cheers,
   Gerd
Juan Quintela - Aug. 21, 2009, 9:04 a.m.
Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/20/09 19:42, Juan Quintela wrote:
>> This uses VARRAY and INT32_EQUAL values
>
> This is one place where I would change the code to reduce load/save
> complexity.
>
> Quick grep shows that the max value for bus->nirqs is
> 32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation
> and allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed
> then.

I agree to drop INT32_EQUAL, but maintain the VARRAY.
VARRAY is nice if there is another field in the stuct that tolds you how
many are used.

If you change the 32 number, you just need to upgrade the version.
Will do something like that.

The reason that I don't want to send the whole thing is that there are
times when we are sending tables that are empty (devices not being
used).  Having an initial size makes things go away.  And then, I will
create the VMSTATE_VARRAY() that handles the two fields in one go.

Later, Juan.

> cheers,
>   Gerd
Gerd Hoffmann - Aug. 21, 2009, 9:23 a.m.
On 08/21/09 11:04, Juan Quintela wrote:
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> On 08/20/09 19:42, Juan Quintela wrote:
>>> This uses VARRAY and INT32_EQUAL values
>> This is one place where I would change the code to reduce load/save
>> complexity.
>>
>> Quick grep shows that the max value for bus->nirqs is
>> 32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation
>> and allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed
>> then.
>
> I agree to drop INT32_EQUAL, but maintain the VARRAY.
> VARRAY is nice if there is another field in the stuct that tolds you how
> many are used.

If you save all of them anyway you don't need that.
(but you still must save the bus->nirqs field obviously).

> If you change the 32 number, you just need to upgrade the version.
> Will do something like that.
>
> The reason that I don't want to send the whole thing is that there are
> times when we are sending tables that are empty (devices not being
> used).

I still would save it.  If you find a NULL pointer save a bunch of zeros 
instead.  Device state isn't that big, and I think it is good to have 
fixed-size sections.

cheers
   Gerd

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 27eac04..681b9d2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -73,36 +73,17 @@  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 static PCIBus *first_bus;

-static void pcibus_save(QEMUFile *f, void *opaque)
-{
-    PCIBus *bus = (PCIBus *)opaque;
-    int i;
-
-    qemu_put_be32(f, bus->nirq);
-    for (i = 0; i < bus->nirq; i++)
-        qemu_put_be32(f, bus->irq_count[i]);
-}
-
-static int  pcibus_load(QEMUFile *f, void *opaque, int version_id)
-{
-    PCIBus *bus = (PCIBus *)opaque;
-    int i, nirq;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    nirq = qemu_get_be32(f);
-    if (bus->nirq != nirq) {
-        fprintf(stderr, "pcibus_load: nirq mismatch: src=%d dst=%d\n",
-                nirq, bus->nirq);
-        return -EINVAL;
+static const VMStateDescription vmstate_pcibus = {
+    .name = "PCIBUS",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32_EQUAL(nirq, PCIBus),
+        VMSTATE_INT32_VARRAY(irq_count, PCIBus, nirq),
+        VMSTATE_END_OF_LIST()
     }
-
-    for (i = 0; i < nirq; i++)
-        bus->irq_count[i] = qemu_get_be32(f);
-
-    return 0;
-}
+};

 static void pci_bus_reset(void *opaque)
 {
@@ -135,7 +116,7 @@  PCIBus *pci_register_bus(DeviceState *parent, const char *name,
     bus->irq_count = qemu_mallocz(nirq * sizeof(bus->irq_count[0]));
     bus->next = first_bus;
     first_bus = bus;
-    register_savevm("PCIBUS", nbus++, 1, pcibus_save, pcibus_load, bus);
+    vmstate_register(nbus++, &vmstate_pcibus, bus);
     qemu_register_reset(pci_bus_reset, bus);
     return bus;
 }