Re: [Qemu-devel] [PATCH] eepro100: Fix memory leak and simplify code for VMStateDescription

[Dr. David Alan Gilbert]

* Stefan Weil (address@hidden) wrote:
> Instead of allocating a VMStateDescription for each NIC instance,
> the code now uses a single constant VMStateDescription for all
> instances. That implies that the name field is always the same.

Doesn't this break migration compatibility?

You might be able to get around that (in the forward direction only)
by adding an entry to qdev_alias_table but I'm not sure.

Dave

> Reported-by: Li Qiang <address@hidden>
> Signed-off-by: Stefan Weil <address@hidden>
> ---
>  hw/net/eepro100.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index bab4dbf..6f9777d 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -269,9 +269,6 @@ typedef struct {
>      /* Configuration bytes. */
>      uint8_t configuration[22];
>  
> -    /* vmstate for each particular nic */
> -    VMStateDescription *vmstate;
> -
>      /* Quasi static device properties (no need to save them). */
>      uint16_t stats_size;
>      bool has_extended_tcb_support;
> @@ -1787,7 +1784,8 @@ static ssize_t nic_receive(NetClientState *nc, const 
> uint8_t * buf, size_t size)
>      return size;
>  }
>  
> -static const VMStateDescription vmstate_eepro100 = {
> +static const VMStateDescription vmstate_e100 = {
> +    .name = "e100",
>      .version_id = 3,
>      .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
> @@ -1842,7 +1840,6 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
>  {
>      EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
>  
> -    vmstate_unregister(&pci_dev->qdev, s->vmstate, s);
>      eeprom93xx_free(&pci_dev->qdev, s->eeprom);
>      qemu_del_nic(s->nic);
>  }
> @@ -1892,11 +1889,6 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error 
> **errp)
>      TRACE(OTHER, logout("%s\n", qemu_get_queue(s->nic)->info_str));
>  
>      qemu_register_reset(nic_reset, s);
> -
> -    s->vmstate = g_malloc(sizeof(vmstate_eepro100));
> -    memcpy(s->vmstate, &vmstate_eepro100, sizeof(vmstate_eepro100));
> -    s->vmstate->name = qemu_get_queue(s->nic)->model;
> -    vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
>  }
>  
>  static void eepro100_instance_init(Object *obj)
> @@ -2083,6 +2075,7 @@ static void eepro100_class_init(ObjectClass *klass, 
> void *data)
>      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
>      dc->props = e100_properties;
>      dc->desc = info->desc;
> +    dc->vmsd = &vmstate_e100;
>      k->vendor_id = PCI_VENDOR_ID_INTEL;
>      k->class_id = PCI_CLASS_NETWORK_ETHERNET;
>      k->romfile = "pxe-eepro100.rom";
> -- 
> 2.1.4
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK