Re: [Qemu-devel] [PATCH 6/7] qbus_find(): report errors via Error

[Luiz Capitulino]

On Fri,  1 Feb 2013 18:38:18 +0100
Laszlo Ersek <address@hidden> wrote:

> 
> Signed-off-by: Laszlo Ersek <address@hidden>

Splitting this as I suggested in the other patch would make review
easier. I honestly got a bit lost while reviewing this one.

> ---
>  hw/qdev-monitor.c |   29 +++++++++++++++--------------
>  1 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 83540fc..0c01b04 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -324,7 +324,7 @@ static BusState *qbus_find_recursive(BusState *bus, const 
> char *name,
>      return NULL;
>  }
>  
> -static BusState *qbus_find(const char *path)
> +static BusState *qbus_find(const char *path, Error **errp)
>  {
>      DeviceState *dev;
>      BusState *bus;
> @@ -336,20 +336,19 @@ static BusState *qbus_find(const char *path)
>          bus = sysbus_get_default();
>          pos = 0;
>      } else {
> -        Error *err = NULL;
> +        Error *find_err = NULL;
>  
>          if (sscanf(path, "%127[^/]%n", elem, &len) != 1) {
>              assert(!path[0]);
>              elem[0] = len = 0;
>          }
> -        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL, &err);
> +        bus = qbus_find_recursive(sysbus_get_default(), elem, NULL, 
> &find_err);
>          if (!bus) {
> -            if (error_is_set(&err)) {
> -                qerror_report_err(err);
> -                error_free(err);
> -            } else {
> -                qerror_report(QERR_BUS_NOT_FOUND, elem);
> -            }
> +            Error *not_found;
> +
> +            error_set(&not_found, QERR_BUS_NOT_FOUND, elem);
> +            error_propagate(errp, find_err);
> +            error_propagate(errp, not_found);
>              return NULL;
>          }
>          pos = len;
> @@ -372,7 +371,7 @@ static BusState *qbus_find(const char *path)
>          pos += len;
>          dev = qbus_find_dev(bus, elem);
>          if (!dev) {
> -            qerror_report(QERR_DEVICE_NOT_FOUND, elem);
> +            error_set(errp, QERR_DEVICE_NOT_FOUND, elem);
>              if (!monitor_cur_is_qmp()) {
>                  qbus_list_dev(bus);
>              }
> @@ -388,12 +387,12 @@ static BusState *qbus_find(const char *path)
>               * one child bus accept it nevertheless */
>              switch (dev->num_child_bus) {
>              case 0:
> -                qerror_report(QERR_DEVICE_NO_BUS, elem);
> +                error_set(errp, QERR_DEVICE_NO_BUS, elem);
>                  return NULL;
>              case 1:
>                  return QLIST_FIRST(&dev->child_bus);
>              default:
> -                qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
> +                error_set(errp, QERR_DEVICE_MULTIPLE_BUSSES, elem);
>                  if (!monitor_cur_is_qmp()) {
>                      qbus_list_bus(dev);
>                  }
> @@ -409,7 +408,7 @@ static BusState *qbus_find(const char *path)
>          pos += len;
>          bus = qbus_find_bus(dev, elem);
>          if (!bus) {
> -            qerror_report(QERR_BUS_NOT_FOUND, elem);
> +            error_set(errp, QERR_BUS_NOT_FOUND, elem);
>              if (!monitor_cur_is_qmp()) {
>                  qbus_list_bus(dev);
>              }
> @@ -454,8 +453,10 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      /* find bus */
>      path = qemu_opt_get(opts, "bus");
>      if (path != NULL) {
> -        bus = qbus_find(path);
> +        bus = qbus_find(path, &local_err);
>          if (!bus) {
> +            qerror_report_err(local_err);
> +            error_free(local_err);
>              return NULL;
>          }
>          if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {