[libvirt] [PATCH] qemu: monitor: Provide more information in generic block job error
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' --- src/qemu/qemu_monitor_json.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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"))); } } -- 1.9.0
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
On 03/05/14 16:49, Eric Blake wrote:
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.
Pushed; Thanks. Peter
participants (2)
-
Eric Blake -
Peter Krempa