Various NICs: Fix suspend/resume of multiple instances

PATCH In review Jan Kiszka 2 participants 3 messages Jul 16, 2008 – Jul 17, 2008

Share & tools

Jan Kiszka

author

2008-07-16 17:20 UTC #0 lore raw

PATCH 1/1 Various NICs: Fix suspend/resume of multiple instances

We ran into the issue that restoring two (or more) instances of the same NIC does not work. The reason is that the same instance number is passed to register_savevm by many NIC emulations. Patch below addresses this for eepro100, ne2000, pcnet and rtl8139 (I just quickly checked the set available for x86).

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/eepro100.c |    4 ++--
 hw/ne2000.c   |    3 ++-
 hw/pcnet.c    |    4 +++-
 hw/rtl8139.c  |    4 ++--
 4 files changed, 9 insertions(+), 6 deletions(-)

hw/pcnet.c +3 -1

Index: b/hw/pcnet.c
===================================================================
@@ -1896,6 +1896,8 @@ static int pcnet_load(QEMUFile *f, void
 
 static void pcnet_common_init(PCNetState *d, NICInfo *nd, const char *info_str)
 {
+    static int instance;
+
     d->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, d);
 
     d->nd = nd;

@@ -1916,7 +1918,7 @@ static void pcnet_common_init(PCNetState
         d->vc = NULL;
     }
     pcnet_h_reset(d);
-    register_savevm("pcnet", 0, 2, pcnet_save, pcnet_load, d);
+    register_savevm("pcnet", instance++, 2, pcnet_save, pcnet_load, d);
 }
 
 /* PCI interface */

hw/eepro100.c +2 -2

Index: b/hw/eepro100.c
===================================================================
@@ -1745,6 +1745,7 @@ static void nic_save(QEMUFile * f, void
 static void nic_init(PCIBus * bus, NICInfo * nd,
                      const char *name, uint32_t device)
 {
+    static int instance;
     PCIEEPRO100State *d;
     EEPRO100State *s;

@@ -1792,8 +1793,7 @@ static void nic_init(PCIBus * bus, NICIn
 
     qemu_register_reset(nic_reset, s);
 
-    /* XXX: instance number ? */
-    register_savevm(name, 0, 3, nic_save, nic_load, s);
+    register_savevm(name, instance++, 3, nic_save, nic_load, s);
 }
 
 void pci_i82551_init(PCIBus * bus, NICInfo * nd, int devfn)

hw/ne2000.c +2 -1

Index: b/hw/ne2000.c
===================================================================
@@ -720,6 +720,7 @@ static int ne2000_load(QEMUFile* f,void*
 
 void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd)
 {
+    static int instance;
     NE2000State *s;
 
     s = qemu_mallocz(sizeof(NE2000State));

@@ -753,7 +754,7 @@ void isa_ne2000_init(int base, qemu_irq
              s->macaddr[4],
              s->macaddr[5]);
 
-    register_savevm("ne2000", 0, 2, ne2000_save, ne2000_load, s);
+    register_savevm("ne2000", instance++, 2, ne2000_save, ne2000_load, s);
 }
 
 /***********************************************************/

hw/rtl8139.c +2 -2

Index: b/hw/rtl8139.c
===================================================================
@@ -3402,6 +3402,7 @@ static void rtl8139_timer(void *opaque)
 
 void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
+    static int instance;
     PCIRTL8139State *d;
     RTL8139State *s;
     uint8_t *pci_conf;

@@ -3454,8 +3455,7 @@ void pci_rtl8139_init(PCIBus *bus, NICIn
     s->cplus_txbuffer_len = 0;
     s->cplus_txbuffer_offset = 0;
 
-    /* XXX: instance number ? */
-    register_savevm("rtl8139", 0, 3, rtl8139_save, rtl8139_load, s);
+    register_savevm("rtl8139", instance++, 3, rtl8139_save, rtl8139_load, s);
 
 #if RTL8139_ONBOARD_TIMER
     s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);

Paul Brook

2008-07-16 23:44 UTC #1 lore raw

Quoted 4 lines · Jan Kiszka

We ran into the issue that restoring two (or more) instances of the same NIC does not work. The reason is that the same instance number is passed to register_savevm by many NIC emulations. Patch below addresses this for eepro100, ne2000, pcnet and rtl8139

This is wrong. You should pass -1.

Paul

Jan Kiszka

author

2008-07-17 08:16 UTC #2 lore raw

Quoted 7 lines · Paul Brook

Quoted 4 lines · Jan Kiszka
We ran into the issue that restoring two (or more) instances of the same NIC does not work. The reason is that the same instance number is passed to register_savevm by many NIC emulations. Patch below addresses this for eepro100, ne2000, pcnet and rtl8139
This is wrong. You should pass -1.

Well... OK. Here is a version that
- documents this preferred usage pattern,
- fixes those four NICs accordingly,
- cleans up the e1000 as well so that no one will ever use this as an incorrect reference again.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/e1000.c    |   10 +++-------
 hw/eepro100.c |    3 +--
 hw/ne2000.c   |    5 ++---
 hw/pcnet.c    |    2 +-
 hw/rtl8139.c  |    3 +--
 vl.c          |    4 +++-
 6 files changed, 11 insertions(+), 16 deletions(-)

hw/pcnet.c +1 -1

Index: b/hw/pcnet.c
===================================================================
@@ -1916,7 +1916,7 @@ static void pcnet_common_init(PCNetState
         d->vc = NULL;
     }
     pcnet_h_reset(d);
-    register_savevm("pcnet", 0, 2, pcnet_save, pcnet_load, d);
+    register_savevm("pcnet", -1, 2, pcnet_save, pcnet_load, d);
 }
 
 /* PCI interface */

hw/eepro100.c +1 -2

Index: b/hw/eepro100.c
===================================================================
@@ -1792,8 +1792,7 @@ static void nic_init(PCIBus * bus, NICIn
 
     qemu_register_reset(nic_reset, s);
 
-    /* XXX: instance number ? */
-    register_savevm(name, 0, 3, nic_save, nic_load, s);
+    register_savevm(name, -1, 3, nic_save, nic_load, s);
 }
 
 void pci_i82551_init(PCIBus * bus, NICInfo * nd, int devfn)

hw/ne2000.c +2 -3

Index: b/hw/ne2000.c
===================================================================
@@ -753,7 +753,7 @@ void isa_ne2000_init(int base, qemu_irq
              s->macaddr[4],
              s->macaddr[5]);
 
-    register_savevm("ne2000", 0, 2, ne2000_save, ne2000_load, s);
+    register_savevm("ne2000", -1, 2, ne2000_save, ne2000_load, s);
 }
 
 /***********************************************************/

