qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path fu


From: Gonglei (Arei)
Subject: Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function
Date: Thu, 4 Sep 2014 06:15:53 +0000

Hi,

> > Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function
> >
> > On Wed, Sep 03, 2014 at 06:45:56AM +0000, Gonglei (Arei) wrote:
> > [...]
> > > > > 4. When we hotplug the virtio-net-pci device, only pass 
> > > > > virtio-net-pci's
> > pointer
> > > > to
> > > > > del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I 
> > > > > add a
> > > > function
> > > > > named is_same_fw_dev_path() to handle this situation.
> > > >
> > > > When hot-unplugging virtio-net-pci I'd expect we free both
> > > > virtio-net-pci and virtio-net-device (and therefore call
> > > > del_boot_device_path twice, once for each device).  Can you check that?
> > > >
> > > Yes, I can.
> > >
> > > The del_boot_device_path() is called only once, just for virtio-net-pci.
> > > For its child, virtio-net-devcie's resource is cleaned by qbus->child
> unrealizing
> > > process, will not call device_finalize().
> >
> > Then we need to fix this to make sure there's a corresponding
> > del_boot_device_path() call (with the same pointer) to every
> > add_boot_device_path() call, instead of adding a hack to
> > del_boot_device_path().
> >
> Good idea. We can add functions named $device_finalize_bootindex(), such as:
> 
> static void virtio_net_set_bootindex(Object *obj, Visitor *v, void *opaque,
>                                      const char *name, Error **errp)
> {
>     VirtIONet *n = VIRTIO_NET(obj);
> 
>     set_bootindex(&n->nic_conf, v, name, errp);
>     add_boot_device_path(n->nic_conf.bootindex,
>                          DEVICE(obj), "/address@hidden");
> }
> 
> static void virtio_net_finalize_bootindex(Object *obj, const char *name,
>                                           void *opaque)
> {
>     del_boot_device_path(DEVICE(obj));
> }
> ...

Oops, I found an issue that when we hot-unplug a virtio-net-pci device,
the virtio_net_finalize_bootindex function will not be called too.
Maybe this is a potential *bug* which will cause the virtio-net-device's 
property
resource leak. 

Paolo, would you have a look at it? Thanks a lot!

Backtrace as below:

#0  object_unref (obj=0x7f207c640468) at qom/object.c:711
#1  0x00007f207b9850da in bus_remove_child (bus=0x7f207c6403f0, 
child=0x7f207c640468) at hw/core/qdev.c:76
#2  0x00007f207b987cbc in device_unparent (obj=0x7f207c640468) at 
hw/core/qdev.c:1013
#3  0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c63fa90, 
name=0x7f207c6027c0 "virtio-backend", opaque=
    0x7f207c640468) at qom/object.c:1036
#4  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90, 
name=0x7f207c6027c0 "virtio-backend", errp=0x0)
    at qom/object.c:778
#5  0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90, 
child=0x7f207c640468, errp=0x0) at qom/object.c:382
#6  0x00007f207ba9aa83 in object_unparent (obj=0x7f207c640468) at 
qom/object.c:391
#7  0x00007f207b986666 in bus_unparent (obj=0x7f207c6403f0) at 
hw/core/qdev.c:548
#8  0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c63fa90, 
name=0x7f207c5db7a0 "virtio-bus", opaque=
    0x7f207c6403f0) at qom/object.c:1036
#9  0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c63fa90, 
name=0x7f207c5db7a0 "virtio-bus", errp=0x0) at qom/object.c:778
#10 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c63fa90, 
child=0x7f207c6403f0, errp=0x0) at qom/object.c:382
#11 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c6403f0) at 
qom/object.c:391
#12 0x00007f207b987c8f in device_unparent (obj=0x7f207c63fa90) at 
hw/core/qdev.c:1010
#13 0x00007f207ba9c5a3 in object_finalize_child_property (obj=0x7f207c4f2f90, 
name=0x7f207c604580 "nic1", opaque=0x7f207c63fa90)
    at qom/object.c:1036
