From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KJOee-0006gO-3Y for qemu-devel@nongnu.org; Thu, 17 Jul 2008 04:16:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KJOed-0006fo-8J for qemu-devel@nongnu.org; Thu, 17 Jul 2008 04:15:59 -0400 Received: from [199.232.76.173] (port=59603 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KJOec-0006fh-Vw for qemu-devel@nongnu.org; Thu, 17 Jul 2008 04:15:59 -0400 Received: from mx20.gnu.org ([199.232.41.8]:63003) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KJOec-0004PF-Gx for qemu-devel@nongnu.org; Thu, 17 Jul 2008 04:15:58 -0400 Received: from lizzard.sbs.de ([194.138.37.39]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KJOeb-00066Y-C4 for qemu-devel@nongnu.org; Thu, 17 Jul 2008 04:15:57 -0400 Message-ID: <487EFFB8.5050707@siemens.com> Date: Thu, 17 Jul 2008 10:15:52 +0200 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Various NICs: Fix suspend/resume of multiple instances References: <487E2DCB.2090902@siemens.com> <200807170044.52401.paul@codesourcery.com> In-Reply-To: <200807170044.52401.paul@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Brook Cc: qemu-devel@nongnu.org Paul Brook wrote: > On Wednesday 16 July 2008, Jan Kiszka wrote: >> 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 --- 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(-) Index: b/hw/pcnet.c =================================================================== --- a/hw/pcnet.c +++ 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 */ Index: b/hw/eepro100.c =================================================================== --- a/hw/eepro100.c +++ 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) Index: b/hw/ne2000.c =================================================================== --- a/hw/ne2000.c +++ 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); } Index: b/hw/rtl8139.c =================================================================== --- a/hw/rtl8139.c +++ 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); Index: b/hw/e1000.c =================================================================== --- a/hw/e1000.c +++ 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); } Index: b/vl.c =================================================================== --- a/vl.c +++ 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,