Re: [Qemu-devel] [PATCH 5/7] qdev-monitor: Convert qbus_find() to Error

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

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)

But that can be a later patch, this one is fine as-is for following
existing practice.

> 
> Signed-off-by: Markus Armbruster <armbru@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.

> @@ -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/

You could always skirt the confusion by creative wording like "has
multiple bus children".  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 <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]