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
