Message ID | 1344407156-25562-16-git-send-email-qemulist@gmail.com |
---|---|
State | New |
Headers | show |
Il 08/08/2012 08:25, Liu Ping Fan ha scritto: > > +static void > +pci_e1000_unmap(PCIDevice *p) > +{ > + /* DO NOT FREE anything!until refcnt=0 */ > + /* isolate from memory view */ > +} At least you need to call the superclass method. Paolo > static int > pci_e1000_uninit(PCIDevice *dev) > { > @@ -1275,6 +1282,7 @@ static void e1000_class_init(ObjectClass *klass, void *data) > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > k->init = pci_e1000_init; > + k->unmap = pci_e1000_unmap; > k->exit = pci_e1000_uninit; > k->romfile = "pxe-e1000.rom"; > k->vendor_id = PCI_VENDOR_ID_INTEL;
On Wed, Aug 8, 2012 at 5:56 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 08/08/2012 08:25, Liu Ping Fan ha scritto: >> >> +static void >> +pci_e1000_unmap(PCIDevice *p) >> +{ >> + /* DO NOT FREE anything!until refcnt=0 */ >> + /* isolate from memory view */ >> +} > > At least you need to call the superclass method. > Refer to 0013-hotplug-introduce-qdev_unplug_complete-to-remove-dev.patch, we have the following sequence qdev_unmap->pci_unmap_device->pci_e1000_unmap. So pci_e1000_unmap need not to do anything. Regards, pingfan > Paolo > >> static int >> pci_e1000_uninit(PCIDevice *dev) >> { >> @@ -1275,6 +1282,7 @@ static void e1000_class_init(ObjectClass *klass, void *data) >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> >> k->init = pci_e1000_init; >> + k->unmap = pci_e1000_unmap; >> k->exit = pci_e1000_uninit; >> k->romfile = "pxe-e1000.rom"; >> k->vendor_id = PCI_VENDOR_ID_INTEL; > >
Il 09/08/2012 09:28, liu ping fan ha scritto: >>> >> +static void >>> >> +pci_e1000_unmap(PCIDevice *p) >>> >> +{ >>> >> + /* DO NOT FREE anything!until refcnt=0 */ >>> >> + /* isolate from memory view */ >>> >> +} >> > >> > At least you need to call the superclass method. >> > > Refer to 0013-hotplug-introduce-qdev_unplug_complete-to-remove-dev.patch, > we have the following sequence > qdev_unmap->pci_unmap_device->pci_e1000_unmap. So pci_e1000_unmap > need not to do anything. But then this patch is unnecessary, isn't it? Paolo
On Thu, Aug 9, 2012 at 3:40 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 09/08/2012 09:28, liu ping fan ha scritto: >>>> >> +static void >>>> >> +pci_e1000_unmap(PCIDevice *p) >>>> >> +{ >>>> >> + /* DO NOT FREE anything!until refcnt=0 */ >>>> >> + /* isolate from memory view */ >>>> >> +} >>> > >>> > At least you need to call the superclass method. >>> > >> Refer to 0013-hotplug-introduce-qdev_unplug_complete-to-remove-dev.patch, >> we have the following sequence >> qdev_unmap->pci_unmap_device->pci_e1000_unmap. So pci_e1000_unmap >> need not to do anything. > > But then this patch is unnecessary, isn't it? > Yes, should remove from next version Regards, pingfan > Paolo >
diff --git a/hw/e1000.c b/hw/e1000.c index 4573f13..fa71455 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -1192,6 +1192,13 @@ e1000_cleanup(VLANClientState *nc) s->nic = NULL; } +static void +pci_e1000_unmap(PCIDevice *p) +{ + /* DO NOT FREE anything!until refcnt=0 */ + /* isolate from memory view */ +} + static int pci_e1000_uninit(PCIDevice *dev) { @@ -1275,6 +1282,7 @@ static void e1000_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); k->init = pci_e1000_init; + k->unmap = pci_e1000_unmap; k->exit = pci_e1000_uninit; k->romfile = "pxe-e1000.rom"; k->vendor_id = PCI_VENDOR_ID_INTEL;