Am 10.09.2013 18:49, schrieb Paolo Bonzini: > Il 10/09/2013 18:21, Stefan Hajnoczi ha scritto: >> qdev_device_add() leaks the created qdev upon failure. I suspect this >> problem crept in because qdev_free() unparents the qdev but does not >> drop a reference - confusing name. > > Right, the name a leftover from pre-refcounting days. > > BTW, not dropping a reference is the right thing to do because the > reference is dropped much earlier, typically as soon as qdev_device_add > returns. The QOM object tree then will still provide means to access > devices, until they are unparented. > > In this case, however, qdev_device_add's caller does not have a > reference to free; doing that is the responsibility of qdev_device_add, > since it returns NULL. > >> Also drop trailing whitespace after curly bracket. >> >> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> >> --- >> qdev-monitor.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> index 410cdcb..5657cdc 100644 >> --- a/qdev-monitor.c >> +++ b/qdev-monitor.c >> @@ -512,6 +512,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) {
Given that qdev_free() doesn't do what one might expect, I would suggest to s/qdev_free/object_unparent/g above. >> @@ -523,8 +524,9 @@ DeviceState *qdev_device_add(QemuOpts *opts) >> object_property_add_child(qdev_get_peripheral_anon(), name, >> OBJECT(qdev), NULL); >> g_free(name); >> - } >> + } >> if (qdev_init(qdev) < 0) { >> + object_unref(OBJECT(qdev)); >> qerror_report(QERR_DEVICE_INIT_FAILED, driver); >> return NULL; >> } >> > > Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> I would like to take this through qom-next tree since I have pending variable cleanups there ("qdev" being touched here). Not sure how to handle that wrt block changes in this series? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg