Re: [Qemu-devel] [RFC 2/2] qbus_find_recursive(): the "free slots" constraint needs a dedicated error

[Laszlo Ersek]

On 01/31/13 17:55, Peter Maydell wrote:

> You probably need to pass in an Error** then, so the leaf can
> provide useful information at the point of error and then the
> top level code can deal with it appropriately whether hmp or qmp.

That's what I'm doing, yes.

> For Error** error_setg() is the right way to specify arbitrary
> error text I think.

The following call chain illustrates the problem. Suppose
monitor_cur_is_qmp() returns false.

do_device_add
  qdev_device_add
    qbus_find
      qbus_find_recursive
        qerror_report(ERROR_CLASS_GENERIC_ERROR)
          qerror_print
            error_vprintf
      qerror_report(QERR_BUS_NOT_FOUND)
        qerror_print
          error_vprintf

Originally, when qbus_find_recursive() returns NULL to qbus_find(),
QERR_BUS_NOT_FOUND is reported.

This was not specific enough for Fred's case, so he extended the error
for one particular case inside qbus_find_recursive(). In that case
ERROR_CLASS_GENERIC_ERROR is generated *in addition*.

If monitor_cur_is_qmp() is currently false, both messages are printed,
either to the monitor or stderr. (qbus_find() itself provides no details
for its caller, only that something went wrong (returns NULL); and my
main goal is to change that.)

If monitor_cur_is_qmp() returns true:

do_device_add
  qdev_device_add
    qbus_find
      qbus_find_recursive
        qerror_report(ERROR_CLASS_GENERIC_ERROR)
          monitor_set_error()
            /* save it for reporting */
      qerror_report(QERR_BUS_NOT_FOUND)
        monitor_set_error()
          QDECREF

then the generic error code overrides the more specific error code and
is therefore not useful to the qmp caller; there's no chance for the qmp
client to act differently based on the generic error code.

The goals are that
(a) qbus_find() not care about monitor_cur_is_qmp() at all,
(b) qbus_find() be silent,
(c1) both pieces of error information (ERROR_CLASS_GENERIC_ERROR /
QERR_BUS_NOT_FOUND) reach the QMP caller,
(c2) (if that's not possible) the most specific error description reach
the caller. (Currently the more specific human readable message is
cross-connected to the less specific machine readable error code.)

The problem is propagating more than one error (or adding them up
somehow). In Java etc. one would throw a derived exception in the
innermost frame (so that it would be specific but subject for catching
by a more generic handler -- c2), or throw a new exception in the outer
frame but link the inner frame's exception object to it (c1).

Based on how qerror_report() works when monitor_cur_is_qmp() returns
true (= swallow all errors after the first), I think I actually know how
to propagate that, but I believe that approach will hurt at least one of
hmp / qmp:

If I prefer ERROR_CLASS_GENERIC_ERROR with the specific text, then the
qmp user will suffer (which is what happens now). If I prefer the more
specific QERR_BUS_NOT_FOUND with the *less* specific text, then the
human user will suffer.

Thanks,
Laszlo