[libvirt] [PATCH 01/11] qemu: monitor: Extract handling of JSON block job error codes

[John Ferlan]

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;
>