diff mbox

[PULL,47/58] qdev-monitor: Unref device when device_add fails

Message ID 1381254296-3203-48-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber Oct. 8, 2013, 5:44 p.m. UTC
From: Stefan Hajnoczi <stefanha@redhat.com>

qdev_device_add() leaks the created device upon failure.  I suspect this
problem crept in because qdev_free() unparents the device but does not
drop a reference - confusing name.

Cc: qemu-stable@nongnu.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 qdev-monitor.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Amos Kong Nov. 18, 2013, 12:29 p.m. UTC | #1
On Tue, Oct 08, 2013 at 07:44:45PM +0200, Andreas Färber wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> qdev_device_add() leaks the created device upon failure.  I suspect this
> problem crept in because qdev_free() unparents the device but does not
> drop a reference - confusing name.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Hi Stefan,

This commit caused a regression bug:

hotplug more than 32 disks to vm, qemu crash

---

[amos@amosk qemu]$ cat radd.sh 
for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
for j in `seq 1 7` 0;do
/bin/cp /images/none.qcow2 /tmp/resize$i$j.qcow2

echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none
echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none | nc -U /tmp/m

echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on
echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on | nc -U /tmp/m
done
done

----

#0  0x00005555558b7f95 in flatview_ref (view=0x0) at /home/devel/qemu/memory.c:300
#1  0x00005555558b9689 in address_space_get_flatview (as=0x55555645d660) at /home/devel/qemu/memory.c:656
#2  0x00005555558ba416 in address_space_update_topology (as=0x55555645d660) at /home/devel/qemu/memory.c:760
#3  0x00005555558ba5cf in memory_region_transaction_commit () at /home/devel/qemu/memory.c:799
#4  0x00005555558bcfcc in memory_region_set_enabled (mr=0x55555647af08, enabled=false) at /home/devel/qemu/memory.c:1503
#5  0x000055555571a0af in do_pci_register_device (pci_dev=0x55555647ac10, bus=0x5555564132b0, name=0x555556261100 "virtio-blk-pci", devfn=26) at hw/pci/pci.c:846
#6  0x000055555571c6cc in pci_qdev_init (qdev=0x55555647ac10) at hw/pci/pci.c:1751
#7  0x0000555555694d70 in device_realize (dev=0x55555647ac10, err=0x7fffffffc8e8) at hw/core/qdev.c:178
#8  0x00005555556966fc in device_set_realized (obj=0x55555647ac10, value=true, err=0x7fffffffca60) at hw/core/qdev.c:699
#9  0x00005555557e7b57 in property_set_bool (obj=0x55555647ac10, v=0x55555679a830, opaque=0x555556461b10, name=0x5555559922ae "realized", errp=0x7fffffffca60)
    at qom/object.c:1315
#10 0x00005555557e665b in object_property_set (obj=0x55555647ac10, v=0x55555679a830, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:803
#11 0x00005555557e816e in object_property_set_qobject (obj=0x55555647ac10, value=0x555556678880, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/qom-qobject.c:24
#12 0x00005555557e6950 in object_property_set_bool (obj=0x55555647ac10, value=true, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:866
#13 0x0000555555694ca7 in qdev_init (dev=0x55555647ac10) at hw/core/qdev.c:163
#14 0x00005555557c60ee in qdev_device_add (opts=0x555556525370) at qdev-monitor.c:543
#15 0x00005555557c6730 in do_device_add (mon=0x5555562fb760, qdict=0x55555645d440, ret_data=0x7fffffffcb80) at qdev-monitor.c:656
#16 0x00005555558c8892 in handle_user_command (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on")
    at /home/devel/qemu/monitor.c:4137
#17 0x00005555558ca10f in monitor_command_cb (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on", 
    opaque=0x0) at /home/devel/qemu/monitor.c:4757
