[libvirt] [PATCH 0/2] qemu: fallback to HMP drive_add/drive_del
Hu Tao (1): qemu: fallback to HMP drive_add/drive_del Jiri Denemark (1): qemu: Detect support for HMP passthrough src/qemu/qemu_monitor.c | 14 ++++++- src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 80 ++++++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor_json.h | 2 + 4 files changed, 82 insertions(+), 16 deletions(-) -- 1.7.4.1
--- src/qemu/qemu_monitor.c | 14 ++++++++++++-- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index da38096..6da6700 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -72,6 +72,7 @@ struct _qemuMonitor { int lastErrno; unsigned json: 1; + unsigned json_hmp: 1; }; @@ -923,15 +924,24 @@ int qemuMonitorSetCapabilities(qemuMonitorPtr mon) return -1; } - if (mon->json) + if (mon->json) { ret = qemuMonitorJSONSetCapabilities(mon); - else + mon->json_hmp = qemuMonitorJSONCheckHMP(mon); + } else { ret = 0; + } return ret; } int +qemuMonitorCheckHMP(qemuMonitorPtr mon) +{ + return !mon->json || mon->json_hmp; +} + + +int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7bea083..823458b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -127,6 +127,8 @@ void qemuMonitorClose(qemuMonitorPtr mon); int qemuMonitorSetCapabilities(qemuMonitorPtr mon); +int qemuMonitorCheckHMP(qemuMonitorPtr mon); + void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 152afba..43245a6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -747,6 +747,48 @@ qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon) int +qemuMonitorJSONCheckHMP(qemuMonitorPtr mon) +{ + int ret = 0; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-commands", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + int i, n; + + if (!cmd) + return ret; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0 || + qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return")) || + data->type != VIR_JSON_TYPE_ARRAY || + (n = virJSONValueArraySize(data)) <= 0) + goto cleanup; + + for (i = 0; i < n; i++) { + virJSONValuePtr entry; + const char *name; + + if (!(entry = virJSONValueArrayGet(data, i)) || + !(name = virJSONValueObjectGetString(entry, "name"))) + goto cleanup; + + if (STREQ(name, "human-monitor-command")) { + ret = 1; + goto cleanup; + } + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn ATTRIBUTE_UNUSED) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9d039dd..086f0e1 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -41,6 +41,8 @@ int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon); +int qemuMonitorJSONCheckHMP(qemuMonitorPtr mon); + int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorJSONStopCPUs(qemuMonitorPtr mon); -- 1.7.4.1
On 03/17/2011 09:00 AM, Jiri Denemark wrote:
--- src/qemu/qemu_monitor.c | 14 ++++++++++++-- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 58 insertions(+), 2 deletions(-)
+++ b/src/qemu/qemu_monitor.h @@ -127,6 +127,8 @@ void qemuMonitorClose(qemuMonitorPtr mon);
int qemuMonitorSetCapabilities(qemuMonitorPtr mon);
+int qemuMonitorCheckHMP(qemuMonitorPtr mon); +
Do we need code in qemu_driver.c:qemuDomainMonitorCommand to make 'virsh qemu-monitor-command --hmp' get a nice error message when qemuMonitorCheckHMP fails, or is that error message already sufficient? Depending on the answer to that question, ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Mar 17, 2011 at 13:40:38 -0600, Eric Blake wrote:
Do we need code in qemu_driver.c:qemuDomainMonitorCommand to make 'virsh qemu-monitor-command --hmp' get a nice error message when qemuMonitorCheckHMP fails, or is that error message already sufficient?
The error returned in this case is: error: internal error unable to execute QEMU command 'human-monitor-command': The command human-monitor-command has not been found It's good enough for someone who knows what that means but not very nice for normal end user. So I made some changes and sent v2 of this series. With that the error is: error: unsupported configuration: HMP passthrough is not supported by qemu process; only QMP commands can be used Similar thing applies to your comment in patch 2/2. Jirka
From: Hu Tao <hutao@cn.fujitsu.com> fallback to HMP drive_add/drive_del commands if not found in QMP --- src/qemu/qemu_monitor_json.c | 38 ++++++++++++++++++++++++-------------- 1 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 43245a6..235985e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2288,11 +2288,18 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0)) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("drive_add command not found, trying HMP"); + ret = qemuMonitorTextAddDrive(mon, drivestr); + goto cleanup; + } + + ret = qemuMonitorJSONCheckError(cmd, reply); +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2313,22 +2320,25 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; - if (ret == 0) { - /* See if drive_del isn't supported */ - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorCheckHMP(mon)) { + VIR_DEBUG0("drive_del command not found, trying HMP"); + ret = qemuMonitorTextDriveDel(mon, drivestr); + } else { + VIR_DEBUG0("HMP passthrough is not supported"); VIR_ERROR0(_("deleting disk is not supported. " "This may leak data if disk is reassigned")); ret = 1; - goto cleanup; - } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { - /* NB: device not found errors mean the drive was - * auto-deleted and we ignore the error */ - ret = 0; - } else { - ret = qemuMonitorJSONCheckError(cmd, reply); } + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + /* NB: device not found errors mean the drive was + * auto-deleted and we ignore the error */ + ret = 0; + } else { + ret = qemuMonitorJSONCheckError(cmd, reply); } cleanup: -- 1.7.4.1
On 03/17/2011 09:00 AM, Jiri Denemark wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
fallback to HMP drive_add/drive_del commands if not found in QMP --- src/qemu/qemu_monitor_json.c | 38 ++++++++++++++++++++++++-------------- 1 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 43245a6..235985e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2288,11 +2288,18 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, if (!cmd) return -1;
- ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0)) + goto cleanup;
- if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("drive_add command not found, trying HMP"); + ret = qemuMonitorTextAddDrive(mon, drivestr);
Here, we always try the hmp variant...
- /* See if drive_del isn't supported */ - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorCheckHMP(mon)) { + VIR_DEBUG0("drive_del command not found, trying HMP"); + ret = qemuMonitorTextDriveDel(mon, drivestr);
...but here, we only try hmp if it is present. I guess that makes sense if the error message for add when hmp is missing is reasonable (the whole point of skipping on delete is that hmp not present is not fatal, so we don't want the error message). So: ACK -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Jiri Denemark