[libvirt] [PATCH] do not return -1 when human-monitor-command is not supported

[Jiri Denemark]

On Thu, Mar 17, 2011 at 11:22:47 +0800, Wen Congyang wrote:
> It is not a fatal error if some monitor command is not supported.
> So we should not return -1 when human-monitor-command is not supported.
> Set reply_str to "unknown command: 'human-monitor-command'", and the caller
> can deal this error the same as the command that we want to run by
> human-monitor-command.
> 
> Signed-off-by: Wen Congyang <wency at cn.fujitsu.com>
> 
> ---
>  src/qemu/qemu_monitor_json.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 13d12c8..7ec4ce7 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -694,6 +694,16 @@ qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
>      if (!cmd || qemuMonitorJSONCommandWithFd(mon, cmd, scm_fd, &reply) < 0)
>          goto cleanup;
>  
> +    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> +        *reply_str = strdup("unknown command: 'human-monitor-command'");
> +        if (!*reply_str) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
>      if (qemuMonitorJSONCheckError(cmd, reply))
>          goto cleanup;
>  

I'm not convinced this is a good idea. In case the caller (from
qemu_monitor_text.c) decides to ignore the error and return success, the
original caller from qemu_monitor_json.c will get confused since it will think
the operation succeeded. I think the json method should really fail in case it
tries to use HMP passthrough which is not supported. In a short while I'll
submit a patch that makes it possible to detect availability of
human-monitor-command so that each json method can decide what to do if HMP
passthrough is not supported.

Jirka