#18 0x00005555557e9491 in readline_handle_byte (rs=0x5555563f0f60, ch=10) at readline.c:373
#19 0x00005555558ca045 in monitor_read (opaque=0x5555562fb760, buf=0x7fffffffccf0 "\n\315\377\377\377\177", size=1) at /home/devel/qemu/monitor.c:4743
#20 0x00005555557c6cc8 in qemu_chr_be_write (s=0x555556269040, buf=0x7fffffffccf0 "\n\315\377\377\377\177", len=1) at qemu-char.c:165
#21 0x00005555557cb026 in tcp_chr_read (chan=0x55555645fe40, cond=G_IO_IN, opaque=0x555556269040) at qemu-char.c:2487
#22 0x00007ffff76ede06 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#23 0x000055555578ef33 in glib_pollfds_poll () at main-loop.c:189
#24 0x000055555578f028 in os_host_main_loop_wait (timeout=77312299) at main-loop.c:234
#25 0x000055555578f100 in main_loop_wait (nonblocking=0) at main-loop.c:483
#26 0x000055555582e234 in main_loop () at vl.c:2014
#27 0x0000555555835697 in main (argc=14, argv=0x7fffffffe298, envp=0x7fffffffe310) at vl.c:4362

> ---
>  qdev-monitor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index b1ce26a..531b258 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -518,6 +518,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      }
>      if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>          qdev_free(qdev);
> +        object_unref(OBJECT(qdev));
>          return NULL;
>      }
>      if (qdev->id) {
> @@ -531,6 +532,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>          g_free(name);
>      }        
>      if (qdev_init(qdev) < 0) {
> +        object_unref(OBJECT(qdev));
>          qerror_report(QERR_DEVICE_INIT_FAILED, driver);
>          return NULL;
>      }
> -- 
> 1.8.1.4
>
Andreas Färber Nov. 18, 2013, 2:35 p.m. UTC | #2
Am 18.11.2013 13:29, schrieb Amos Kong:
> On Tue, Oct 08, 2013 at 07:44:45PM +0200, Andreas Färber wrote:
>> From: Stefan Hajnoczi <stefanha@redhat.com>
>>
>> qdev_device_add() leaks the created device upon failure.  I suspect this
>> problem crept in because qdev_free() unparents the device but does not
>> drop a reference - confusing name.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Hi Stefan,
> 
> This commit caused a regression bug:
> 
> hotplug more than 32 disks to vm, qemu crash
> 
> ---
> 
> [amos@amosk qemu]$ cat radd.sh 
> for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
> for j in `seq 1 7` 0;do
> /bin/cp /images/none.qcow2 /tmp/resize$i$j.qcow2
> 
> echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none
> echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none | nc -U /tmp/m
> 
> echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on
> echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on | nc -U /tmp/m
> done
> done

Hi, thanks for catching this.

Is this only with virtio-blk-pci or with any PCI card or only when
drives are involved? Either way it would be really great if you could
add such tests to Stefan's qtest using QMP, so that it can easily be run
by everyone.

The stacktrace below is not really telling to me. I wonder if we forget
to clean up some MemoryRegion in the device that still has a back
reference or whether the Memory API still references MemoryRegions that
have been destroyed by the device or forgets the reference devices it
still needs... Paolo?

I had reviewed the call paths and believe the patch to be 100% good, so
the fault will very likely be elsewhere.

Regards,
Andreas

