On 03/05/2014 07:15 AM, Peter Krempa wrote:
> The qemuMonitorJSONBlockJob handles a few errors internally. If qemu
> returns a different error we would report a rather unhelpful message:
> 
>  $ virsh blockpull gluster-job vda --base /dev/null
>  error: internal error: Unexpected error
> 
> As the actual message from qemu contains a bit more info, let's use it
> to report something a little more useful:
> 
>  $ virsh blockpull gluster-job vda --base /dev/null
>  error: internal error: Unexpected error: (GenericError) 'Base '/dev/null' 
> not found'

The fact that we're reporting an internal error instead of catching it
up front and avoiding the monitor command in the first place is a bit
worrisome (ideally, we should be tracking the backing chain ourselves,
and have some clue that qemu can't succeed because the input the user
gave was not part of the chain).  But fixing _that_ is a bigger task,
and meanwhile, this patch definitely improves the error quality by
reflecting qemu's complaints.

For that matter, the fact that it is so easy to trigger an internal
error is a bit worrisome, but I don't know if we have a better error
category to use.

> ---
>  src/qemu/qemu_monitor_json.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

ACK.

> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9a5b812..ee3ae15 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3678,8 +3678,12 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
>              virReportError(VIR_ERR_OPERATION_INVALID,
>                             _("Command '%s' is not found"), cmd_name);
>          } else {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("Unexpected error"));
> +            virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
> +
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unexpected error: (%s) '%s'"),
> +                           NULLSTR(virJSONValueObjectGetString(error, 
> "class")),
> +                           NULLSTR(virJSONValueObjectGetString(error, 
> "desc")));
>          }
>      }
> 

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to