Patchwork [15/15] e1000: using new interface--unmap to unplug

login
register
mail settings
Submitter pingfan liu
Date Aug. 8, 2012, 6:25 a.m.
Message ID <1344407156-25562-16-git-send-email-qemulist@gmail.com>
Download mbox | patch
Permalink /patch/175860/
State New
Headers show

Comments

pingfan liu - Aug. 8, 2012, 6:25 a.m.
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 hw/e1000.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Paolo Bonzini - Aug. 8, 2012, 9:56 a.m.
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;
pingfan liu - Aug. 9, 2012, 7:28 a.m.
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;
>
>
Paolo Bonzini - Aug. 9, 2012, 7:40 a.m.
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
pingfan liu - Aug. 10, 2012, 6:43 a.m.
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
>

Patch

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;