On 31 January 2013 18:19, Laszlo Ersek <ler...@redhat.com> wrote: > 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*.
OK, that's a problem. We should only be reporting one error: "we failed because you asked for this bus and it's full" should override the default "we failed to find this bus". We can fix that by having the recursion stop as soon as we get an error. > 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, I think the QMP caller should also only get one error. > 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. Why does the qmp user need to get QERR_BUS_NOT_FOUND? (it would be an incorrect error anyway in the case where we have the GENERIC_ERROR text, because we have in fact found the bus, we just couldn't use it.) -- PMM
