From 58346214d03ffcd774e86e3ce72b4196769eb710 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 2 Nov 2017 11:10:05 +0100 Subject: qdev_monitor: Simplify error handling in qdev_device_add() Instead of doing the clean-ups on errors multiple times, introduce a jump label at the end of the function that can be used by all error paths that need this cleanup. Suggested-by: Igor Mammedov Signed-off-by: Thomas Huth Message-Id: <1509617407-21191-2-git-send-email-thuth@redhat.com> Reviewed-by: Cornelia Huck Signed-off-by: Eduardo Habkost --- qdev-monitor.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'qdev-monitor.c') diff --git a/qdev-monitor.c b/qdev-monitor.c index b4abb4b5ea..2abb80d7e4 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -619,22 +619,22 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) /* set properties */ if (qemu_opt_foreach(opts, set_property, dev, &err)) { - error_propagate(errp, err); - object_unparent(OBJECT(dev)); - object_unref(OBJECT(dev)); - return NULL; + goto err_del_dev; } dev->opts = opts; object_property_set_bool(OBJECT(dev), true, "realized", &err); if (err != NULL) { - error_propagate(errp, err); dev->opts = NULL; - object_unparent(OBJECT(dev)); - object_unref(OBJECT(dev)); - return NULL; + goto err_del_dev; } return dev; + +err_del_dev: + error_propagate(errp, err); + object_unparent(OBJECT(dev)); + object_unref(OBJECT(dev)); + return NULL; } -- cgit v1.2.3 From 03fcbd9dc5084ff4676c153fbe04fb0fcf939d09 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 2 Nov 2017 11:10:06 +0100 Subject: qdev: Check for the availability of a hotplug controller before adding a device The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, so QEMU crashes when the user tries to device_add + device_del a device that does not have a corresponding hotplug controller. This could be provoked for a couple of devices in the past (see commit 4c93950659487c7ad or 84ebd3e8c7d4fe955 for example), and can currently for example also be triggered like this: $ s390x-softmmu/qemu-system-s390x -M none -nographic QEMU 2.10.50 monitor - type 'help' for more information (qemu) device_add qemu-s390x-cpu,id=x (qemu) device_del x ** ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl) Aborted (core dumped) So devices clearly need a hotplug controller when they should be usable with device_add. The code in qdev_device_add() already checks whether the bus has a proper hotplug controller, but for devices that do not have a corresponding bus, there is no appropriate check available yet. In that case we should check whether the machine itself provides a suitable hotplug controller and refuse to plug the device if none is available. Reviewed-by: Igor Mammedov Signed-off-by: Thomas Huth Message-Id: <1509617407-21191-3-git-send-email-thuth@redhat.com> Reviewed-by: Cornelia Huck Signed-off-by: Eduardo Habkost --- qdev-monitor.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'qdev-monitor.c') diff --git a/qdev-monitor.c b/qdev-monitor.c index 2abb80d7e4..c436616446 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -613,6 +613,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) if (bus) { qdev_set_parent_bus(dev, bus); + } else if (qdev_hotplug && !qdev_get_machine_hotplug_handler(dev)) { + /* No bus, no machine hotplug handler --> device is not hotpluggable */ + error_setg(&err, "Device '%s' can not be hotplugged on this machine", + driver); + goto err_del_dev; } qdev_set_id(dev, qemu_opts_id(opts)); -- cgit v1.2.3