Eric Blake <ebl...@redhat.com> writes: > On 06/08/2015 12:57 PM, Markus Armbruster wrote: >> As usual, the conversion breaks printing explanatory messages after >> the error: actual printing of the error gets delayed, so the >> explanations precede rather than follow it. >> >> Pity. Disable them for now. See also commit 7216ae3. > > Could we add some sort of error_append_hmp_hint() that adds additional > messages to an existing error object, for use when the error will be > printed via HMP but is a no-op for QMP? (and make it callable more than > once, since qbus_list_dev() uses error_printf() in that role more than once)
Should work. Should get its own series, of course. Volunteer welcome :) > But that can be a later patch, this one is fine as-is for following > existing practice. Agree. >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> include/qapi/qmp/qerror.h | 3 --- >> qdev-monitor.c | 30 +++++++++++++++++++----------- >> 2 files changed, 19 insertions(+), 14 deletions(-) >> >> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h >> index e567339..6468e40 100644 >> --- a/include/qapi/qmp/qerror.h >> +++ b/include/qapi/qmp/qerror.h >> @@ -43,9 +43,6 @@ void qerror_report_err(Error *err); >> #define QERR_BUS_NO_HOTPLUG \ >> ERROR_CLASS_GENERIC_ERROR, "Bus '%s' does not support hotplugging" >> >> -#define QERR_BUS_NOT_FOUND \ >> - ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found" > > Might want to mention one less baroque macro in qerror.h in the commit > message as an intentional part of the conversion. Could append: While there, eliminate QERR_BUS_NOT_FOUND. >> @@ -475,14 +479,15 @@ static BusState *qbus_find(const char *path) >> break; >> } >> if (dev->num_child_bus) { >> - qerror_report(ERROR_CLASS_GENERIC_ERROR, >> - "Device '%s' has multiple child busses", >> elem); >> + error_setg(errp, "Device '%s' has multiple child busses", > > Stupid spell-check on my mailer is flagging 'busses' as a typo, even > though dictionary.com says both spellings are acceptable. Other sources > prefer 'buses' and say 'busses' is out of favor: > http://grammarist.com/spelling/buses-busses/ If I have to respin anyway, I can fold in s/busses/buses/, and amend While there, eliminate QERR_BUS_NOT_FOUND, and clean up unusual spelling in the error message. > You could always skirt the confusion by creative wording like "has > multiple bus children". Nah, we just spell like the dictionary says we should. > But it is a pre-existing issue [if an issue at > all], so I don't care enough to make it hold up this patch. > > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!