[Qemu-devel] Re: [PATCH 2/7] net: Add VLAN client cleanup handler

[Jan Kiszka <jan.kiszka@siemens.com>]

Mark McLoughlin wrote:
> On Wed, 2009-04-15 at 14:18 +0100, Mark McLoughlin wrote:
>> Hi Jan,
>>
>> On Tue, 2009-04-14 at 19:29 +0200, Jan Kiszka wrote:
>>> Do proper VLAN client cleanup via a callback handler. This fixes
>>> resource leakage on host_net_remove and allows a generic net_cleanup
>>> implementation.
>> Yep, we need this.
>>
>> However, I've got a different version in my queue (see below) - I've
>> been holding back on posting it until I finished some other patches I'm
>> working on, but I'll polish it off and post this afternoon.
>>
>> Differences from yours include:
>>
>>  - I've added cleanup code to all the NICs, including adding 
>>    unregister_savevm() so that we don't try and save deleted NICs
>>
>>  - Rather than adding yet another param to new_vlan_client(), I just 
>>    initialize vc->cleanup after creating the client; another patch in
>>    my queue removes all callbacks to new_vlan_client() because as more
>>    are added it just gets terribly unwieldy.
>>
>>  - I remove the io handler on e.g. the tapfd when freeing - otherwise 
>>    we'll continue to poll the fd AFAICS
>>
>>  - I implement net_cleanup() by calling del_vlan_client() on all clients
>>
>> Apart from that, the patches are very similar - are you okay with just
>> waiting for my patch?
> 
> Doh, it would be helpful if I attached :-)

Ah, good, there it is. Is it considered ready for inclusion, also into
stable? Then I would start rebasing my patches and give feedback if I
happen to find quirks.

Jan