> 
> ----
> 
> #0  0x00005555558b7f95 in flatview_ref (view=0x0) at /home/devel/qemu/memory.c:300
> #1  0x00005555558b9689 in address_space_get_flatview (as=0x55555645d660) at /home/devel/qemu/memory.c:656
> #2  0x00005555558ba416 in address_space_update_topology (as=0x55555645d660) at /home/devel/qemu/memory.c:760
> #3  0x00005555558ba5cf in memory_region_transaction_commit () at /home/devel/qemu/memory.c:799
> #4  0x00005555558bcfcc in memory_region_set_enabled (mr=0x55555647af08, enabled=false) at /home/devel/qemu/memory.c:1503
> #5  0x000055555571a0af in do_pci_register_device (pci_dev=0x55555647ac10, bus=0x5555564132b0, name=0x555556261100 "virtio-blk-pci", devfn=26) at hw/pci/pci.c:846
> #6  0x000055555571c6cc in pci_qdev_init (qdev=0x55555647ac10) at hw/pci/pci.c:1751
> #7  0x0000555555694d70 in device_realize (dev=0x55555647ac10, err=0x7fffffffc8e8) at hw/core/qdev.c:178
> #8  0x00005555556966fc in device_set_realized (obj=0x55555647ac10, value=true, err=0x7fffffffca60) at hw/core/qdev.c:699
> #9  0x00005555557e7b57 in property_set_bool (obj=0x55555647ac10, v=0x55555679a830, opaque=0x555556461b10, name=0x5555559922ae "realized", errp=0x7fffffffca60)
>     at qom/object.c:1315
> #10 0x00005555557e665b in object_property_set (obj=0x55555647ac10, v=0x55555679a830, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:803
> #11 0x00005555557e816e in object_property_set_qobject (obj=0x55555647ac10, value=0x555556678880, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/qom-qobject.c:24
> #12 0x00005555557e6950 in object_property_set_bool (obj=0x55555647ac10, value=true, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:866
> #13 0x0000555555694ca7 in qdev_init (dev=0x55555647ac10) at hw/core/qdev.c:163
> #14 0x00005555557c60ee in qdev_device_add (opts=0x555556525370) at qdev-monitor.c:543
> #15 0x00005555557c6730 in do_device_add (mon=0x5555562fb760, qdict=0x55555645d440, ret_data=0x7fffffffcb80) at qdev-monitor.c:656
> #16 0x00005555558c8892 in handle_user_command (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on")
>     at /home/devel/qemu/monitor.c:4137
> #17 0x00005555558ca10f in monitor_command_cb (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on", 
>     opaque=0x0) at /home/devel/qemu/monitor.c:4757
> #18 0x00005555557e9491 in readline_handle_byte (rs=0x5555563f0f60, ch=10) at readline.c:373
> #19 0x00005555558ca045 in monitor_read (opaque=0x5555562fb760, buf=0x7fffffffccf0 "\n\315\377\377\377\177", size=1) at /home/devel/qemu/monitor.c:4743
> #20 0x00005555557c6cc8 in qemu_chr_be_write (s=0x555556269040, buf=0x7fffffffccf0 "\n\315\377\377\377\177", len=1) at qemu-char.c:165
> #21 0x00005555557cb026 in tcp_chr_read (chan=0x55555645fe40, cond=G_IO_IN, opaque=0x555556269040) at qemu-char.c:2487
> #22 0x00007ffff76ede06 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
> #23 0x000055555578ef33 in glib_pollfds_poll () at main-loop.c:189
> #24 0x000055555578f028 in os_host_main_loop_wait (timeout=77312299) at main-loop.c:234
> #25 0x000055555578f100 in main_loop_wait (nonblocking=0) at main-loop.c:483
> #26 0x000055555582e234 in main_loop () at vl.c:2014
> #27 0x0000555555835697 in main (argc=14, argv=0x7fffffffe298, envp=0x7fffffffe310) at vl.c:4362
> 
>> ---
>>  qdev-monitor.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index b1ce26a..531b258 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -518,6 +518,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>>      }
>>      if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>>          qdev_free(qdev);
>> +        object_unref(OBJECT(qdev));
>>          return NULL;
>>      }
>>      if (qdev->id) {
>> @@ -531,6 +532,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>>          g_free(name);
>>      }        
>>      if (qdev_init(qdev) < 0) {
>> +        object_unref(OBJECT(qdev));
>>          qerror_report(QERR_DEVICE_INIT_FAILED, driver);
>>          return NULL;
>>      }
>> -- 
>> 1.8.1.4
Amos Kong Nov. 19, 2013, 8:31 a.m. UTC | #3
On Mon, Nov 18, 2013 at 03:35:20PM +0100, Andreas Färber wrote:
> Am 18.11.2013 13:29, schrieb Amos Kong:
> > On Tue, Oct 08, 2013 at 07:44:45PM +0200, Andreas Färber wrote:
> >> From: Stefan Hajnoczi <stefanha@redhat.com>
> >>
> >> qdev_device_add() leaks the created device upon failure.  I suspect this
> >> problem crept in because qdev_free() unparents the device but does not
> >> drop a reference - confusing name.
> >>
> >> Cc: qemu-stable@nongnu.org
> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> > 
> > Hi Stefan,
> > 
> > This commit caused a regression bug:
> > 
> > hotplug more than 32 disks to vm, qemu crash
> > 
> > ---
> > 
> > [amos@amosk qemu]$ cat radd.sh 
> > for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
> > for j in `seq 1 7` 0;do
> > /bin/cp /images/none.qcow2 /tmp/resize$i$j.qcow2
> > 
> > echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none
> > echo drive_add $i.$j id=drv$i$j,file=/tmp/resize$i$j.qcow2,if=none | nc -U /tmp/m
> > 
> > echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on
> > echo device_add virtio-blk-pci,id=dev$i$j,drive=drv$i$j,addr=0x$i.$j,multifunction=on | nc -U /tmp/m
> > done
> > done
> 
> Hi, thanks for catching this.
> 
> Is this only with virtio-blk-pci or with any PCI card or only when
> drives are involved?