@@ -823,6 +823,5 @@ void pci_ne2000_init(PCIBus *bus, NICInf
              s->macaddr[4],
              s->macaddr[5]);
 
-    /* XXX: instance number ? */
-    register_savevm("ne2000", 0, 3, ne2000_save, ne2000_load, s);
+    register_savevm("ne2000", -1, 3, ne2000_save, ne2000_load, s);
 }

hw/rtl8139.c +1 -2

Index: b/hw/rtl8139.c
===================================================================
@@ -3454,8 +3454,7 @@ void pci_rtl8139_init(PCIBus *bus, NICIn
     s->cplus_txbuffer_len = 0;
     s->cplus_txbuffer_offset = 0;
 
-    /* XXX: instance number ? */
-    register_savevm("rtl8139", 0, 3, rtl8139_save, rtl8139_load, s);
+    register_savevm("rtl8139", -1, 3, rtl8139_save, rtl8139_load, s);
 
 #if RTL8139_ONBOARD_TIMER
     s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);

hw/e1000.c +3 -7

Index: b/hw/e1000.c
===================================================================
@@ -76,7 +76,6 @@ typedef struct E1000State_st {
     PCIDevice dev;
     VLANClientState *vc;
     NICInfo *nd;
-    uint32_t instance;
     uint32_t mmio_base;
     int mmio_index;

@@ -814,7 +813,6 @@ nic_save(QEMUFile *f, void *opaque)
     int i, j;
 
     pci_device_save(&s->dev, f);
-    qemu_put_be32s(f, &s->instance);
     qemu_put_be32s(f, &s->mmio_base);
     qemu_put_be32s(f, &s->rxbuf_size);
     qemu_put_be32s(f, &s->rxbuf_min_shift);

@@ -859,7 +857,8 @@ nic_load(QEMUFile *f, void *opaque, int
 
     if ((ret = pci_device_load(&s->dev, f)) < 0)
         return ret;
-    qemu_get_be32s(f, &s->instance);
+    if (version_id == 1)
+        qemu_get_be32s(f, &i); /* once some unused instance id */
     qemu_get_be32s(f, &s->mmio_base);
     qemu_get_be32s(f, &s->rxbuf_size);
     qemu_get_be32s(f, &s->rxbuf_min_shift);

@@ -958,7 +957,6 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd,
 {
     E1000State *d;
     uint8_t *pci_conf;
-    static int instance;
     uint16_t checksum = 0;
     char *info_str = "e1000";
     int i;

@@ -989,8 +987,6 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd,
     pci_register_io_region((PCIDevice *)d, 1, IOPORT_SIZE,
                            PCI_ADDRESS_SPACE_IO, ioport_map);
 
-    d->instance = instance++;
-
     d->nd = nd;
     memmove(d->eeprom_data, e1000_eeprom_template,
         sizeof e1000_eeprom_template);

@@ -1016,5 +1012,5 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd,
              d->nd->macaddr[0], d->nd->macaddr[1], d->nd->macaddr[2],
              d->nd->macaddr[3], d->nd->macaddr[4], d->nd->macaddr[5]);
 
-    register_savevm(info_str, d->instance, 1, nic_save, nic_load, d);
+    register_savevm(info_str, -1, 2, nic_save, nic_load, d);
 }

vl.c +3 -1

Index: b/vl.c
===================================================================
@@ -6063,7 +6063,9 @@ typedef struct SaveStateEntry {
 static SaveStateEntry *first_se;
 
 /* TODO: Individual devices generally have very little idea about the rest
-   of the system, so instance_id should be removed/replaced.  */
+   of the system, so instance_id should be removed/replaced.
+   Meanwhile pass -1 as instance_id if you do not already have a clearly
+   distinguishing id for all instances of your device class. */
 int register_savevm(const char *idstr,
                     int instance_id,
                     int version_id,