* [Qemu-devel] weird qdev error @ 2012-02-12 17:07 Michael S. Tsirkin 2012-02-12 17:31 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-12 17:07 UTC (permalink / raw) To: Anthony Liguori, qemu-devel I got this assert when working on qemu: pci hotplug callback failed so qdev_free was called. (gdb) where #0 0x00007ffff5fa1905 in raise () from /lib64/libc.so.6 #1 0x00007ffff5fa30e5 in abort () from /lib64/libc.so.6 #2 0x00007ffff7413a7f in g_assertion_message () from /lib64/libglib-2.0.so.0 #3 0x00007ffff7414020 in g_assertion_message_expr () from /lib64/libglib-2.0.so.0 #4 0x00007ffff7e452a9 in object_delete (obj=0x7ffff9124e60) at qom/object.c:375 #5 0x00007ffff7e2f5d4 in qdev_free (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:250 #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 #7 0x00007ffff7e2a7fe in qdev_device_add (opts=0x7ffff8b0d3a0) at /home/mst/scm/qemu/hw/qdev-monitor.c:473 #8 0x00007ffff7e06da9 in device_init_func (opts=<value optimized out>, opaque=<value optimized out>) at /home/mst/scm/qemu/vl.c:1754 #9 0x00007ffff7e3737a in qemu_opts_foreach (list=<value optimized out>, func= 0x7ffff7e06d90 <device_init_func>, opaque=0x0, abort_on_failure=<value optimized out>) at qemu-option.c:1048 #10 0x00007ffff7e09cdb in main (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3407 (gdb) frame 6 #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 149 qdev_free(dev); The problems seems to be that pci_qdev_init calls do_pci_unregister_device on hotplug error which will free the device twice? -- MST ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 17:07 [Qemu-devel] weird qdev error Michael S. Tsirkin @ 2012-02-12 17:31 ` Michael S. Tsirkin 2012-02-12 17:38 ` Anthony Liguori 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-12 17:31 UTC (permalink / raw) To: Anthony Liguori, qemu-devel On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote: > I got this assert when working on qemu: pci hotplug > callback failed so qdev_free was called. > > (gdb) where > #0 0x00007ffff5fa1905 in raise () from /lib64/libc.so.6 > #1 0x00007ffff5fa30e5 in abort () from /lib64/libc.so.6 > #2 0x00007ffff7413a7f in g_assertion_message () from > /lib64/libglib-2.0.so.0 > #3 0x00007ffff7414020 in g_assertion_message_expr () from > /lib64/libglib-2.0.so.0 > #4 0x00007ffff7e452a9 in object_delete (obj=0x7ffff9124e60) at > qom/object.c:375 > #5 0x00007ffff7e2f5d4 in qdev_free (dev=0x7ffff9124e60) > at /home/mst/scm/qemu/hw/qdev.c:250 > #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 > #7 0x00007ffff7e2a7fe in qdev_device_add (opts=0x7ffff8b0d3a0) > at /home/mst/scm/qemu/hw/qdev-monitor.c:473 > #8 0x00007ffff7e06da9 in device_init_func (opts=<value optimized out>, > opaque=<value optimized out>) at /home/mst/scm/qemu/vl.c:1754 > #9 0x00007ffff7e3737a in qemu_opts_foreach (list=<value optimized out>, > func= > 0x7ffff7e06d90 <device_init_func>, opaque=0x0, > abort_on_failure=<value optimized out>) at qemu-option.c:1048 > #10 0x00007ffff7e09cdb in main (argc=<value optimized out>, argv=<value > optimized out>, > envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3407 > (gdb) frame 6 > #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 > 149 qdev_free(dev); > > The problems seems to be that > pci_qdev_init calls do_pci_unregister_device on > hotplug error which will free the device twice? Here's a reproducer to a similar error in property parsing: qemu-system-x86_64 -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56 -netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on -vnc :1 -monitor stdio > -- > MST ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 17:31 ` Michael S. Tsirkin @ 2012-02-12 17:38 ` Anthony Liguori 2012-02-12 17:57 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Anthony Liguori @ 2012-02-12 17:38 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 2279 bytes --] On 02/12/2012 11:31 AM, Michael S. Tsirkin wrote: > On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote: >> I got this assert when working on qemu: pci hotplug >> callback failed so qdev_free was called. >> >> (gdb) where >> #0 0x00007ffff5fa1905 in raise () from /lib64/libc.so.6 >> #1 0x00007ffff5fa30e5 in abort () from /lib64/libc.so.6 >> #2 0x00007ffff7413a7f in g_assertion_message () from >> /lib64/libglib-2.0.so.0 >> #3 0x00007ffff7414020 in g_assertion_message_expr () from >> /lib64/libglib-2.0.so.0 >> #4 0x00007ffff7e452a9 in object_delete (obj=0x7ffff9124e60) at >> qom/object.c:375 >> #5 0x00007ffff7e2f5d4 in qdev_free (dev=0x7ffff9124e60) >> at /home/mst/scm/qemu/hw/qdev.c:250 >> #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 >> #7 0x00007ffff7e2a7fe in qdev_device_add (opts=0x7ffff8b0d3a0) >> at /home/mst/scm/qemu/hw/qdev-monitor.c:473 >> #8 0x00007ffff7e06da9 in device_init_func (opts=<value optimized out>, >> opaque=<value optimized out>) at /home/mst/scm/qemu/vl.c:1754 >> #9 0x00007ffff7e3737a in qemu_opts_foreach (list=<value optimized out>, >> func= >> 0x7ffff7e06d90<device_init_func>, opaque=0x0, >> abort_on_failure=<value optimized out>) at qemu-option.c:1048 >> #10 0x00007ffff7e09cdb in main (argc=<value optimized out>, argv=<value >> optimized out>, >> envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3407 >> (gdb) frame 6 >> #6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 >> 149 qdev_free(dev); >> >> The problems seems to be that >> pci_qdev_init calls do_pci_unregister_device on >> hotplug error which will free the device twice? > > Here's a reproducer to a similar error in property parsing: > > qemu-system-x86_64 -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2 > -netdev user,id=bar -net > nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir > tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56 > -netdev > tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on > -vnc :1 -monitor stdio Here's the fix. I need to do some regression testing and then I'll post as a proper top-level patch. Thanks for the report. Regards, Anthony Liguori > > > >> -- >> MST [-- Attachment #2: 0001-device_add-don-t-add-a-peripheral-link-until-init-is.patch --] [-- Type: text/x-patch, Size: 1637 bytes --] >From b7fc6f1eb7c5e041eac7d610061a1be950707e5b Mon Sep 17 00:00:00 2001 From: Anthony Liguori <aliguori@us.ibm.com> Date: Sun, 12 Feb 2012 11:36:24 -0600 Subject: [PATCH] device_add: don't add a /peripheral link until init is complete Otherwise we end up with a dangling reference which causes qdev_free() to fail. Reported-by: Michael Tsirkin <mst@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/qdev-monitor.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-) diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c index 49f13ca..a310cc7 100644 --- a/hw/qdev-monitor.c +++ b/hw/qdev-monitor.c @@ -457,6 +457,16 @@ DeviceState *qdev_device_add(QemuOpts *opts) id = qemu_opts_id(opts); if (id) { qdev->id = id; + } + if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) { + qdev_free(qdev); + return NULL; + } + if (qdev_init(qdev) < 0) { + qerror_report(QERR_DEVICE_INIT_FAILED, driver); + return NULL; + } + if (qdev->id) { object_property_add_child(qdev_get_peripheral(), qdev->id, OBJECT(qdev), NULL); } else { @@ -466,14 +476,6 @@ DeviceState *qdev_device_add(QemuOpts *opts) OBJECT(qdev), NULL); g_free(name); } - if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) { - qdev_free(qdev); - return NULL; - } - if (qdev_init(qdev) < 0) { - qerror_report(QERR_DEVICE_INIT_FAILED, driver); - return NULL; - } qdev->opts = opts; return qdev; } -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 17:38 ` Anthony Liguori @ 2012-02-12 17:57 ` Michael S. Tsirkin 2012-02-12 20:04 ` Anthony Liguori 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-12 17:57 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: > On 02/12/2012 11:31 AM, Michael S. Tsirkin wrote: > >On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote: > >>I got this assert when working on qemu: pci hotplug > >>callback failed so qdev_free was called. > >> > >>(gdb) where > >>#0 0x00007ffff5fa1905 in raise () from /lib64/libc.so.6 > >>#1 0x00007ffff5fa30e5 in abort () from /lib64/libc.so.6 > >>#2 0x00007ffff7413a7f in g_assertion_message () from > >>/lib64/libglib-2.0.so.0 > >>#3 0x00007ffff7414020 in g_assertion_message_expr () from > >>/lib64/libglib-2.0.so.0 > >>#4 0x00007ffff7e452a9 in object_delete (obj=0x7ffff9124e60) at > >>qom/object.c:375 > >>#5 0x00007ffff7e2f5d4 in qdev_free (dev=0x7ffff9124e60) > >> at /home/mst/scm/qemu/hw/qdev.c:250 > >>#6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 > >>#7 0x00007ffff7e2a7fe in qdev_device_add (opts=0x7ffff8b0d3a0) > >> at /home/mst/scm/qemu/hw/qdev-monitor.c:473 > >>#8 0x00007ffff7e06da9 in device_init_func (opts=<value optimized out>, > >> opaque=<value optimized out>) at /home/mst/scm/qemu/vl.c:1754 > >>#9 0x00007ffff7e3737a in qemu_opts_foreach (list=<value optimized out>, > >>func= > >> 0x7ffff7e06d90<device_init_func>, opaque=0x0, > >> abort_on_failure=<value optimized out>) at qemu-option.c:1048 > >>#10 0x00007ffff7e09cdb in main (argc=<value optimized out>, argv=<value > >>optimized out>, > >> envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3407 > >>(gdb) frame 6 > >>#6 qdev_init (dev=0x7ffff9124e60) at /home/mst/scm/qemu/hw/qdev.c:149 > >>149 qdev_free(dev); > >> > >>The problems seems to be that > >>pci_qdev_init calls do_pci_unregister_device on > >>hotplug error which will free the device twice? > > > >Here's a reproducer to a similar error in property parsing: > > > >qemu-system-x86_64 -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2 > >-netdev user,id=bar -net > >nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir > >tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56 > >-netdev > >tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on > >-vnc :1 -monitor stdio > > Here's the fix. I need to do some regression testing and then I'll > post as a proper top-level patch. > > Thanks for the report. > > Regards, > > Anthony Liguori > > > > > > > > >>-- > >>MST > > >From b7fc6f1eb7c5e041eac7d610061a1be950707e5b Mon Sep 17 00:00:00 2001 > From: Anthony Liguori <aliguori@us.ibm.com> > Date: Sun, 12 Feb 2012 11:36:24 -0600 > Subject: [PATCH] device_add: don't add a /peripheral link until init is complete > > Otherwise we end up with a dangling reference which causes qdev_free() to fail. > > Reported-by: Michael Tsirkin <mst@redhat.com> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> This handles the option parsing but what about hotplug failures (when bus->hotplug returns an error)? > --- > hw/qdev-monitor.c | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > index 49f13ca..a310cc7 100644 > --- a/hw/qdev-monitor.c > +++ b/hw/qdev-monitor.c > @@ -457,6 +457,16 @@ DeviceState *qdev_device_add(QemuOpts *opts) > id = qemu_opts_id(opts); > if (id) { > qdev->id = id; > + } > + if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) { > + qdev_free(qdev); > + return NULL; > + } > + if (qdev_init(qdev) < 0) { > + qerror_report(QERR_DEVICE_INIT_FAILED, driver); > + return NULL; > + } > + if (qdev->id) { > object_property_add_child(qdev_get_peripheral(), qdev->id, > OBJECT(qdev), NULL); > } else { > @@ -466,14 +476,6 @@ DeviceState *qdev_device_add(QemuOpts *opts) > OBJECT(qdev), NULL); > g_free(name); > } > - if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) { > - qdev_free(qdev); > - return NULL; > - } > - if (qdev_init(qdev) < 0) { > - qerror_report(QERR_DEVICE_INIT_FAILED, driver); > - return NULL; > - } > qdev->opts = opts; > return qdev; > } > -- > 1.7.4.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 17:57 ` Michael S. Tsirkin @ 2012-02-12 20:04 ` Anthony Liguori 2012-02-12 20:15 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Anthony Liguori @ 2012-02-12 20:04 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote: > On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: >> From: Anthony Liguori<aliguori@us.ibm.com> >> Date: Sun, 12 Feb 2012 11:36:24 -0600 >> Subject: [PATCH] device_add: don't add a /peripheral link until init is complete >> >> Otherwise we end up with a dangling reference which causes qdev_free() to fail. >> >> Reported-by: Michael Tsirkin<mst@redhat.com> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> > > This handles the option parsing but what about hotplug > failures (when bus->hotplug returns an error)? Sorry, I don't follow. The assert you reported was that object_free() noted a reference count of !0 which indicates something else was holding the reference to the object. In this case, it was the child link in /peripheral. By delaying creating the link in /peripheral, we eliminate the problem completely. BTW, the explicit calls to do_pci_unregister are redundant. finalize() will be called during cleanup which means exit() will be invoked (which already calls do_pci_unregister). I'm not sure why this isn't failing more aggressively but it looks clearly wrong to me. Regards, Anthony Liguori ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 20:04 ` Anthony Liguori @ 2012-02-12 20:15 ` Michael S. Tsirkin 2012-02-12 20:19 ` Anthony Liguori 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-12 20:15 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote: > On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote: > >On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: > >>From: Anthony Liguori<aliguori@us.ibm.com> > >>Date: Sun, 12 Feb 2012 11:36:24 -0600 > >>Subject: [PATCH] device_add: don't add a /peripheral link until init is complete > >> > >>Otherwise we end up with a dangling reference which causes qdev_free() to fail. > >> > >>Reported-by: Michael Tsirkin<mst@redhat.com> > >>Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> > > > >This handles the option parsing but what about hotplug > >failures (when bus->hotplug returns an error)? > > Sorry, I don't follow. > > The assert you reported was that object_free() noted a reference > count of !0 which indicates something else was holding the reference > to the object. In this case, it was the child link in /peripheral. > > By delaying creating the link in /peripheral, we eliminate the problem completely. Th other problem was internal in pci which calls ->hostplug during initialization. This doesn't seem affected? But I didn't try, maybe I misundertand. > BTW, the explicit calls to do_pci_unregister are redundant. > finalize() will be called during cleanup which means exit() will be > invoked (which already calls do_pci_unregister). I'm not sure why > this isn't failing more aggressively but it looks clearly wrong to > me. > > Regards, > > Anthony Liguori Me too. Want to try to drop them? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 20:15 ` Michael S. Tsirkin @ 2012-02-12 20:19 ` Anthony Liguori 2012-02-13 0:17 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Anthony Liguori @ 2012-02-12 20:19 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote: > On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote: >> On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote: >>> On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: >>>> From: Anthony Liguori<aliguori@us.ibm.com> >>>> Date: Sun, 12 Feb 2012 11:36:24 -0600 >>>> Subject: [PATCH] device_add: don't add a /peripheral link until init is complete >>>> >>>> Otherwise we end up with a dangling reference which causes qdev_free() to fail. >>>> >>>> Reported-by: Michael Tsirkin<mst@redhat.com> >>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >>> >>> This handles the option parsing but what about hotplug >>> failures (when bus->hotplug returns an error)? >> >> Sorry, I don't follow. >> >> The assert you reported was that object_free() noted a reference >> count of !0 which indicates something else was holding the reference >> to the object. In this case, it was the child link in /peripheral. >> >> By delaying creating the link in /peripheral, we eliminate the problem completely. > > Th other problem was internal in pci which calls ->hostplug > during initialization. This doesn't seem affected? > But I didn't try, maybe I misundertand. Yeah, from qdev's perspective it's all just init failing. hotplug is entirely a PCI concept. > >> BTW, the explicit calls to do_pci_unregister are redundant. >> finalize() will be called during cleanup which means exit() will be >> invoked (which already calls do_pci_unregister). I'm not sure why >> this isn't failing more aggressively but it looks clearly wrong to >> me. >> >> Regards, >> >> Anthony Liguori > > Me too. Want to try to drop them? Yeah, I'll make this a two patch series. Regards, Anthony Liguori > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-12 20:19 ` Anthony Liguori @ 2012-02-13 0:17 ` Michael S. Tsirkin 2012-02-13 1:18 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-13 0:17 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel On Sun, Feb 12, 2012 at 02:19:19PM -0600, Anthony Liguori wrote: > On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote: > >On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote: > >>On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote: > >>>On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: > >>>>From: Anthony Liguori<aliguori@us.ibm.com> > >>>>Date: Sun, 12 Feb 2012 11:36:24 -0600 > >>>>Subject: [PATCH] device_add: don't add a /peripheral link until init is complete > >>>> > >>>>Otherwise we end up with a dangling reference which causes qdev_free() to fail. > >>>> > >>>>Reported-by: Michael Tsirkin<mst@redhat.com> > >>>>Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> > >>> > >>>This handles the option parsing but what about hotplug > >>>failures (when bus->hotplug returns an error)? > >> > >>Sorry, I don't follow. > >> > >>The assert you reported was that object_free() noted a reference > >>count of !0 which indicates something else was holding the reference > >>to the object. In this case, it was the child link in /peripheral. > >> > >>By delaying creating the link in /peripheral, we eliminate the problem completely. > > > >Th other problem was internal in pci which calls ->hostplug > >during initialization. This doesn't seem affected? > >But I didn't try, maybe I misundertand. > > Yeah, from qdev's perspective it's all just init failing. hotplug > is entirely a PCI concept. > > > > >>BTW, the explicit calls to do_pci_unregister are redundant. > >>finalize() will be called during cleanup which means exit() will be > >>invoked (which already calls do_pci_unregister). I'm not sure why > >>this isn't failing more aggressively but it looks clearly wrong to > >>me. > >> > >>Regards, > >> > >>Anthony Liguori > > > >Me too. Want to try to drop them? > > Yeah, I'll make this a two patch series. > > Regards, > > Anthony Liguori I also see this: device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla device_del bla *** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64: corrupted double-linked list: 0x00007fae434565a0 *** Am I alone? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-13 0:17 ` Michael S. Tsirkin @ 2012-02-13 1:18 ` Michael S. Tsirkin 2012-02-13 4:58 ` Michael S. Tsirkin 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-13 1:18 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel On Mon, Feb 13, 2012 at 02:17:35AM +0200, Michael S. Tsirkin wrote: > On Sun, Feb 12, 2012 at 02:19:19PM -0600, Anthony Liguori wrote: > > On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote: > > >On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote: > > >>On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote: > > >>>On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote: > > >>>>From: Anthony Liguori<aliguori@us.ibm.com> > > >>>>Date: Sun, 12 Feb 2012 11:36:24 -0600 > > >>>>Subject: [PATCH] device_add: don't add a /peripheral link until init is complete > > >>>> > > >>>>Otherwise we end up with a dangling reference which causes qdev_free() to fail. > > >>>> > > >>>>Reported-by: Michael Tsirkin<mst@redhat.com> > > >>>>Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> > > >>> > > >>>This handles the option parsing but what about hotplug > > >>>failures (when bus->hotplug returns an error)? > > >> > > >>Sorry, I don't follow. > > >> > > >>The assert you reported was that object_free() noted a reference > > >>count of !0 which indicates something else was holding the reference > > >>to the object. In this case, it was the child link in /peripheral. > > >> > > >>By delaying creating the link in /peripheral, we eliminate the problem completely. > > > > > >Th other problem was internal in pci which calls ->hostplug > > >during initialization. This doesn't seem affected? > > >But I didn't try, maybe I misundertand. > > > > Yeah, from qdev's perspective it's all just init failing. hotplug > > is entirely a PCI concept. > > > > > > > >>BTW, the explicit calls to do_pci_unregister are redundant. > > >>finalize() will be called during cleanup which means exit() will be > > >>invoked (which already calls do_pci_unregister). I'm not sure why > > >>this isn't failing more aggressively but it looks clearly wrong to > > >>me. > > >> > > >>Regards, > > >> > > >>Anthony Liguori > > > > > >Me too. Want to try to drop them? > > > > Yeah, I'll make this a two patch series. > > > > Regards, > > > > Anthony Liguori > > > I also see this: > > device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla > device_del bla > *** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64: > corrupted double-linked list: 0x00007fae434565a0 *** > > Am I alone? > > Tried some tracing: I set breakpoint at g_free and gave the device_del command. (gdb) b g_free Breakpoint 3 at 0x7ffff73f55b0 (gdb) c Continuing. Program received signal SIGTSTP, Stopped (user). [Switching to Thread 0x7fffaf1a3700 (LWP 22749)] 0x00007ffff6ae074b in pthread_cond_timedwait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 (gdb) c Continuing. Program received signal SIGTSTP, Stopped (user). [Switching to Thread 0x7ffff4b28700 (LWP 22727)] 0x00007ffff6ae03cc in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 (gdb) continue Continuing. Program received signal SIGTSTP, Stopped (user). [Switching to Thread 0x7ffff7cb3700 (LWP 22712)] 0x00007ffff604c383 in select () from /lib64/libc.so.6 (gdb) continue Continuing. [Thread 0x7fffaf1a3700 (LWP 22749) exited] Breakpoint 3, 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 (gdb) continue Continuing. (qemu) device_del bla Breakpoint 3, 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 (gdb) where #0 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 #1 0x00007ffff7ec1d60 in monitor_parse_command (mon=0x7ffff8b207a0, cmdline=<value optimized out>, qdict=0x7ffff9161660) at /home/mst/scm/qemu/monitor.c:3724 #2 0x00007ffff7ec2684 in handle_user_command (mon=0x7ffff8b207a0, cmdline= 0x7ffff8c86a30 "device_del bla") at /home/mst/scm/qemu/monitor.c:3803 #3 0x00007ffff7ec2b6e in monitor_command_cb (mon=0x7ffff8b207a0, cmdline=<value optimized out>, opaque=<value optimized out>) at /home/mst/scm/qemu/monitor.c:4436 #4 0x00007ffff7e463ed in readline_handle_byte (rs=0x7ffff8c86a30, ch=<value optimized out>) at readline.c:370 #5 0x00007ffff7ec28b8 in monitor_read (opaque=<value optimized out>, buf= 0x7fffffffce20 "\r\023\f\366\377\177", size=1) at /home/mst/scm/qemu/monitor.c:4422 #6 0x00007ffff7e313bb in qemu_chr_be_write (opaque=0x7ffff8b0ca00) at qemu-char.c:163 #7 fd_chr_read (opaque=0x7ffff8b0ca00) at qemu-char.c:587 #8 0x00007ffff7d7c967 in qemu_iohandler_poll (readfds=0x7fffffffdfb0, writefds= 0x7fffffffdf30, xfds=<value optimized out>, ret=<value optimized out>) at iohandler.c:121 #9 0x00007ffff7e1085f in main_loop_wait (nonblocking=<value optimized out>) at main-loop.c:464 #10 0x00007ffff7e09284 in main_loop (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:1482 #11 main (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3525 (gdb) frame 1 #1 0x00007ffff7ec1d60 in monitor_parse_command (mon=0x7ffff8b207a0, cmdline=<value optimized out>, qdict=0x7ffff9161660) at /home/mst/scm/qemu/monitor.c:3724 3724 g_free(key); (gdb) p key $1 = 0x7ffff9166820 "id" (gdb) continue Continuing. Breakpoint 3, 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 (gdb) where #0 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 #1 0x00007ffff7e42fd4 in object_property_del (obj=0x7ffff8ca79d0, name=<value optimized out>, errp=<value optimized out>) at qom/object.c:629 #2 0x00007ffff7e4308d in object_property_del_child (obj=0x7ffff9123e50) at qom/object.c:310 #3 object_unparent (obj=0x7ffff9123e50) at qom/object.c:318 #4 0x00007ffff7de0a0d in pci_unplug_device (qdev=<value optimized out>) at /home/mst/scm/qemu/hw/pci.c:1521 #5 0x00007ffff7ec26b5 in handle_user_command (mon=0x7ffff8b207a0, cmdline=<value optimized out>) at /home/mst/scm/qemu/monitor.c:3813 #6 0x00007ffff7ec2b6e in monitor_command_cb (mon=0x7ffff8b207a0, cmdline=<value optimized out>, opaque=<value optimized out>) at /home/mst/scm/qemu/monitor.c:4436 #7 0x00007ffff7e463ed in readline_handle_byte (rs=0x7ffff8c86a30, ch=<value optimized out>) at readline.c:370 #8 0x00007ffff7ec28b8 in monitor_read (opaque=<value optimized out>, buf= 0x7fffffffce20 "\r\023\f\366\377\177", size=1) at /home/mst/scm/qemu/monitor.c:4422 #9 0x00007ffff7e313bb in qemu_chr_be_write (opaque=0x7ffff8b0ca00) at qemu-char.c:163 #10 fd_chr_read (opaque=0x7ffff8b0ca00) at qemu-char.c:587 #11 0x00007ffff7d7c967 in qemu_iohandler_poll (readfds=0x7fffffffdfb0, writefds= 0x7fffffffdf30, xfds=<value optimized out>, ret=<value optimized out>) at iohandler.c:121 #12 0x00007ffff7e1085f in main_loop_wait (nonblocking=<value optimized out>) at main-loop.c:464 #13 0x00007ffff7e09284 in main_loop (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:1482 #14 main (argc=<value optimized out>, argv=<value optimized out>, ---Type <return> to continue, or q <return> to quit--- envp=<value optimized out>) at /home/mst/scm/qemu/vl.c:3525 (gdb) frame 1 #1 0x00007ffff7e42fd4 in object_property_del (obj=0x7ffff8ca79d0, name=<value optimized out>, errp=<value optimized out>) at qom/object.c:629 629 g_free(prop->name); (gdb) p prop->name $2 = (gchar *) 0x7ffff9164510 "bla" (gdb) continue Continuing. Breakpoint 3, 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 (gdb) frame 1 #1 0x00007ffff7e42fdd in object_property_del (obj=0x7ffff8ca79d0, name=<value optimized out>, errp=<value optimized out>) at qom/object.c:630 630 g_free(prop->type); (gdb) p prop->type $3 = (gchar *) 0x7ffff9164580 "child<virtio-net-pci>" (gdb) continue Continuing. Breakpoint 3, 0x00007ffff73f55b0 in g_free () from /lib64/libglib-2.0.so.0 (gdb) frame 1 #1 0x00007ffff7e4308d in object_property_del_child (obj=0x7ffff9123e50) at qom/object.c:310 310 object_property_del(obj, prop->name, errp); (gdb) p prop->name $4 = (gchar *) 0x7ffff9164510 "\020h\026\371\377\177" >>>>>>>>>>>>>>>>>>>>>>>>>>>> It seems clear that there is at least a use after free here. >>>>>>>>>>>>>>>>>>>>>>>>>>>> This is not an immediate source of the crash, however. -- MST ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-13 1:18 ` Michael S. Tsirkin @ 2012-02-13 4:58 ` Michael S. Tsirkin 2012-02-13 10:19 ` Paolo Bonzini 0 siblings, 1 reply; 11+ messages in thread From: Michael S. Tsirkin @ 2012-02-13 4:58 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel On Mon, Feb 13, 2012 at 03:18:13AM +0200, Michael S. Tsirkin wrote: > > I also see this: > > > > device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla > > device_del bla > > *** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64: > > corrupted double-linked list: 0x00007fae434565a0 *** > > > > Am I alone? Doesn't solve this issue, but shouldn't we use _SAFE in object_property_del_child? Like this: ---> qemu: use safe list macro As we might remove an element from list, use the safe macro to walk it. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- diff --git a/qom/object.c b/qom/object.c index 5e5b261..8b64fb6 100644 --- a/qom/object.c +++ b/qom/object.c @@ -299,9 +299,9 @@ static void object_property_del_all(Object *obj) static void object_property_del_child(Object *obj, Object *child, Error **errp) { - ObjectProperty *prop; + ObjectProperty *prop, *next; - QTAILQ_FOREACH(prop, &obj->properties, node) { + QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) { if (!strstart(prop->type, "child<", NULL)) { continue; } ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] weird qdev error 2012-02-13 4:58 ` Michael S. Tsirkin @ 2012-02-13 10:19 ` Paolo Bonzini 0 siblings, 0 replies; 11+ messages in thread From: Paolo Bonzini @ 2012-02-13 10:19 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel On 02/13/2012 05:58 AM, Michael S. Tsirkin wrote: > Doesn't solve this issue, but shouldn't we use _SAFE > in object_property_del_child? Like this: > > ---> > qemu: use safe list macro > > As we might remove an element from list, use the safe macro > to walk it. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > diff --git a/qom/object.c b/qom/object.c > index 5e5b261..8b64fb6 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -299,9 +299,9 @@ static void object_property_del_all(Object *obj) > > static void object_property_del_child(Object *obj, Object *child, Error **errp) > { > - ObjectProperty *prop; > + ObjectProperty *prop, *next; > > - QTAILQ_FOREACH(prop, &obj->properties, node) { > + QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) { > if (!strstart(prop->type, "child<", NULL)) { > continue; > } Yup, I thought I had pointed this out in a review of Anthony's QOM series... Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-02-13 10:19 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-12 17:07 [Qemu-devel] weird qdev error Michael S. Tsirkin 2012-02-12 17:31 ` Michael S. Tsirkin 2012-02-12 17:38 ` Anthony Liguori 2012-02-12 17:57 ` Michael S. Tsirkin 2012-02-12 20:04 ` Anthony Liguori 2012-02-12 20:15 ` Michael S. Tsirkin 2012-02-12 20:19 ` Anthony Liguori 2012-02-13 0:17 ` Michael S. Tsirkin 2012-02-13 1:18 ` Michael S. Tsirkin 2012-02-13 4:58 ` Michael S. Tsirkin 2012-02-13 10:19 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).