| Message ID | 57f8e18a.04321c0a.35f53.8b64@mx.google.com |
|---|---|
| State | New |
| Headers | show |
Am 08.10.2016 um 14:07 schrieb Li Qiang: > From: Li Qiang <liqiang6-s@360.cn> > > The exit dispatch of eepro100 network card device doesn't free > the 's->vmstate' field which was allocated in device realize thus > leading a host memory leak. This patch avoid this. > > Signed-off-by: Li Qiang <liqiang6-s@360.cn> Thank you for reporting this memory leak. I think that an even better solution would be avoiding the dynamic memory allocation. We could use this declaration for example: /* vmstate for each particular nic */ VMStateDescription vmstate; Do you want to prepare a new patch, or should I do it? Regards Stefan
Am 08.10.2016 um 18:19 schrieb Stefan Weil: > Am 08.10.2016 um 14:07 schrieb Li Qiang: >> From: Li Qiang <liqiang6-s@360.cn> >> >> The exit dispatch of eepro100 network card device doesn't free >> the 's->vmstate' field which was allocated in device realize thus >> leading a host memory leak. This patch avoid this. >> >> Signed-off-by: Li Qiang <liqiang6-s@360.cn> > > Thank you for reporting this memory leak. > > I think that an even better solution would be avoiding the dynamic > memory allocation. We could use this declaration for example: > > /* vmstate for each particular nic */ > VMStateDescription vmstate; > > Do you want to prepare a new patch, or should I do it? While thinking more about it, the solution used for e1000 looks better: vmstate could be a static const object, and the name field would always be "e100", no matter which specific nic was chosen. Stefan
Hello Stefan, I'm not familiar with the migration. In order not miss something, I think you can provide this patch. Thanks. 2016-10-09 0:43 GMT+08:00 Stefan Weil <sw@weilnetz.de>: > Am 08.10.2016 um 18:19 schrieb Stefan Weil: > >> Am 08.10.2016 um 14:07 schrieb Li Qiang: >> >>> From: Li Qiang <liqiang6-s@360.cn> >>> >>> The exit dispatch of eepro100 network card device doesn't free >>> the 's->vmstate' field which was allocated in device realize thus >>> leading a host memory leak. This patch avoid this. >>> >>> Signed-off-by: Li Qiang <liqiang6-s@360.cn> >>> >> >> Thank you for reporting this memory leak. >> >> I think that an even better solution would be avoiding the dynamic >> memory allocation. We could use this declaration for example: >> >> /* vmstate for each particular nic */ >> VMStateDescription vmstate; >> >> Do you want to prepare a new patch, or should I do it? >> > > While thinking more about it, the solution used for e1000 looks better: > vmstate could be a static const object, and the name field would always be > "e100", no matter which specific nic was chosen. > > Stefan > >
On 2016年10月08日 20:07, Li Qiang wrote: > From: Li Qiang <liqiang6-s@360.cn> > > The exit dispatch of eepro100 network card device doesn't free > the 's->vmstate' field which was allocated in device realize thus > leading a host memory leak. This patch avoid this. > > Signed-off-by: Li Qiang <liqiang6-s@360.cn> > --- > hw/net/eepro100.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c > index bab4dbf..4bf71f2 100644 > --- a/hw/net/eepro100.c > +++ b/hw/net/eepro100.c > @@ -1843,6 +1843,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev) > EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev); > > vmstate_unregister(&pci_dev->qdev, s->vmstate, s); > + g_free(s->vmstate); > eeprom93xx_free(&pci_dev->qdev, s->eeprom); > qemu_del_nic(s->nic); > } Applied, thanks. We may want to switch to use dc->vmsd instead of this in the future.
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c index bab4dbf..4bf71f2 100644 --- a/hw/net/eepro100.c +++ b/hw/net/eepro100.c @@ -1843,6 +1843,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev) EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev); vmstate_unregister(&pci_dev->qdev, s->vmstate, s); + g_free(s->vmstate); eeprom93xx_free(&pci_dev->qdev, s->eeprom); qemu_del_nic(s->nic); }