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 <aligu...@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 <m...@redhat.com>
> Signed-off-by: Anthony Liguori <aligu...@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
>
