Re: [libvirt] [PATCH 01/11] qemu: monitor: Extract handling of JSON block job error codes
On 04/01/2015 01:20 PM, Peter Krempa wrote:
My intention is to split qemuMonitorJSONBlockJob() into simpler
separate
functions for every block job type. Since the error handling code is the
same for all block jobs, this patch extracts the code into a separate
function that will later be reused in more places.
---
src/qemu/qemu_monitor_json.c | 64 +++++++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index aa844ca..edd0a3f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4252,6 +4252,39 @@ qemuMonitorJSONBlockJobInfo(qemuMonitorPtr mon,
}
+static int
+qemuMonitorJSONBlockJobError(virJSONValuePtr reply,
+ const char *cmd_name,
+ const char *device)
+{
+ if (!virJSONValueObjectHasKey(reply, "error"))
+ return 0;
+
+ if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("No active operation on device: %s"), device);
+ } else if (qemuMonitorJSONHasError(reply, "DeviceInUse")) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Device %s in use"), device);
+ } else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Operation is not supported for device: %s"),
device);
+ } else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Command '%s' is not found"), cmd_name);
+ } else {
+ virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
Because you touched it - Coverity whines that 'error' is not checked for
NULL:
(8) Event returned_null: "virJSONValueObjectGet" returns null (checked
96 out of 99 times). [details]
(16) Event var_assigned: Assigning: "error" = null return value from
"virJSONValueObjectGet".
Also see events:
(17) Event dereference: Dereferencing a pointer that might be null
"error" when calling "virJSONValueObjectGetString". [details]
There's many examples (96) of checking...
ACK with the check
John
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected error: (%s) '%s'"),
+ NULLSTR(virJSONValueObjectGetString(error, "class")),
+ NULLSTR(virJSONValueObjectGetString(error, "desc")));
+ }
+
+ return -1;
+}
+
+
/* speed is in bytes/sec */
int
qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
@@ -4322,34 +4355,15 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
if (!cmd)
return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- if (ret == 0 && virJSONValueObjectHasKey(reply, "error")) {
- ret = -1;
- if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("No active operation on device: %s"),
- device);
- } else if (qemuMonitorJSONHasError(reply, "DeviceInUse")) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Device %s in use"), device);
- } else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("Operation is not supported for device: %s"),
- device);
- } else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("Command '%s' is not found"), cmd_name);
- } else {
- virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
+ if (qemuMonitorJSONBlockJobError(reply, cmd_name, device) < 0)
+ goto cleanup;
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unexpected error: (%s) '%s'"),
- NULLSTR(virJSONValueObjectGetString(error,
"class")),
- NULLSTR(virJSONValueObjectGetString(error,
"desc")));
- }
- }
+ ret = 0;
+ cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;