On Fri, 1 Feb 2013 18:38:19 +0100
Laszlo Ersek <ler...@redhat.com> wrote:
>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
> hw/qdev-monitor.h | 3 ++-
> hw/qdev-monitor.c | 29 +++++++++++++++--------------
> vl.c | 9 +++++++--
> 3 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/hw/qdev-monitor.h b/hw/qdev-monitor.h
> index 9ec4850..3760bf5 100644
> --- a/hw/qdev-monitor.h
> +++ b/hw/qdev-monitor.h
> @@ -3,6 +3,7 @@
>
> #include "qdev-core.h"
> #include "monitor/monitor.h"
> +#include "qapi/error.h"
>
> /*** monitor commands ***/
>
> @@ -11,6 +12,6 @@ void do_info_qdm(Monitor *mon, const QDict *qdict);
> int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
> int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
> int qdev_device_help(QemuOpts *opts);
> -DeviceState *qdev_device_add(QemuOpts *opts);
> +DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
>
> #endif
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 0c01b04..7e103af 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -417,7 +417,7 @@ static BusState *qbus_find(const char *path, Error **errp)
> }
> }
>
> -DeviceState *qdev_device_add(QemuOpts *opts)
> +DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> {
> ObjectClass *obj;
> DeviceClass *k;
> @@ -428,7 +428,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>
> driver = qemu_opt_get(opts, "driver");
> if (!driver) {
> - qerror_report(QERR_MISSING_PARAMETER, "driver");
> + error_set(errp, QERR_MISSING_PARAMETER, "driver");
> return NULL;
> }
>
> @@ -444,7 +444,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> }
>
> if (!obj) {
> - qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "driver", "device
> type");
This patch looks correct, just one small note: you're free to use error_setg()
and improve the error message for all error classes that are not part of
the @ErrorClass enum.
The example above, for example, could be improved I guess. As the real error
cause seems to be something like "driver name not found" and not
"Parameter 'driver' expects device type".
> return NULL;
> }
>
> @@ -455,13 +455,12 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> if (path != NULL) {
> bus = qbus_find(path, &local_err);
> if (!bus) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> + error_propagate(errp, local_err);
> return NULL;
> }
> if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {
> - qerror_report(QERR_BAD_BUS_FOR_DEVICE,
> - driver, object_get_typename(OBJECT(bus)));
> + error_set(errp, QERR_BAD_BUS_FOR_DEVICE,
> + driver, object_get_typename(OBJECT(bus)));
> return NULL;
> }
> } else {
> @@ -469,13 +468,12 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> &local_err);
> if (!bus) {
> assert(!error_is_set(&local_err));
> - qerror_report(QERR_NO_BUS_FOR_DEVICE,
> - k->bus_type, driver);
> + error_set(errp, QERR_NO_BUS_FOR_DEVICE, k->bus_type, driver);
> return NULL;
> }
> }
> if (qdev_hotplug && !bus->allow_hotplug) {
> - qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
> + error_set(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> return NULL;
> }
>
> @@ -496,8 +494,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> PropertyContext ctx = { .dev = qdev, .errp = &local_err };
>
> if (qemu_opt_foreach(opts, set_property, &ctx, 1) != 0) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> + error_propagate(errp, local_err);
> qdev_free(qdev);
> return NULL;
> }
> @@ -514,7 +511,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> g_free(name);
> }
> if (qdev_init(qdev) < 0) {
> - qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> + error_set(errp, QERR_DEVICE_INIT_FAILED, driver);
> return NULL;
> }
> qdev->opts = opts;
> @@ -627,7 +624,11 @@ int do_device_add(Monitor *mon, const QDict *qdict,
> QObject **ret_data)
> qemu_opts_del(opts);
> return 0;
> }
> - if (!qdev_device_add(opts)) {
> +
> + if (!qdev_device_add(opts, &local_err)) {
> + assert(error_is_set(&local_err));
> + qerror_report_err(local_err);
> + error_free(local_err);
> qemu_opts_del(opts);
> return -1;
> }
> diff --git a/vl.c b/vl.c
> index 44656d8..f2f8093 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2232,10 +2232,15 @@ static int device_help_func(QemuOpts *opts, void
> *opaque)
> static int device_init_func(QemuOpts *opts, void *opaque)
> {
> DeviceState *dev;
> + Error *local_err = NULL;
>
> - dev = qdev_device_add(opts);
> - if (!dev)
> + dev = qdev_device_add(opts, &local_err);
> + assert((dev == NULL) == error_is_set(&local_err));
> + if (!dev) {
> + qerror_report_err(local_err);
> + error_free(local_err);
> return -1;
> + }
> return 0;
> }
>