> 
> Cheers,
> Mark.
> 
> From: Mark McLoughlin <markmc@redhat.com>
> Subject: [PATCH] Introduce VLANClientState::cleanup()
> 
> We're currently leaking memory and file descriptors on device
> hot-unplug.
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  qemu/hw/e1000.c          |   12 +++++-----
>  qemu/hw/eepro100.c       |   12 +++++++++
>  qemu/hw/etraxfs_eth.c    |   11 +++++++++
>  qemu/hw/mcf_fec.c        |   18 +++++++++++---
>  qemu/hw/mipsnet.c        |   14 +++++++++++
>  qemu/hw/musicpal.c       |   18 +++++++++++---
>  qemu/hw/ne2000.c         |   24 +++++++++++++++++++
>  qemu/hw/pcnet.c          |   43 +++++++++++++++++++++++++++++++---
>  qemu/hw/rtl8139.c        |   20 ++++++++++++++++
>  qemu/hw/smc91c111.c      |   17 ++++++++++---
>  qemu/hw/stellaris_enet.c |   20 +++++++++++++---
>  qemu/hw/usb-net.c        |   11 +++++++-
>  qemu/hw/virtio-net.c     |   14 +++++++++++
>  qemu/net.c               |   57 +++++++++++++++++++++++++++++++++------------
>  qemu/net.h               |    2 +
>  qemu/tap-win32.c         |   13 ++++++++++
>  16 files changed, 263 insertions(+), 43 deletions(-)
> 
> diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
> index 2d16774..978f789 100644
> --- a/qemu/hw/e1000.c
> +++ b/qemu/hw/e1000.c
> @@ -1033,14 +1033,14 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
>                                       excluded_regs[i] - 4);
>  }
>  
> -static int
> -pci_e1000_uninit(PCIDevice *dev)
> +static void
> +e1000_cleanup(VLANClientState *vc)
>  {
> -    E1000State *d = (E1000State *) dev;
> +    E1000State *d = vc->opaque;
>  
> -    cpu_unregister_io_memory(d->mmio_index);
> +    unregister_savevm("e1000", d);
>  
> -    return 0;
> +    cpu_unregister_io_memory(d->mmio_index);
>  }
>  
>  PCIDevice *
> @@ -1095,12 +1095,12 @@ pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn)
>  
>      d->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   e1000_receive, e1000_can_receive, d);
> +    d->vc->cleanup = e1000_cleanup;
>      d->vc->link_status_changed = e1000_set_link_status;
>  
>      qemu_format_nic_info_str(d->vc, nd->macaddr);
>  
>      register_savevm(info_str, -1, 2, nic_save, nic_load, d);
> -    d->dev.unregister = pci_e1000_uninit;
>  
>      return (PCIDevice *)d;
>  }
> diff --git a/qemu/hw/eepro100.c b/qemu/hw/eepro100.c
> index 0a343df..dc50704 100644
> --- a/qemu/hw/eepro100.c
> +++ b/qemu/hw/eepro100.c
> @@ -1710,6 +1710,17 @@ static void nic_save(QEMUFile * f, void *opaque)
>      qemu_put_buffer(f, s->configuration, sizeof(s->configuration));
>  }
>  
> +static void nic_cleanup(VLANClientState *vc)
> +{
> +    EEPRO100State *s = vc->opaque;
> +
> +    unregister_savevm(vc->name, s);
> +
> +    cpu_unregister_io_memory(s->mmio_index);
> +
> +    eeprom93xx_free(s->eeprom);
> +}
> +
>  static PCIDevice *nic_init(PCIBus * bus, NICInfo * nd,
>                       const char *name, uint32_t device)
>  {
> @@ -1752,6 +1763,7 @@ static PCIDevice *nic_init(PCIBus * bus, NICInfo * nd,
>  
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   nic_receive, nic_can_receive, s);
> +    s->vc->cleanup = nic_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  
> diff --git a/qemu/hw/etraxfs_eth.c b/qemu/hw/etraxfs_eth.c
> index c87e55f..b0f7f6d 100644
> --- a/qemu/hw/etraxfs_eth.c
> +++ b/qemu/hw/etraxfs_eth.c
> @@ -554,6 +554,16 @@ static CPUWriteMemoryFunc *eth_write[] = {
>  	&eth_writel,
>  };
>  
> +static void etraxfs_eth_cleanup(VLANClientState *vc)
> +{
> +        struct fs_eth *eth = vc->opaque;
> +
> +        cpu_unregister_io_memory(eth->ethregs);
> +
> +        qemu_free(eth->dma_out);
> +        qemu_free(eth);
> +}
> +
>  void *etraxfs_eth_init(NICInfo *nd, CPUState *env, 
>  		       qemu_irq *irq, target_phys_addr_t base, int phyaddr)
>  {
> @@ -586,6 +596,7 @@ void *etraxfs_eth_init(NICInfo *nd, CPUState *env,
>  
>  	eth->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>  				       eth_receive, eth_can_receive, eth);
> +	eth->vc->cleanup = etraxfs_eth_cleanup;
>  	eth->vc->opaque = eth;
>  	eth->vc->link_status_changed = eth_set_link;
>  
> diff --git a/qemu/hw/mcf_fec.c b/qemu/hw/mcf_fec.c
> index 413c569..6204772 100644
> --- a/qemu/hw/mcf_fec.c
> +++ b/qemu/hw/mcf_fec.c
> @@ -24,6 +24,7 @@ do { printf("mcf_fec: " fmt , ##args); } while (0)
>  
>  typedef struct {
>      qemu_irq *irq;
> +    int mmio_index;
>      VLANClientState *vc;
>      uint32_t irq_state;
>      uint32_t eir;
> @@ -441,21 +442,30 @@ static CPUWriteMemoryFunc *mcf_fec_writefn[] = {
>     mcf_fec_write
>  };
>  
> +static void mcf_fec_cleanup(VLANClientState *vc)
> +{
> +    mcf_fec_state *s = vc->opaque;
> +
> +    cpu_unregister_io_memory(s->mmio_index);
> +
> +    qemu_free(s);
> +}
> +
>  void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq)
>  {
>      mcf_fec_state *s;
> -    int iomemtype;
>  
>      qemu_check_nic_model(nd, "mcf_fec");
>  
>      s = (mcf_fec_state *)qemu_mallocz(sizeof(mcf_fec_state));
>      s->irq = irq;
> -    iomemtype = cpu_register_io_memory(0, mcf_fec_readfn,
> -                                       mcf_fec_writefn, s);
> -    cpu_register_physical_memory(base, 0x400, iomemtype);
> +    s->mmio_index = cpu_register_io_memory(0, mcf_fec_readfn,
> +                                           mcf_fec_writefn, s);
> +    cpu_register_physical_memory(base, 0x400, s->mmio_index);
>  
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   mcf_fec_receive, mcf_fec_can_receive, s);
> +    s->vc->cleanup = mcf_fec_cleanup;
>      memcpy(s->macaddr, nd->macaddr, 6);
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  }
> diff --git a/qemu/hw/mipsnet.c b/qemu/hw/mipsnet.c
> index de001ec..1fde386 100644
> --- a/qemu/hw/mipsnet.c
> +++ b/qemu/hw/mipsnet.c
> @@ -33,6 +33,7 @@ typedef struct MIPSnetState {
>      uint32_t intctl;
>      uint8_t rx_buffer[MAX_ETH_FRAME_SIZE];
>      uint8_t tx_buffer[MAX_ETH_FRAME_SIZE];
> +    int io_base;
>      qemu_irq irq;
>      VLANClientState *vc;
>  } MIPSnetState;
> @@ -231,6 +232,17 @@ static int mipsnet_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> +static void mipsnet_cleanup(VLANClientState *vc)
> +{
> +    MIPSnetState *s = vc->opaque;
> +
> +    unregister_savevm("mipsnet", s);
> +
> +    isa_unassign_ioport(s->io_base, 36);
> +
> +    qemu_free(s);
> +}
> +
>  void mipsnet_init (int base, qemu_irq irq, NICInfo *nd)
>  {
>      MIPSnetState *s;
> @@ -246,9 +258,11 @@ void mipsnet_init (int base, qemu_irq irq, NICInfo *nd)
>      register_ioport_write(base, 36, 4, mipsnet_ioport_write, s);
>      register_ioport_read(base, 36, 4, mipsnet_ioport_read, s);
>  
> +    s->io_base = base;
>      s->irq = irq;
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   mipsnet_receive, mipsnet_can_receive, s);
> +    s->vc->cleanup = mipsnet_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, nd->macaddr);
>  
> diff --git a/qemu/hw/musicpal.c b/qemu/hw/musicpal.c
> index b19115f..395d2ea 100644
> --- a/qemu/hw/musicpal.c
> +++ b/qemu/hw/musicpal.c
> @@ -554,6 +554,7 @@ typedef struct mv88w8618_eth_state {
>      uint32_t smir;
>      uint32_t icr;
>      uint32_t imr;
> +    int mmio_index;
>      int vlan_header;
>      mv88w8618_tx_desc *tx_queue[2];
>      mv88w8618_rx_desc *rx_queue[4];
> @@ -714,10 +715,18 @@ static CPUWriteMemoryFunc *mv88w8618_eth_writefn[] = {
>      mv88w8618_eth_write
>  };
>  
> +static void mv88w8618_eth_cleanup(VLANClientState *vc)
> +{
> +    mv88w8618_eth_state *s = vc->opaque;
> +
> +    cpu_unregister_io_memory(s->mmio_index);
> +
> +    qemu_free(s);
> +}
> +
>  static void mv88w8618_eth_init(NICInfo *nd, uint32_t base, qemu_irq irq)
>  {
>      mv88w8618_eth_state *s;
> -    int iomemtype;
>  
>      qemu_check_nic_model(nd, "mv88w8618");
>  
> @@ -725,9 +734,10 @@ static void mv88w8618_eth_init(NICInfo *nd, uint32_t base, qemu_irq irq)
>      s->irq = irq;
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   eth_receive, eth_can_receive, s);
> -    iomemtype = cpu_register_io_memory(0, mv88w8618_eth_readfn,
> -                                       mv88w8618_eth_writefn, s);
> -    cpu_register_physical_memory(base, MP_ETH_SIZE, iomemtype);
> +    s->vc->cleanup = mv88w8618_eth_cleanup;
> +    s->mmio_index = cpu_register_io_memory(0, mv88w8618_eth_readfn,
> +                                           mv88w8618_eth_writefn, s);
> +    cpu_register_physical_memory(base, MP_ETH_SIZE, s->mmio_index);
>  }
>  
>  /* LCD register offsets */
> diff --git a/qemu/hw/ne2000.c b/qemu/hw/ne2000.c
> index 24a66bb..7fe3975 100644
> --- a/qemu/hw/ne2000.c
> +++ b/qemu/hw/ne2000.c
> @@ -140,6 +140,7 @@ typedef struct NE2000State {
>      uint8_t curpag;
>      uint8_t mult[8]; /* multicast mask array */
>      qemu_irq irq;
> +    int isa_io_base;
>      PCIDevice *pci_dev;
>      VLANClientState *vc;
>      uint8_t macaddr[6];
> @@ -718,6 +719,19 @@ static int ne2000_load(QEMUFile* f,void* opaque,int version_id)
>  	return 0;
>  }
>  
> +static void isa_ne2000_cleanup(VLANClientState *vc)
> +{
> +    NE2000State *s = vc->opaque;
> +
> +    unregister_savevm("ne2000", s);
> +
> +    isa_unassign_ioport(s->isa_io_base, 16);
> +    isa_unassign_ioport(s->isa_io_base + 0x10, 2);
> +    isa_unassign_ioport(s->isa_io_base + 0x1f, 1);
> +
> +    qemu_free(s);
> +}
> +
>  void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd)
>  {
>      NE2000State *s;
> @@ -736,6 +750,7 @@ void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd)
>  
>      register_ioport_write(base + 0x1f, 1, 1, ne2000_reset_ioport_write, s);
>      register_ioport_read(base + 0x1f, 1, 1, ne2000_reset_ioport_read, s);
> +    s->isa_io_base = base;
>      s->irq = irq;
>      memcpy(s->macaddr, nd->macaddr, 6);
>  
> @@ -743,6 +758,7 @@ void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd)
>  
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   ne2000_receive, ne2000_can_receive, s);
> +    s->vc->cleanup = isa_ne2000_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  
> @@ -777,6 +793,13 @@ static void ne2000_map(PCIDevice *pci_dev, int region_num,
>      register_ioport_read(addr + 0x1f, 1, 1, ne2000_reset_ioport_read, s);
>  }
>  
> +static void ne2000_cleanup(VLANClientState *vc)
> +{
> +    NE2000State *s = vc->opaque;
> +
> +    unregister_savevm("ne2000", s);
> +}
> +
>  PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn)
>  {
>      PCINE2000State *d;
> @@ -803,6 +826,7 @@ PCIDevice *pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn)
>      ne2000_reset(s);
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   ne2000_receive, ne2000_can_receive, s);
> +    s->vc->cleanup = ne2000_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  
> diff --git a/qemu/hw/pcnet.c b/qemu/hw/pcnet.c
> index 0e63c3d..dd22a6c 100644
> --- a/qemu/hw/pcnet.c
> +++ b/qemu/hw/pcnet.c
> @@ -75,6 +75,7 @@ struct PCNetState_st {
>      uint8_t buffer[4096];
>      int tx_busy;
>      qemu_irq irq;
> +    qemu_irq *reset_irq;
>      void (*phys_mem_read)(void *dma_opaque, target_phys_addr_t addr,
>                           uint8_t *buf, int len, int do_bswap);
>      void (*phys_mem_write)(void *dma_opaque, target_phys_addr_t addr,
> @@ -1929,6 +1930,13 @@ static int pcnet_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> +static void pcnet_common_cleanup(PCNetState *d)
> +{
> +    unregister_savevm("pcnet", d);
> +    qemu_del_timer(d->poll_timer);
> +    qemu_free_timer(d->poll_timer);
> +}
> +
>  static void pcnet_common_init(PCNetState *d, NICInfo *nd)
>  {
>      d->poll_timer = qemu_new_timer(vm_clock, pcnet_poll_timer, d);
> @@ -1982,6 +1990,15 @@ static void pci_physical_memory_read(void *dma_opaque, target_phys_addr_t addr,
>      cpu_physical_memory_read(addr, buf, len);
>  }
>  
> +static void pci_pcnet_cleanup(NetClient *client)
> +{
> +    PCNetState *d = client->opaque;
> +
> +    pcnet_common_cleanup(d);
> +
> +    cpu_unregister_io_memory(d->mmio_index);
> +}
> +
>  PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn)
>  {
>      PCNetState *d;
> @@ -2029,6 +2046,9 @@ PCIDevice *pci_pcnet_init(PCIBus *bus, NICInfo *nd, int devfn)
>      d->pci_dev = &d->dev;
>  
>      pcnet_common_init(d, nd);
> +
> +    d->client->cleanup = pci_pcnet_cleanup;
> +
>      return (PCIDevice *)d;
>  }
>  
> @@ -2078,29 +2098,44 @@ static CPUWriteMemoryFunc *lance_mem_write[3] = {
>      NULL,
>  };
>  
> +static void lance_cleanup(NetClient *client)
> +{
> +    PCNetState *d = client->opaque;
> +
> +    pcnet_common_cleanup(d);
> +
> +    qemu_free_irqs(d->reset_irq);
> +
> +    cpu_unregister_io_memory(d->mmio_index);
> +
> +    qemu_free(d);
> +}
> +
>  void lance_init(NICInfo *nd, target_phys_addr_t leaddr, void *dma_opaque,
>                  qemu_irq irq, qemu_irq *reset)
>  {
>      PCNetState *d;
> -    int lance_io_memory;
>  
>      qemu_check_nic_model(nd, "lance");
>  
>      d = qemu_mallocz(sizeof(PCNetState));
>  
> -    lance_io_memory =
> +    d->mmio_index =
>          cpu_register_io_memory(0, lance_mem_read, lance_mem_write, d);
>  
>      d->dma_opaque = dma_opaque;
>  
> -    *reset = *qemu_allocate_irqs(parent_lance_reset, d, 1);
> +    d->reset_irq = qemu_allocate_irqs(parent_lance_reset, d, 1);
> +    *reset = *d->reset_irq;
>  
> -    cpu_register_physical_memory(leaddr, 4, lance_io_memory);
> +    cpu_register_physical_memory(leaddr, 4, d->mmio_index);
>  
>      d->irq = irq;
>      d->phys_mem_read = ledma_memory_read;
>      d->phys_mem_write = ledma_memory_write;
>  
>      pcnet_common_init(d, nd);
> +
> +    d->client->cleanup = lance_cleanup;
>  }
>  #endif /* TARGET_SPARC */
> diff --git a/qemu/hw/rtl8139.c b/qemu/hw/rtl8139.c
> index 9fa69db..e381ab0 100644
> --- a/qemu/hw/rtl8139.c
> +++ b/qemu/hw/rtl8139.c
> @@ -3414,6 +3414,25 @@ static void rtl8139_timer(void *opaque)
>  }
>  #endif /* RTL8139_ONBOARD_TIMER */
>  
> +static void rtl8139_cleanup(VLANClientState *vc)
> +{
> +    RTL8139State *s = vc->opaque;
> +
> +    if (s->cplus_txbuffer) {
> +        qemu_free(s->cplus_txbuffer);
> +        s->cplus_txbuffer = NULL;
> +    }
> +
> +#ifdef RTL8139_ONBOARD_TIMER
> +    qemu_del_timer(s->timer);
> +    qemu_free_timer(s->timer);
> +#endif
> +
> +    unregister_savevm("rtl8139", s);
> +
> +    cpu_unregister_io_memory(s->rtl8139_mmio_io_addr);
> +}
> +
>  PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
>  {
>      PCIRTL8139State *d;
> @@ -3451,6 +3470,7 @@ PCIDevice *pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
>      rtl8139_reset(s);
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   rtl8139_receive, rtl8139_can_receive, s);
> +    s->vc->cleanup = rtl8139_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  
> diff --git a/qemu/hw/smc91c111.c b/qemu/hw/smc91c111.c
> index f5b29a7..b132ca3 100644
> --- a/qemu/hw/smc91c111.c
> +++ b/qemu/hw/smc91c111.c
> @@ -42,6 +42,7 @@ typedef struct {
>      uint8_t int_level;
>      uint8_t int_mask;
>      uint8_t macaddr[6];
> +    int mmio_index;
>  } smc91c111_state;
>  
>  #define RCR_SOFT_RST  0x8000
> @@ -690,17 +691,24 @@ static CPUWriteMemoryFunc *smc91c111_writefn[] = {
>      smc91c111_writel
>  };
>  
> +static void smc91c111_cleanup(VLANClientState *vc)
> +{
> +    smc91c111_state *s = vc->opaque;
> +
> +    cpu_unregister_io_memory(s->mmio_index);
> +    qemu_free(s);
> +}
> +
>  void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
>  {
>      smc91c111_state *s;
> -    int iomemtype;
>  
>      qemu_check_nic_model(nd, "smc91c111");
>  
>      s = (smc91c111_state *)qemu_mallocz(sizeof(smc91c111_state));
> -    iomemtype = cpu_register_io_memory(0, smc91c111_readfn,
> -                                       smc91c111_writefn, s);
> -    cpu_register_physical_memory(base, 16, iomemtype);
> +    s->mmio_index = cpu_register_io_memory(0, smc91c111_readfn,
> +                                           smc91c111_writefn, s);
> +    cpu_register_physical_memory(base, 16, s->mmio_index);
>      s->irq = irq;
>      memcpy(s->macaddr, nd->macaddr, 6);
>  
> @@ -708,6 +716,7 @@ void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
>  
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   smc91c111_receive, smc91c111_can_receive, s);
> +    s->vc->cleanup = smc91c111_cleanup;
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>      /* ??? Save/restore.  */
>  }
> diff --git a/qemu/hw/stellaris_enet.c b/qemu/hw/stellaris_enet.c
> index 9d39169..a029607 100644
> --- a/qemu/hw/stellaris_enet.c
> +++ b/qemu/hw/stellaris_enet.c
> @@ -69,6 +69,7 @@ typedef struct {
>      VLANClientState *vc;
>      qemu_irq irq;
>      uint8_t macaddr[6];
> +    int mmio_index;
>  } stellaris_enet_state;
>  
>  static void stellaris_enet_update(stellaris_enet_state *s)
> @@ -384,22 +385,33 @@ static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> +static void stellaris_enet_cleanup(VLANClientState *vc)
> +{
> +    stellaris_enet_state *s = vc->opaque;
> +
> +    unregister_savevm("stellaris_enet", s);
> +
> +    cpu_unregister_io_memory(s->mmio_index);
> +
> +    qemu_free(s);
> +}
> +
>  void stellaris_enet_init(NICInfo *nd, uint32_t base, qemu_irq irq)
>  {
>      stellaris_enet_state *s;
> -    int iomemtype;
>  
>      qemu_check_nic_model(nd, "stellaris");
>  
>      s = (stellaris_enet_state *)qemu_mallocz(sizeof(stellaris_enet_state));
> -    iomemtype = cpu_register_io_memory(0, stellaris_enet_readfn,
> -                                       stellaris_enet_writefn, s);
> -    cpu_register_physical_memory(base, 0x00001000, iomemtype);
> +    s->mmio_index = cpu_register_io_memory(0, stellaris_enet_readfn,
> +                                           stellaris_enet_writefn, s);
> +    cpu_register_physical_memory(base, 0x00001000, s->mmio_index);
>      s->irq = irq;
>      memcpy(s->macaddr, nd->macaddr, 6);
>  
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   stellaris_enet_receive, stellaris_enet_can_receive, s);
> +    s->vc->cleanup = stellaris_enet_cleanup;
>      qemu_format_nic_info_str(s->vc, s->macaddr);
>  
>      stellaris_enet_reset(s);
> diff --git a/qemu/hw/usb-net.c b/qemu/hw/usb-net.c
> index 863c25f..99c132f 100644
> --- a/qemu/hw/usb-net.c
> +++ b/qemu/hw/usb-net.c
> @@ -1415,14 +1415,20 @@ static int usbnet_can_receive(void *opaque)
>      return !s->in_len;
>  }
>  
> +static void usbnet_cleanup(VLANClientState *vc)
> +{
> +    USBNetState *s = vc->opaque;
> +
> +    rndis_clear_responsequeue(s);
> +    qemu_free(s);
> +}
> +
>  static void usb_net_handle_destroy(USBDevice *dev)
>  {
>      USBNetState *s = (USBNetState *) dev;
>  
>      /* TODO: remove the nd_table[] entry */
>      qemu_del_vlan_client(s->vc);
> -    rndis_clear_responsequeue(s);
> -    qemu_free(s);
>  }
>  
>  USBDevice *usb_net_init(NICInfo *nd)
> @@ -1453,6 +1459,7 @@ USBDevice *usb_net_init(NICInfo *nd)
>                      "QEMU USB Network Interface");
>      s->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                      usbnet_receive, usbnet_can_receive, s);
> +    s->vc->cleanup = usbnet_cleanup;
>  
>      qemu_format_nic_info_str(s->vc, s->mac);
>  
> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
> index 094a2fb..650cc76 100644
> --- a/qemu/hw/virtio-net.c
> +++ b/qemu/hw/virtio-net.c
> @@ -570,6 +570,19 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> +static void virtio_net_cleanup(VLANClientState *vc)
> +{
> +    VirtIONet *n = vc->opaque;
> +
> +    unregister_savevm("virtio-net", n);
> +
> +    qemu_free(n->mac_table.macs);
> +    qemu_free(n->vlans);
> +
> +    qemu_del_timer(n->tx_timer);
> +    qemu_free_timer(n->tx_timer);
> +}
> +
>  PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
>  {
>      VirtIONet *n;
> @@ -599,6 +612,7 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
>      n->status = VIRTIO_NET_S_LINK_UP;
>      n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
>                                   virtio_net_receive, virtio_net_can_receive, n);
> +    n->vc->cleanup = virtio_net_cleanup;
>      n->vc->link_status_changed = virtio_net_set_link_status;
>  
>      qemu_format_nic_info_str(n->vc, n->mac);
> diff --git a/qemu/net.c b/qemu/net.c
> index e3abe89..2d1c1b4 100644
> --- a/qemu/net.c
> +++ b/qemu/net.c
> @@ -362,6 +362,8 @@ void qemu_del_vlan_client(VLANClientState *vc)
>      while (*pvc != NULL)
>          if (*pvc == vc) {
>              *pvc = vc->next;
> +            if (vc->cleanup)
> +                vc->cleanup(client);
>              free(vc->name);
>              free(vc->model);
>              free(vc);
> @@ -709,6 +711,8 @@ typedef struct TAPState {
>      char down_script_arg[128];
>  } TAPState;
>  
> +static int launch_script(const char *setup_script, const char *ifname, int fd);
> +
>  static ssize_t tap_receive_iov(void *opaque, const struct iovec *iov,
>                                 int iovcnt)
>  {
> @@ -775,6 +779,18 @@ static void tap_send(void *opaque)
>      }
>  }
>  
> +static void tap_cleanup(VLANClientState *vc)
> +{
> +    TAPState *s = vc->opaque;
> +
> +    if (s->down_script[0])
> +        launch_script(s->down_script, s->down_script_arg, s->fd);
> +
> +    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> +    close(s->fd);
> +    free(s);
> +}
> +
>  /* fd support */
>  
>  static TAPState *net_tap_fd_init(VLANState *vlan,
> @@ -787,6 +803,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan,
>      s = qemu_mallocz(sizeof(TAPState));
>      s->fd = fd;
>      s->vc = qemu_new_vlan_client(vlan, model, name, tap_receive, NULL, s);
> +    s->vc->cleanup = tap_cleanup;
>      s->vc->fd_readv = tap_receive_iov;
>      qemu_set_fd_handler2(s->fd, tap_can_send, tap_send, NULL, s);
>      snprintf(s->vc->info_str, sizeof(s->vc->info_str), "fd=%d", fd);
> @@ -1085,6 +1102,14 @@ static void vde_from_qemu(void *opaque, const uint8_t *buf, int size)
>      }
>  }
>  
> +static void vde_cleanup(VLANClientState *vc)
> +{
> +    VDEState *s = vc->opaque;
> +    qemu_set_fd_handler(vde_datafd(s->vde), NULL, NULL, NULL);
> +    vde_close(s->vde);
> +    free(s);
> +}
> +
>  static int net_vde_init(VLANState *vlan, const char *model,
>                          const char *name, const char *sock,
>                          int port, const char *group, int mode)
> @@ -1106,6 +1131,7 @@ static int net_vde_init(VLANState *vlan, const char *model,
>          return -1;
>      }
>      s->vc = qemu_new_vlan_client(vlan, model, name, vde_from_qemu, NULL, s);
> +    s->vc->cleanup = vde_cleanup;
>      qemu_set_fd_handler(vde_datafd(s->vde), vde_to_qemu, NULL, s);
>      snprintf(s->vc->info_str, sizeof(s->vc->info_str), "sock=%s,fd=%d",
>               sock, vde_datafd(s->vde));
> @@ -1290,6 +1316,14 @@ fail:
>      return -1;
>  }
>  
> +static void net_socket_cleanup(VLANClientState *vc)
> +{
> +    NetSocketState *s = vc->opaque;
> +    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> +    close(s->fd);
> +    free(s);
> +}
> +
>  static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
>                                                  const char *model,
>                                                  const char *name,
> @@ -1335,6 +1369,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
>      s->fd = fd;
>  
>      s->vc = qemu_new_vlan_client(vlan, model, name, net_socket_receive_dgram, NULL, s);
> +    s->vc->cleanup = net_socket_cleanup;
>      qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s);
>  
>      /* mcast: save bound address as dst */
> @@ -1363,6 +1398,7 @@ static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
>      s->fd = fd;
>      s->vc = qemu_new_vlan_client(vlan, model, name,
>                                   net_socket_receive, NULL, s);
> +    s->vc->cleanup = net_socket_cleanup;
>      snprintf(s->vc->info_str, sizeof(s->vc->info_str),
>               "socket: fd=%d", fd);
>      if (is_connected) {
> @@ -1922,29 +1958,20 @@ done:
>  
>  void net_cleanup(void)
>  {
> -#if !defined(_WIN32)
>      VLANState *vlan;
>  
>      /* close network clients */
>      for(vlan = first_vlan; vlan != NULL; vlan = vlan->next) {
> -        VLANClientState *vc;
> +        VLANClientState *vc = vlan->first_client;
>  
> -        for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
> -            if (vc->fd_read == tap_receive) {
> -                TAPState *s = vc->opaque;
> +        while (vc) {
> +            VLANClientState *next = vc->next;
>  
> -                if (s->down_script[0])
> -                    launch_script(s->down_script, s->down_script_arg, s->fd);
> -            }
> -#if defined(CONFIG_VDE)
> -            if (vc->fd_read == vde_from_qemu) {
> -                VDEState *s = vc->opaque;
> -                vde_close(s->vde);
> -            }
> -#endif
> +            qemu_del_vlan_client(vc);
> +
> +            vc = next;
>          }
>      }
> -#endif
>  }
>  
>  void net_client_check(void)
> diff --git a/qemu/net.h b/qemu/net.h
> index 1a51be7..5def263 100644
> --- a/qemu/net.h
> +++ b/qemu/net.h
> @@ -9,6 +9,7 @@ typedef ssize_t (IOReadvHandler)(void *, const struct iovec *, int);
>  
>  typedef struct VLANClientState VLANClientState;
>  
> +typedef void (NetCleanup) (VLANClientState *);
>  typedef void (LinkStatusChanged)(VLANClientState *);
>  
>  struct VLANClientState {
> @@ -17,6 +18,7 @@ struct VLANClientState {
>      /* Packets may still be sent if this returns zero.  It's used to
>         rate-limit the slirp code.  */
>      IOCanRWHandler *fd_can_read;
> +    NetCleanup *cleanup;
>      LinkStatusChanged *link_status_changed;
>      int link_down;
>      void *opaque;
> diff --git a/qemu/tap-win32.c b/qemu/tap-win32.c
> index e8a04dc..5948060 100644
> --- a/qemu/tap-win32.c
> +++ b/qemu/tap-win32.c
> @@ -638,6 +638,18 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
>       tap_win32_overlapped_t *handle;
>   } TAPState;
>  
> +static void tap_cleanup(VLANClientState *vc)
> +{
> +    TAPState *s = vc->opaque;
> +
> +    qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
> +
> +    /* FIXME: need to kill thread and close file handle:
> +       tap_win32_close(s);
> +    */
> +    free(s);
> +}
> +
>  static void tap_receive(void *opaque, const uint8_t *buf, int size)
>  {
>      TAPState *s = opaque;
> @@ -673,6 +685,7 @@ int tap_win32_init(VLANState *vlan, const char *model,
>      }
>  
>      s->vc = qemu_new_vlan_client(vlan, model, name, tap_receive, NULL, s);
> +    s->vc->cleanup = tap_cleanup;
>  
>      snprintf(s->vc->info_str, sizeof(s->vc->info_str),
>               "tap: ifname=%s", ifname);

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux