Re: [PATCH 06/11] qdev: Add Error parameter to qdev_set_id()

object_property_add_child() fails (with &error_abort) if an object with the same name already exists. As long as QemuOpts is in use for -device and device_add, it catches duplicate IDs before qdev_set_id() is even called. However, for enabling non-QemuOpts code paths, we need to make sure that the condition doesn't cause a crash, but fails gracefully. Signed-off-by: Kevin Wolf <kwolf(a)redhat.com&gt; --- include/monitor/qdev.h | 2 +- hw/xen/xen-legacy-backend.c | 3 ++- softmmu/qdev-monitor.c | 16 ++++++++++------ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h index 389287eb44..7961308c75 100644 --- a/include/monitor/qdev.h +++ b/include/monitor/qdev.h @@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp); int qdev_device_help(QemuOpts *opts); DeviceState *qdev_device_add(QemuOpts *opts, Error **errp); -void qdev_set_id(DeviceState *dev, char *id); +void qdev_set_id(DeviceState *dev, char *id, Error **errp); #endif diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c index dd8ae1452d..17aca85ddc 100644 --- a/hw/xen/xen-legacy-backend.c +++ b/hw/xen/xen-legacy-backend.c @@ -276,7 +276,8 @@ static struct XenLegacyDevice *xen_be_get_xendev(const char *type, int dom, xendev = g_malloc0(ops->size); object_initialize(&xendev->qdev, ops->size, TYPE_XENBACKEND); OBJECT(xendev)->free = g_free; - qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev)); + qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev), + &error_abort); qdev_realize(DEVICE(xendev), xen_sysbus, &error_fatal); object_unref(OBJECT(xendev)); diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 1207e57a46..c2af906df0 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -593,26 +593,27 @@ static BusState *qbus_find(const char *path, Error **errp) } /* Takes ownership of @id, will be freed when deleting the device */ -void qdev_set_id(DeviceState *dev, char *id) +void qdev_set_id(DeviceState *dev, char *id, Error **errp)

{ if (id) { dev->id = id; }

if (dev->id) { - object_property_add_child(qdev_get_peripheral(), dev->id, - OBJECT(dev)); + object_property_try_add_child(qdev_get_peripheral(), dev->id, + OBJECT(dev), errp); } else { static int anon_count; gchar *name = g_strdup_printf("device[%d]", anon_count++); - object_property_add_child(qdev_get_peripheral_anon(), name, - OBJECT(dev)); + object_property_try_add_child(qdev_get_peripheral_anon(), name, + OBJECT(dev), errp); g_free(name); } } DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) { + ERRP_GUARD(); DeviceClass *dc; const char *driver, *path; DeviceState *dev = NULL; @@ -691,7 +692,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } } - qdev_set_id(dev, g_strdup(qemu_opts_id(opts))); + qdev_set_id(dev, g_strdup(qemu_opts_id(opts)), errp); + if (*errp) { + goto err_del_dev; + }

/* set properties */ if (qemu_opt_foreach(opts, set_property, dev, errp)) {