I can reproduce by hotplugging virtio-net-pci NIC

for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
for j in `seq 1 7` 0;do

echo "netdev_add tap,id=dev$i$j" | nc -U /tmp/m
echo "device_add virtio-net-pci,netdev=dev$i$j,id=nic$i$j,addr=0x$i.$j,multifunction=on" | nc -U /tmp/m

done
done


> Either way it would be really great if you could
> add such tests to Stefan's qtest using QMP, so that it can easily be run
> by everyone.

I will do it.
 
> The stacktrace below is not really telling to me. I wonder if we forget
> to clean up some MemoryRegion in the device that still has a back
> reference or whether the Memory API still references MemoryRegions that
> have been destroyed by the device or forgets the reference devices it
> still needs... Paolo?
> 
> I had reviewed the call paths and believe the patch to be 100% good, so
> the fault will very likely be elsewhere.
> 
> Regards,
> Andreas
> 
> > 
> > ----
> > 
> > #0  0x00005555558b7f95 in flatview_ref (view=0x0) at /home/devel/qemu/memory.c:300
> > #1  0x00005555558b9689 in address_space_get_flatview (as=0x55555645d660) at /home/devel/qemu/memory.c:656
> > #2  0x00005555558ba416 in address_space_update_topology (as=0x55555645d660) at /home/devel/qemu/memory.c:760
> > #3  0x00005555558ba5cf in memory_region_transaction_commit () at /home/devel/qemu/memory.c:799
> > #4  0x00005555558bcfcc in memory_region_set_enabled (mr=0x55555647af08, enabled=false) at /home/devel/qemu/memory.c:1503
> > #5  0x000055555571a0af in do_pci_register_device (pci_dev=0x55555647ac10, bus=0x5555564132b0, name=0x555556261100 "virtio-blk-pci", devfn=26) at hw/pci/pci.c:846
> > #6  0x000055555571c6cc in pci_qdev_init (qdev=0x55555647ac10) at hw/pci/pci.c:1751
> > #7  0x0000555555694d70 in device_realize (dev=0x55555647ac10, err=0x7fffffffc8e8) at hw/core/qdev.c:178
> > #8  0x00005555556966fc in device_set_realized (obj=0x55555647ac10, value=true, err=0x7fffffffca60) at hw/core/qdev.c:699
> > #9  0x00005555557e7b57 in property_set_bool (obj=0x55555647ac10, v=0x55555679a830, opaque=0x555556461b10, name=0x5555559922ae "realized", errp=0x7fffffffca60)
> >     at qom/object.c:1315
> > #10 0x00005555557e665b in object_property_set (obj=0x55555647ac10, v=0x55555679a830, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:803
> > #11 0x00005555557e816e in object_property_set_qobject (obj=0x55555647ac10, value=0x555556678880, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/qom-qobject.c:24
> > #12 0x00005555557e6950 in object_property_set_bool (obj=0x55555647ac10, value=true, name=0x5555559922ae "realized", errp=0x7fffffffca60) at qom/object.c:866
> > #13 0x0000555555694ca7 in qdev_init (dev=0x55555647ac10) at hw/core/qdev.c:163
> > #14 0x00005555557c60ee in qdev_device_add (opts=0x555556525370) at qdev-monitor.c:543
> > #15 0x00005555557c6730 in do_device_add (mon=0x5555562fb760, qdict=0x55555645d440, ret_data=0x7fffffffcb80) at qdev-monitor.c:656
> > #16 0x00005555558c8892 in handle_user_command (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on")
> >     at /home/devel/qemu/monitor.c:4137
> > #17 0x00005555558ca10f in monitor_command_cb (mon=0x5555562fb760, cmdline=0x5555563f0f60 "device_add virtio-blk-pci,id=dev32,drive=drv32,addr=0x3.2,multifunction=on", 
> >     opaque=0x0) at /home/devel/qemu/monitor.c:4757
> > #18 0x00005555557e9491 in readline_handle_byte (rs=0x5555563f0f60, ch=10) at readline.c:373
> > #19 0x00005555558ca045 in monitor_read (opaque=0x5555562fb760, buf=0x7fffffffccf0 "\n\315\377\377\377\177", size=1) at /home/devel/qemu/monitor.c:4743
> > #20 0x00005555557c6cc8 in qemu_chr_be_write (s=0x555556269040, buf=0x7fffffffccf0 "\n\315\377\377\377\177", len=1) at qemu-char.c:165
> > #21 0x00005555557cb026 in tcp_chr_read (chan=0x55555645fe40, cond=G_IO_IN, opaque=0x555556269040) at qemu-char.c:2487
> > #22 0x00007ffff76ede06 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
> > #23 0x000055555578ef33 in glib_pollfds_poll () at main-loop.c:189
> > #24 0x000055555578f028 in os_host_main_loop_wait (timeout=77312299) at main-loop.c:234
> > #25 0x000055555578f100 in main_loop_wait (nonblocking=0) at main-loop.c:483
> > #26 0x000055555582e234 in main_loop () at vl.c:2014
> > #27 0x0000555555835697 in main (argc=14, argv=0x7fffffffe298, envp=0x7fffffffe310) at vl.c:4362
> > 
> >> ---
> >>  qdev-monitor.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/qdev-monitor.c b/qdev-monitor.c
> >> index b1ce26a..531b258 100644
> >> --- a/qdev-monitor.c
> >> +++ b/qdev-monitor.c
> >> @@ -518,6 +518,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >>      }
> >>      if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
> >>          qdev_free(qdev);
> >> +        object_unref(OBJECT(qdev));
> >>          return NULL;
> >>      }
> >>      if (qdev->id) {
> >> @@ -531,6 +532,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >>          g_free(name);
> >>      }        
> >>      if (qdev_init(qdev) < 0) {
> >> +        object_unref(OBJECT(qdev));
> >>          qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> >>          return NULL;
> >>      }
> >> -- 
> >> 1.8.1.4
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Paolo Bonzini Nov. 19, 2013, 10:25 a.m. UTC | #4
Il 19/11/2013 09:31, Amos Kong ha scritto:
> I can reproduce by hotplugging virtio-net-pci NIC
> 
> for i in `seq 3 9` a b c d e f 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f;do
> for j in `seq 1 7` 0;do
> 
> echo "netdev_add tap,id=dev$i$j" | nc -U /tmp/m
> echo "device_add virtio-net-pci,netdev=dev$i$j,id=nic$i$j,addr=0x$i.$j,multifunction=on" | nc -U /tmp/m
> 
> done
> done

Do you have the full command line?

Also, why/where is device_add failing?  Can you add a puts(driver); before the unrefs?

I tried something very similar to the above, with commands like

netdev_add bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=dev1f0
device_add virtio-net-pci,netdev=dev1f0,id=nic1f0,addr=0x1f.0,multifunction=on

and it hotplugged all 224 devices this way without any failure.

Did you try running with MALLOC_PERTURB_=42
diff mbox

Patch

diff --git a/qdev-monitor.c b/qdev-monitor.c
index b1ce26a..531b258 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -518,6 +518,7 @@  DeviceState *qdev_device_add(QemuOpts *opts)
     }
     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
         qdev_free(qdev);
+        object_unref(OBJECT(qdev));
         return NULL;
     }
     if (qdev->id) {
@@ -531,6 +532,7 @@  DeviceState *qdev_device_add(QemuOpts *opts)
         g_free(name);
     }        
     if (qdev_init(qdev) < 0) {
+        object_unref(OBJECT(qdev));
         qerror_report(QERR_DEVICE_INIT_FAILED, driver);
         return NULL;
     }