Re: [Qemu-devel] weird qdev error

[Anthony Liguori <anthony@codemonkey.ws>]

[-- 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