[libvirt] [PATCH] qemu: Use QMP for send-key if supported

Instead of always using HMP use the QMP send-key command introduced in qemu 1.3. --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 25 +++++++++++------ src/qemu/qemu_monitor.c | 5 ++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 64 ++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 50712b0..051285c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -216,6 +216,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "ipv6-migration", /* 135 */ "machine-opt", + "send-key", ); struct _virQEMUCaps { @@ -1954,6 +1955,8 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, virQEMUCapsSet(qemuCaps, QEMU_CAPS_ADD_FD); else if (STREQ(name, "nbd-server-start")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_NBD_SERVER); + else if (STREQ(name, "send-key")) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_SEND_KEY); VIR_FREE(name); } VIR_FREE(commands); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b2dc588..bcd4daf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -176,6 +176,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_SCSI_MEGASAS = 134, /* -device megasas */ QEMU_CAPS_IPV6_MIGRATION = 135, /* -incoming [::] */ QEMU_CAPS_MACHINE_OPT = 136, /* -machine xxxx*/ + QEMU_CAPS_SEND_KEY = 137, /* send-key QMP command */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2c0d7d1..cfb6fc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2227,14 +2227,16 @@ cleanup: return ret; } -static int qemuDomainSendKey(virDomainPtr domain, - unsigned int codeset, - unsigned int holdtime, - unsigned int *keycodes, - int nkeycodes, - unsigned int flags) +static int +qemuDomainSendKey(virDomainPtr domain, + unsigned int codeset, + unsigned int holdtime, + unsigned int *keycodes, + int nkeycodes, + unsigned int flags) { virQEMUDriverPtr driver = domain->conn->privateData; + virQEMUCapsPtr qemuCaps = NULL; virDomainObjPtr vm = NULL; int ret = -1; qemuDomainObjPrivatePtr priv; @@ -2251,7 +2253,8 @@ static int qemuDomainSendKey(virDomainPtr domain, keycodes[i]); if (keycode < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot translate keycode %u of %s codeset to rfb keycode"), + _("cannot translate keycode %u of %s codeset to " + "rfb keycode"), keycodes[i], virKeycodeSetTypeToString(codeset)); return -1; @@ -2263,6 +2266,10 @@ static int qemuDomainSendKey(virDomainPtr domain, if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, + vm->def->emulator))) + goto cleanup; + priv = vm->privateData; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -2275,7 +2282,8 @@ static int qemuDomainSendKey(virDomainPtr domain, } qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes); + ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes, + virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEND_KEY)); qemuDomainObjExitMonitor(driver, vm); endjob: @@ -2285,6 +2293,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); + virObjectUnref(qemuCaps); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d1c6690..d9b5c23 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3008,14 +3008,15 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes) + unsigned int nkeycodes, + bool useQMP) { int ret; VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes); - if (mon->json) + if (mon->json && useQMP) ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); else ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3808a8..fadad08 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -601,7 +601,8 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes); + unsigned int nkeycodes, + bool useQMP); typedef enum { BLOCK_JOB_ABORT, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1bf8baf..43057fb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3372,11 +3372,65 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, unsigned int *keycodes, unsigned int nkeycodes) { - /* - * FIXME: qmp sendkey has not been implemented yet, - * and qmp API of it cannot be anticipated, so we use hmp temporary. - */ - return qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes); + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr keys = NULL; + virJSONValuePtr key = NULL; + unsigned int i; + + /* create the key data array */ + if (!(keys = virJSONValueNewArray())) + goto no_memory; + + for (i = 0; i < nkeycodes; i++) { + if (keycodes[i] > 0xffff) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("keycode %d is invalid: 0x%X"), i, keycodes[i]); + goto cleanup; + } + + /* create single key object */ + if (!(key = virJSONValueNewObject())) + goto no_memory; + + /* Union KeyValue has two, types, use the generic one */ + if (virJSONValueObjectAppendString(key, "type", "number") < 0) + goto no_memory; + + /* with the keycode */ + if (virJSONValueObjectAppendNumberInt(key, "data", keycodes[i]) < 0) + goto no_memory; + + if (virJSONValueArrayAppend(keys, key) < 0) + goto no_memory; + + key = NULL; + + } + + cmd = qemuMonitorJSONMakeCommand("send-key", + "a:keys", keys, + holdtime ? "U:hold-time" : NULL, holdtime, + NULL); + if (!cmd) + goto cleanup; + + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; + + ret = qemuMonitorJSONCheckError(cmd, reply); + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(keys); + virJSONValueFree(key); + return ret; + +no_memory: + virReportOOMError(); + goto cleanup; } int qemuMonitorJSONScreendump(qemuMonitorPtr mon, -- 1.8.1.5

On 04/11/2013 06:45 AM, Peter Krempa wrote:
Instead of always using HMP use the QMP send-key command introduced in qemu 1.3. ---
+++ b/src/qemu/qemu_monitor.c @@ -3008,14 +3008,15 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes) + unsigned int nkeycodes, + bool useQMP) { int ret;
VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes);
- if (mon->json) + if (mon->json && useQMP) ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); else ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
I'm not sure I like adding the useQMP parameter. That makes for a leaky abstraction (the caller is burdened with telling us which version of a polymorphic function to dispatch to). What we have done in other cases is to always dispatch to the json code, and then have the json code _try_ the command, and if the command fails, fall back to the hmp command. See for example qemuMonitorJSONInjectNMI. And if you do it that way, you don't even need a qemu_capabilities bit, and the caller no longer has to care about which version of monitor it is talking to. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 04/11/13 15:00, Eric Blake wrote:
On 04/11/2013 06:45 AM, Peter Krempa wrote:
Instead of always using HMP use the QMP send-key command introduced in qemu 1.3. ---
+++ b/src/qemu/qemu_monitor.c @@ -3008,14 +3008,15 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes) + unsigned int nkeycodes, + bool useQMP) { int ret;
VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes);
- if (mon->json) + if (mon->json && useQMP) ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); else ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
I'm not sure I like adding the useQMP parameter. That makes for a leaky abstraction (the caller is burdened with telling us which version of a polymorphic function to dispatch to). What we have done in other cases is to always dispatch to the json code, and then have the json code _try_ the command, and if the command fails, fall back to the hmp
I saw that approach and I didn't like it thus I created this.
command. See for example qemuMonitorJSONInjectNMI. And if you do it that way, you don't even need a qemu_capabilities bit, and the caller no longer has to care about which version of monitor it is talking to.
Peter

