[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
From: Osier Yang <jyang@redhat.com> * src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error. * src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed. Without this patch, libvirt always report: An error occurred, but the cause is unknown This patch was adapted from a patch by Osier Yang <jyang@redhat.com> to break out detection of unrecognized text monitor commands into a separate function. Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + } if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..9119f06 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,20 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); } +/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version")) + return 1; + return 0; +} + static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, @@ -3031,18 +3045,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, char *cmd = NULL; char *reply = NULL; int ret; - - if (mode == BLOCK_JOB_ABORT) - ret = virAsprintf(&cmd, "block_job_cancel %s", device); - else if (mode == BLOCK_JOB_INFO) - ret = virAsprintf(&cmd, "info block-jobs"); - else if (mode == BLOCK_JOB_SPEED) - ret = virAsprintf(&cmd, "block_job_set_speed %s %llu", device, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "info block-jobs"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + ret = virAsprintf(&cmd, "%s %s %llu", cmd_name, device, bandwidth * 1024ULL * 1024ULL); - else if (mode == BLOCK_JOB_PULL) - ret = virAsprintf(&cmd, "block_stream %s", device); - else + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else { return -1; + } if (ret < 0) { virReportOOMError(); @@ -3056,6 +3076,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + if (qemuMonitorTextCommandNotFound(reply)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.3
于 2011年08月23日 00:53, Adam Litke 写道:
From: Osier Yang <jyang@redhat.com>
* src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error.
* src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed.
Without this patch, libvirt always report:
An error occurred, but the cause is unknown
This patch was adapted from a patch by Osier Yang <jyang@redhat.com> to break out detection of unrecognized text monitor commands into a separate function.
Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + }
if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..9119f06 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,20 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); }
+/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version")) + return 1; + return 0; +} +
Hi, Adam I likes the idea to wrap the checking as a function seperately, but the function won't work well if the command is "help info", though we don't have a use of "help info" yet. My point is since the function is intending to work for all the command, as a common function, it needs to consider some boudary too. How about below? qemuMonitorTextCommandNotFound(const char *cmd, const char *reply) { if (STRPREFIX(cmd, "info ")) { if (strstr(reply, "info version")) return 1; } else { if (strstr(reply, "unknown command:")) return 1; } return 0; } And there might be other different info qemu will output for a unknown command we don't known yet. Using "cmd" as an argument will allow us to extend the checking methods. Thanks Osier
On 08/22/2011 10:04 PM, Osier Yang wrote:
Hi, Adam
I likes the idea to wrap the checking as a function seperately, but the function won't work well if the command is "help info", though we don't have a use of "help info" yet.
My point is since the function is intending to work for all the command, as a common function, it needs to consider some boudary too.
How about below?
Good points. The function below looks good.
qemuMonitorTextCommandNotFound(const char *cmd, const char *reply) { if (STRPREFIX(cmd, "info ")) { if (strstr(reply, "info version")) return 1; } else { if (strstr(reply, "unknown command:")) return 1; }
return 0; }
And there might be other different info qemu will output for a unknown command we don't known yet. Using "cmd" as an argument will allow us to extend the checking methods.
Thanks Osier
-- Adam Litke IBM Linux Technology Center
于 2011年08月23日 22:31, Adam Litke 写道:
On 08/22/2011 10:04 PM, Osier Yang wrote:
Hi, Adam
I likes the idea to wrap the checking as a function seperately, but the function won't work well if the command is "help info", though we don't have a use of "help info" yet.
My point is since the function is intending to work for all the command, as a common function, it needs to consider some boudary too.
How about below? Good points. The function below looks good.
Given there was an ACK from Eric, I pushed with modifying the function as below. Thanks.
qemuMonitorTextCommandNotFound(const char *cmd, const char *reply) { if (STRPREFIX(cmd, "info ")) { if (strstr(reply, "info version")) return 1; } else { if (strstr(reply, "unknown command:")) return 1; }
return 0; }
And there might be other different info qemu will output for a unknown command we don't known yet. Using "cmd" as an argument will allow us to extend the checking methods.
Thanks Osier
participants (2)
-
Adam Litke -
Osier Yang