[libvirt] [PATCH] qemu: Add shortcut for HMP pass through

Currently users who want to use virDomainQemuMonitorCommand() API or it's virsh equivalent has to use the same protocol as libvirt uses for communication to qemu. Since the protocol is QMP with current qemu and HMP much more usable for humans, one ends up typing something like the following: virsh qemu-monitor-command DOM \ '{"execute":"human-monitor-command","arguments":{"command-line":"info kvm"}}' which is not a very convenient way of debugging qemu. This patch introduces --hmp option to qemu-monitor-command, which says that the provided command is in HMP. If libvirt uses QMP to talk with qemu, the command will automatically be converted into QMP. So the example above is simplified to just virsh qemu-monitor-command --hmp DOM "info kvm" Also the result is converted from {"return":"kvm support: enabled\r\n"} to just plain HMP: kvm support: enabled If libvirt talks to qemu in HMP, --hmp flag is obviously a noop. --- include/libvirt/libvirt-qemu.h | 5 +++++ src/internal.h | 1 + src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_monitor.c | 9 ++++++--- src/qemu/qemu_monitor.h | 5 ++++- src/qemu/qemu_monitor_json.c | 27 ++++++++++++++++++++++++--- src/qemu/qemu_monitor_json.h | 3 ++- tools/virsh.c | 7 ++++++- 8 files changed, 53 insertions(+), 11 deletions(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index 9c191c7..9257c2f 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -20,6 +20,11 @@ extern "C" { # endif +enum { + VIR_DOMAIN_QEMU_MONITOR_COMMAND_DEFAULT = 0, + VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP = (1 << 0), /* cmd is in HMP */ +} virDomainQemuMonitorCommandFlags; + int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags); diff --git a/src/internal.h b/src/internal.h index 038b862..dc1d9cb 100644 --- a/src/internal.h +++ b/src/internal.h @@ -38,6 +38,7 @@ # define N_(str) str # include "libvirt/libvirt.h" +# include "libvirt/libvirt-qemu.h" # include "libvirt/virterror.h" # include "libvirt_internal.h" diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 23e0db0..bb50264 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10226,8 +10226,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, virDomainObjPtr vm = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; + bool hmp; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); @@ -10253,10 +10254,12 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, priv->monitor_warned = 1; } + hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP); + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result); + ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); qemuDomainObjExitMonitorWithDriver(driver, vm); if (qemuDomainObjEndJob(vm) == 0) { vm = NULL; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 0c14277..fdb6b79 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2007,14 +2007,17 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) return ret; } -int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply) +int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, + const char *cmd, + char **reply, + bool hmp) { int ret; - DEBUG("mon=%p, cmd=%s, reply=%p", mon, cmd, reply); + DEBUG("mon=%p, cmd=%s, reply=%p, hmp=%d", mon, cmd, reply, hmp); if (mon->json) - ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply); + ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply, hmp); else ret = qemuMonitorTextArbitraryCommand(mon, cmd, reply); return ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 92c550b..0ea1330 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -397,7 +397,10 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name); -int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply); +int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, + const char *cmd, + char **reply, + bool hmp); /** * When running two dd process and using <> redirection, we need a diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ca06e7e..9ac12f8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2457,20 +2457,41 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, - char **reply_str) + char **reply_str, + bool hmp) { virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; int ret = -1; - cmd = virJSONValueFromString(cmd_str); + if (!hmp) { + cmd = virJSONValueFromString(cmd_str); + } else { + cmd = qemuMonitorJSONMakeCommand("human-monitor-command", + "s:command-line", cmd_str, + NULL); + } + if (!cmd) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - *reply_str = virJSONValueToString(reply); + if (!hmp) { + *reply_str = virJSONValueToString(reply); + } else if (qemuMonitorJSONCheckError(cmd, reply)) { + goto cleanup; + } else { + const char *data; + if (!(data = virJSONValueObjectGetString(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("human monitor command was missing return data")); + goto cleanup; + } + *reply_str = strdup(data); + } + if (!(*reply_str)) goto cleanup; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4c47f10..4ae472a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -209,6 +209,7 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, - char **reply_str); + char **reply_str, + bool hmp); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tools/virsh.c b/tools/virsh.c index 4533d38..59d3de5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -9932,6 +9932,7 @@ static const vshCmdInfo info_qemu_monitor_command[] = { static const vshCmdOptDef opts_qemu_monitor_command[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, {"cmd", VSH_OT_DATA, VSH_OFLAG_REQ, N_("command")}, + {"hmp", VSH_OT_BOOL, 0, N_("command is in human monitor protocol")}, {NULL, 0, 0, NULL} }; @@ -9942,6 +9943,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) int ret = FALSE; char *monitor_cmd; char *result = NULL; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -9956,7 +9958,10 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, 0) < 0) + if (vshCommandOptBool(cmd, "hmp")) + flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP; + + if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0) goto cleanup; printf("%s\n", result); -- 1.7.4

On 02/02/2011 08:49 AM, Jiri Denemark wrote:
Currently users who want to use virDomainQemuMonitorCommand() API or it's virsh equivalent has to use the same protocol as libvirt uses for communication to qemu. Since the protocol is QMP with current qemu and HMP much more usable for humans, one ends up typing something like the following:
virsh qemu-monitor-command DOM \ '{"execute":"human-monitor-command","arguments":{"command-line":"info kvm"}}'
which is not a very convenient way of debugging qemu.
+enum { + VIR_DOMAIN_QEMU_MONITOR_COMMAND_DEFAULT = 0, + VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP = (1 << 0), /* cmd is in HMP */ +} virDomainQemuMonitorCommandFlags; + int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags);
Aren't you glad we included the flags parameter when first adding the API? Makes life a lot easier when you don't have to add a new API.
- DEBUG("mon=%p, cmd=%s, reply=%p", mon, cmd, reply); + DEBUG("mon=%p, cmd=%s, reply=%p, hmp=%d", mon, cmd, reply, hmp);
if (mon->json) - ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply); + ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply, hmp); else ret = qemuMonitorTextArbitraryCommand(mon, cmd, reply); return ret;
Nice - only JSON has to care about the difference.
- *reply_str = virJSONValueToString(reply); + if (!hmp) { + *reply_str = virJSONValueToString(reply); + } else if (qemuMonitorJSONCheckError(cmd, reply)) { + goto cleanup; + } else { + const char *data; + if (!(data = virJSONValueObjectGetString(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("human monitor command was missing return data")); + goto cleanup; + } + *reply_str = strdup(data);
Hmm, if this fails, you're missing a call to virReportOOMError()...
+ } + if (!(*reply_str)) goto cleanup;
since pre-patch, you had been relying on virJSONValueToString to report it on your behalf. ACK with that nit fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Feb 02, 2011 at 09:18:25 -0700, Eric Blake wrote:
On 02/02/2011 08:49 AM, Jiri Denemark wrote:
+enum { + VIR_DOMAIN_QEMU_MONITOR_COMMAND_DEFAULT = 0, + VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP = (1 << 0), /* cmd is in HMP */ +} virDomainQemuMonitorCommandFlags; + int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags);
Aren't you glad we included the flags parameter when first adding the API? Makes life a lot easier when you don't have to add a new API.
Oh yes. All new APIs should be added with the flags parameter.
ACK with that nit fixed.
Thanks. I realized I also forgot to document the new flag in virsh man page so I decided to send a v2 so that the documentation is reviewed as well. Jirka
participants (2)
-
Eric Blake
-
Jiri Denemark