#14 0x00007f207ba9b9a9 in object_property_del (obj=0x7f207c4f2f90, 
name=0x7f207c604580 "nic1", errp=0x0) at qom/object.c:778
#15 0x00007f207ba9a9ca in object_property_del_child (obj=0x7f207c4f2f90, 
child=0x7f207c63fa90, errp=0x0) at qom/object.c:382
#16 0x00007f207ba9aa83 in object_unparent (obj=0x7f207c63fa90) at 
qom/object.c:391
#17 0x00007f207b94f7c7 in acpi_pcihp_eject_slot (s=0x7f207c5801e8, bsel=0, 
slots=8) at hw/acpi/pcihp.c:139
#18 0x00007f207b94fe65 in pci_write (opaque=0x7f207c5801e8, addr=8, data=8, 
size=4) at hw/acpi/pcihp.c:277
#19 0x00007f207b818a38 in memory_region_write_accessor (mr=0x7f207c580df8, 
addr=8, value=0x7f2075886b38, size=4, shift=0, mask=
    4294967295) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:443
#20 0x00007f207b818b74 in access_with_adjusted_size (addr=8, 
value=0x7f2075886b38, size=4, access_size_min=1, access_size_max=4, 
    access=0x7f207b8189ab <memory_region_write_accessor>, mr=0x7f207c580df8) at 
/mnt/sdb/gonglei/qemu.git/qemu/memory.c:480
#21 0x00007f207b81c141 in memory_region_dispatch_write (mr=0x7f207c580df8, 
addr=8, data=8, size=4)
    at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1137
#22 0x00007f207b81fcf0 in io_mem_write (mr=0x7f207c580df8, addr=8, val=8, 
size=4) at /mnt/sdb/gonglei/qemu.git/qemu/memory.c:1973
#23 0x00007f207b7ca289 in address_space_rw (as=0x7f207bfeb1e0 
<address_space_io>, addr=44552, buf=0x7f207b719000 "\b", len=4, 
    is_write=true) at /mnt/sdb/gonglei/qemu.git/qemu/exec.c:2042
#24 0x00007f207b815266 in kvm_handle_io (port=44552, data=0x7f207b719000, 
direction=1, size=4, count=1)
    at /mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1597
#25 0x00007f207b8158a4 in kvm_cpu_exec (cpu=0x7f207c4f6610) at 
/mnt/sdb/gonglei/qemu.git/qemu/kvm-all.c:1748
#26 0x00007f207b7fcd68 in qemu_kvm_cpu_thread_fn (arg=0x7f207c4f6610) at 
/mnt/sdb/gonglei/qemu.git/qemu/cpus.c:940
#27 0x00007f2078a357f6 in start_thread () from /lib64/libpthread.so.0
#28 0x00007f207879109d in clone () from /lib64/libc.so.6
#29 0x0000000000000000 in ?? ()
(gdb) p *obj
$64 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first = 
0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 3, parent = 
    0x7f207c63fa90}
(gdb) n
712         if (!obj) {
(gdb) 
715         g_assert(obj->ref > 0);
(gdb) 
718         if (atomic_fetch_dec(&obj->ref) == 1) {
(gdb) 
721     }
(gdb) p *obj
$65 = {class = 0x7f207c4bc5f0, free = 0x0, properties = {tqh_first = 
0x7f207c591fd0, tqh_last = 0x7f207c6027a8}, ref = 2, parent = 
    0x7f207c63fa90}

Because of the virtio-net-device object's ref is 3, will not enter 
object_finailze(), *leak resource*. Am I wrong?

Best regards,
-Gonglei

> object_property_add(obj, "bootindex", "int",
>                         virtio_net_get_bootindex,
>                         virtio_net_set_bootindex,
>                         virtio_net_finalize_bootindex, dev, NULL);
> 
> as the previous email, we lay add_boot_device_path() in
> $device_set_bootindex,
> and lay del_boot_device_path() in $device_finalize_bootindex is a good idea
> IMO.
> 
> Do you like it? Thanks.
> 
> Best regards,
> -Gonglei



reply via email to

[Prev in Thread] Current Thread [Next in Thread]