On Thu, Apr 11, 2013 at 03:06:51PM +0200, Peter Krempa wrote:
On 04/11/13 15:00, Eric Blake wrote:
On 04/11/2013 06:45 AM, Peter Krempa wrote:
Instead of always using HMP use the QMP send-key command introduced in qemu 1.3. ---
+++ b/src/qemu/qemu_monitor.c @@ -3008,14 +3008,15 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes) + unsigned int nkeycodes, + bool useQMP) { int ret;
VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes);
- if (mon->json) + if (mon->json && useQMP) ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); else ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
I'm not sure I like adding the useQMP parameter. That makes for a leaky abstraction (the caller is burdened with telling us which version of a polymorphic function to dispatch to). What we have done in other cases is to always dispatch to the json code, and then have the json code _try_ the command, and if the command fails, fall back to the hmp
I saw that approach and I didn't like it thus I created this.
We use that approach for all other monitors commands, and we should maintain the consistent approach for this sendkey command too. So please make this work as Eric describes. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 04/11/13 15:41, Daniel P. Berrange wrote:
On Thu, Apr 11, 2013 at 03:06:51PM +0200, Peter Krempa wrote:
On 04/11/13 15:00, Eric Blake wrote:
On 04/11/2013 06:45 AM, Peter Krempa wrote:
Instead of always using HMP use the QMP send-key command introduced in qemu 1.3. ---
+++ b/src/qemu/qemu_monitor.c @@ -3008,14 +3008,15 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon) int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, - unsigned int nkeycodes) + unsigned int nkeycodes, + bool useQMP) { int ret;
VIR_DEBUG("mon=%p, holdtime=%u, nkeycodes=%u", mon, holdtime, nkeycodes);
- if (mon->json) + if (mon->json && useQMP) ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes); else ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
I'm not sure I like adding the useQMP parameter. That makes for a leaky abstraction (the caller is burdened with telling us which version of a polymorphic function to dispatch to). What we have done in other cases is to always dispatch to the json code, and then have the json code _try_ the command, and if the command fails, fall back to the hmp
I saw that approach and I didn't like it thus I created this.
We use that approach for all other monitors commands, and we should maintain the consistent approach for this sendkey command too. So please make this work as Eric describes.
I already done it that way: http://www.redhat.com/archives/libvir-list/2013-April/msg00897.html Peter
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Peter Krempa