[libvirt] [PATCH v2 0/3] qemu: fallback to HMP drive_add/drive_del
Version 2: - provide nicer errors when human-monitor-command is not available Hu Tao (1): qemu: fallback to HMP drive_add/drive_del Jiri Denemark (2): qemu: Detect support for HMP passthrough qemu: Only use HMP passthrough if it is supported src/qemu/qemu_monitor.c | 22 +++++++++- src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_monitor_json.h | 2 + 4 files changed, 104 insertions(+), 20 deletions(-) -- 1.7.4.1
--- Notes: Version 2: - optionally log that HMP passthrough cannot be used for a given command src/qemu/qemu_monitor.c | 22 ++++++++++++++++++++-- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dc08594..d7ca934 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; }; @@ -924,15 +925,32 @@ 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, const char *cmd) +{ + if (!mon->json || mon->json_hmp) + return 1; + + if (cmd) { + VIR_DEBUG("HMP passthrough not supported by qemu process;" + " not trying HMP for command %s", cmd); + } + + return 0; +} + + +int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7bea083..55d7590 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, const char *cmd); + 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/21/2011 06:52 AM, Jiri Denemark wrote:
--- Notes: Version 2: - optionally log that HMP passthrough cannot be used for a given command
src/qemu/qemu_monitor.c | 22 ++++++++++++++++++++-- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 66 insertions(+), 2 deletions(-)
int +qemuMonitorCheckHMP(qemuMonitorPtr mon, const char *cmd) +{ + if (!mon->json || mon->json_hmp) + return 1; + + if (cmd) { + VIR_DEBUG("HMP passthrough not supported by qemu process;" + " not trying HMP for command %s", cmd);
I had to check the rest of the series to make sure that the message never leaks out to the user (otherwise, the message would need translation and VIR_DEBUG would not be right); fortunately, the one place where you return failure to the user you used a separate qemuReportError so this message is indeed debug-only (and useful). ACK -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Avoids calling text monitor methods when it is know they will not succeed and also results in nicer error messages. --- Notes: Version 2: - new patch src/qemu/qemu_monitor_json.c | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 43245a6..d2d6ba1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1495,7 +1495,8 @@ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound") && + qemuMonitorCheckHMP(mon, "cpu_set")) { VIR_DEBUG0("cpu_set command not found, trying HMP"); ret = qemuMonitorTextSetCPU(mon, cpu, online); goto cleanup; @@ -2384,7 +2385,8 @@ int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name) if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound") && + qemuMonitorCheckHMP(mon, "savevm")) { VIR_DEBUG0("savevm command not found, trying HMP"); ret = qemuMonitorTextCreateSnapshot(mon, name); goto cleanup; @@ -2413,7 +2415,8 @@ int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name) if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound") && + qemuMonitorCheckHMP(mon, "loadvm")) { VIR_DEBUG0("loadvm command not found, trying HMP"); ret = qemuMonitorTextLoadSnapshot(mon, name); goto cleanup; @@ -2442,7 +2445,8 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound") && + qemuMonitorCheckHMP(mon, "delvm")) { VIR_DEBUG0("delvm command not found, trying HMP"); ret = qemuMonitorTextDeleteSnapshot(mon, name); goto cleanup; @@ -2466,6 +2470,12 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, int ret = -1; if (hmp) { + if (!qemuMonitorCheckHMP(mon, NULL)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HMP passthrough is not supported by qemu" + " process; only QMP commands can be used")); + return -1; + } return qemuMonitorJSONHumanCommandWithFd(mon, cmd_str, -1, reply_str); } else { if (!(cmd = virJSONValueFromString(cmd_str))) -- 1.7.4.1
On 03/21/2011 06:52 AM, Jiri Denemark wrote:
Avoids calling text monitor methods when it is know they will not succeed and also results in nicer error messages. --- Notes: Version 2: - new patch
- if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound") && + qemuMonitorCheckHMP(mon, "cpu_set")) { VIR_DEBUG0("cpu_set command not found, trying HMP");
So either we get a debug message about trying HMP, or we get a debug message about skipping HMP. Looks nice.
@@ -2466,6 +2470,12 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, int ret = -1;
if (hmp) { + if (!qemuMonitorCheckHMP(mon, NULL)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HMP passthrough is not supported by qemu" + " process; only QMP commands can be used"));
And this is the one place where it becomes a user-visible effect. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
From: Hu Tao <hutao@cn.fujitsu.com> fallback to HMP drive_add/drive_del commands if not found in QMP --- Notes: Version 2: - check for human-monitor-command availability before calling text monitor methods 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 d2d6ba1..6bd03d6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2289,11 +2289,19 @@ 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") && + qemuMonitorCheckHMP(mon, "drive_add")) { + 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; @@ -2314,22 +2322,24 @@ 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, "drive_del")) { + VIR_DEBUG0("drive_del command not found, trying HMP"); + ret = qemuMonitorTextDriveDel(mon, drivestr); + } else { 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/21/2011 06:52 AM, Jiri Denemark wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
fallback to HMP drive_add/drive_del commands if not found in QMP --- Notes: Version 2: - check for human-monitor-command availability before calling text monitor methods
src/qemu/qemu_monitor_json.c | 38 ++++++++++++++++++++++++-------------- 1 files changed, 24 insertions(+), 14 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Mar 21, 2011 at 13:52:35 +0100, Jiri Denemark wrote:
Version 2: - provide nicer errors when human-monitor-command is not available
Hu Tao (1): qemu: fallback to HMP drive_add/drive_del
Jiri Denemark (2): qemu: Detect support for HMP passthrough qemu: Only use HMP passthrough if it is supported
src/qemu/qemu_monitor.c | 22 +++++++++- src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_monitor_json.h | 2 + 4 files changed, 104 insertions(+), 20 deletions(-)
Thanks Eric, I've just pushed this series. Jirka
On Tue, Mar 22, 2011 at 03:08:19PM +0100, Jiri Denemark wrote:
On Mon, Mar 21, 2011 at 13:52:35 +0100, Jiri Denemark wrote:
Version 2: - provide nicer errors when human-monitor-command is not available
Hu Tao (1): qemu: fallback to HMP drive_add/drive_del
Jiri Denemark (2): qemu: Detect support for HMP passthrough qemu: Only use HMP passthrough if it is supported
src/qemu/qemu_monitor.c | 22 +++++++++- src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 98 ++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_monitor_json.h | 2 + 4 files changed, 104 insertions(+), 20 deletions(-)
Thanks Eric, I've just pushed this series.
Jirka
Thanks Jirka and Eric. -- Thanks, Hu Tao
participants (3)
-
Eric Blake -
Hu Tao -
Jiri Denemark