[libvirt] [PATCH] qemu_agent: Report error class at least

[Daniel P. Berrange]

On Thu, Apr 12, 2012 at 02:06:21PM +0200, Michal Privoznik wrote:
> Currently, qemu GA is not providing 'desc' field for errors like
> we are used to from qemu monitor. Therefore, we fall back to this
> general 'unknown error' string. However, GA is reporting 'class' which
> is not perfect, but much more helpful than generic error string.
> Thus we should fall back to class firstly and if even no class
> is presented, then we can fall back to that generic string.
> 
> Before this patch:
> virsh # dompmsuspend --target mem f16
> error: Domain f16 could not be suspended
> error: internal error unable to execute QEMU command
> 'guest-suspend-ram': unknown QEMU command error
> 
> After this patch:
> virsh # dompmsuspend --target mem f16
> error: Domain f16 could not be suspended
> error: internal error unable to execute QEMU command
> 'guest-suspend-ram': CommandNotFound
> ---
>  src/qemu/qemu_agent.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index b759b7f..decfd0e 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1035,19 +1035,17 @@ static const char *
>  qemuAgentStringifyError(virJSONValuePtr error)
>  {
>      const char *klass = virJSONValueObjectGetString(error, "class");
> -    const char *detail = NULL;
> +    const char *detail = virJSONValueObjectGetString(error, "desc");
>  
>      /* The QMP 'desc' field is usually sufficient for our generic
> -     * error reporting needs.
> +     * error reporting needs. However, older agents did not provide
> +     * any 'desc'. Reporting 'class' is not perfect but better
> +     * than bare 'unknown error'.
>       */
> -    if (klass)
> -        detail = virJSONValueObjectGetString(error, "desc");
> -
> -
> -    if (!detail)
> +    if (!detail && !klass)
>          detail = "unknown QEMU command error";
>  
> -    return detail;
> +    return detail ? detail : klass;
>  }

You can get a list of all 'class' names that QEMU currently supports
from qerror.h.  As we do for VIR_ERR_XXXXX constants, you should create
a mapping from QEMU class names, to error message strings.

Even better, share this mapping between the agent & json monitor code
so we can improve error messages in both cases.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|