Hi. I like your approach to list the command that is not found, however
I think the logic to detect an unknown command on the text monitor
should be broken out into a separate function (for reuse and
maintainability). I will respin a patch and let you decide if you agree.
On 08/22/2011 08:52 AM, Osier Yang wrote:
* 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
---
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..d296675 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -3031,18 +3031,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 +3062,27 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon,
goto cleanup;
}
+ /* Check error: Command not found, for "info block-jobs" command.
+ * If qemu monitor command "info" doesn't support one sub-command,
+ * it will print the output of "help info" instead, so simply check
+ * if "info version" is in the output.
+ */
+ if (mode == BLOCK_JOB_INFO) {
+ if (strstr(reply, "info version")) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("Command '%s' is not found"),
cmd_name);
+ ret = -1;
+ goto cleanup;
+ }
+ } else {
+ if (strstr(reply, "unknown command")) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ _("Command '%s' is not found"),
cmd_name);
+ ret = -1;
+ goto cleanup;
+ }
+ }
+
ret = qemuMonitorTextParseBlockJob(reply, device, info);
cleanup:
--
Adam Litke
IBM Linux Technology Center