8:54 a.m.
When using modifiers with send-key, then we cannot know with what keys
those modifiers should be pressed down. QEMU changed the order of the
release events few times and that caused few send-key command to work
differently than expected.
We already state in virsh man page that the keys sent will be sent to
the guest simultaneously and can be received in random order. This
patch just tries improving user experience and keeping old behaviour.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_monitor.c | 6 +-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 142 ++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_monitor_json.h | 3 +-
tests/qemumonitorjsontest.c | 72 ++++++++++++++++++++--
6 files changed, 198 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 56e8430..bdf99eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2589,7 +2589,9 @@ static int qemuDomainSendKey(virDomainPtr domain,
}
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes);
+ ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes,
+ virQEMUCapsGet(priv->qemuCaps,
+ QEMU_CAPS_INPUT_EVENT));
qemuDomainObjExitMonitor(driver, vm);
endjob:
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 276649d..4f128a1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3431,7 +3431,8 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon)
int qemuMonitorSendKey(qemuMonitorPtr mon,
unsigned int holdtime,
unsigned int *keycodes,
- unsigned int nkeycodes)
+ unsigned int nkeycodes,
+ bool events)
{
int ret;
@@ -3439,7 +3440,8 @@ int qemuMonitorSendKey(qemuMonitorPtr mon,
mon, holdtime, nkeycodes);
if (mon->json)
- ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes);
+ ret = qemuMonitorJSONSendKey(mon, holdtime, keycodes, nkeycodes,
+ events);
else
ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
return ret;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 76c91a3..14c6347 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -740,7 +740,8 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
int qemuMonitorSendKey(qemuMonitorPtr mon,
unsigned int holdtime,
unsigned int *keycodes,
- unsigned int nkeycodes);
+ unsigned int nkeycodes,
+ bool events);
typedef enum {
BLOCK_JOB_ABORT,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 91a7aba..cd8a38b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -44,6 +44,7 @@
#include "virprobe.h"
#include "virstring.h"
#include "cpu/cpu_x86.h"
+#include "virkeycode.h"
#ifdef WITH_DTRACE_PROBES
# include "libvirt_qemu_probes.h"
@@ -3938,52 +3939,151 @@ int qemuMonitorJSONInjectNMI(qemuMonitorPtr mon)
return ret;
}
+static int
+qemuMonitorJSONAppendKey(virJSONValuePtr list,
+ unsigned int keycode,
+ bool use_events,
+ bool down)
+{
+ virJSONValuePtr data = NULL;
+ virJSONValuePtr event = NULL;
+ virJSONValuePtr key = NULL;
+
+ if (!(key = virJSONValueNewObject()))
+ goto cleanup;
+
+ /* Union KeyValue has two types, use the generic one */
+ if (virJSONValueObjectAppendString(key, "type", "number") <
0)
+ goto cleanup;
+
+ /* with the keycode */
+ if (virJSONValueObjectAppendNumberInt(key, "data", keycode) < 0)
+ goto cleanup;
+
+ if (!use_events) {
+ if (virJSONValueArrayAppend(list, key) < 0)
+ goto cleanup;
+
+ /* That's all if we're not using 'input-send-event'. */
+ return 0;
+ }
+
+ if (!(event = virJSONValueNewObject()) ||
+ !(data = virJSONValueNewObject()))
+ goto cleanup;
+
+ if (virJSONValueObjectAppendBoolean(data, "down", down) < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectAppend(data, "key", key) < 0)
+ goto cleanup;
+
+ key = NULL;
+
+ if (virJSONValueObjectAppendString(event, "type", "key") < 0)
+ goto cleanup;
+
+ if (virJSONValueObjectAppend(event, "data", data) < 0)
+ goto cleanup;
+
+ data = NULL;
+
+ if (virJSONValueArrayAppend(list, event) < 0)
+ goto cleanup;
+
+ event = NULL;
+
+ return 0;
+
+ cleanup:
+ virJSONValueFree(data);
+ virJSONValueFree(event);
+ virJSONValueFree(key);
+ return -1;
+}
+
int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
unsigned int holdtime,
unsigned int *keycodes,
- unsigned int nkeycodes)
+ unsigned int nkeycodes,
+ bool events)
{
int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
virJSONValuePtr keys = NULL;
- virJSONValuePtr key = NULL;
- size_t i;
+ ssize_t i;
+ size_t nreverts = 0;
+ unsigned int *reverts = NULL;
+
+
+ /*
+ * We are trying to use input-send-event to control press/release
+ * events more precisely, but that API doesn't support the
+ * 'holdtime' parameter (obviously). Due to backward
+ * compatibility, we need to keep the 'holdtime' parameter working
+ * as that is the thing that wasn't broken (yet), unlike the order
+ * of press/release events 'send-key' sends to the guest.
+ *
+ * And we cannot combine 'input-send-event' and 'send-key' because
+ * if there's any error between sending press and release events,
+ * some modifiers might be left pressed.
+ */
+ if (events && holdtime) {
+ events = false;
+ VIR_WARN("Using only send-key even though "
+ "this QEMU binary supports input-send-event");
+ }
/* create the key data array */
if (!(keys = virJSONValueNewArray()))
goto cleanup;
for (i = 0; i < nkeycodes; i++) {
+ bool modifier = virKeycodeValueIsModifier(keycodes[i]);
+
if (keycodes[i] > 0xffff) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("keycode %zu is invalid: 0x%X"), i, keycodes[i]);
goto cleanup;
}
- /* create single key object */
- if (!(key = virJSONValueNewObject()))
+ if (qemuMonitorJSONAppendKey(keys, keycodes[i],
+ events, true) < 0)
goto cleanup;
- /* Union KeyValue has two types, use the generic one */
- if (virJSONValueObjectAppendString(key, "type", "number")
< 0)
- goto cleanup;
-
- /* with the keycode */
- if (virJSONValueObjectAppendNumberInt(key, "data", keycodes[i]) <
0)
- goto cleanup;
+ if (events) {
+ if (modifier) {
+ if (VIR_APPEND_ELEMENT_COPY(reverts, nreverts, keycodes[i]) < 0)
+ goto cleanup;
+ } else {
+ if (qemuMonitorJSONAppendKey(keys, keycodes[i],
+ events, false) < 0)
+ goto cleanup;
+ }
+ }
+ }
- if (virJSONValueArrayAppend(keys, key) < 0)
+ /*
+ * At the end, release all modifiers in reversed order.
+ */
+ for (i = nreverts; i > 0; i--) {
+ if (qemuMonitorJSONAppendKey(keys, keycodes[i - 1],
+ events, false) < 0)
goto cleanup;
+ }
- key = NULL;
-
+ if (events) {
+ cmd = qemuMonitorJSONMakeCommand("input-send-event",
+ "a:events", keys,
+ NULL);
+ } else {
+ cmd = qemuMonitorJSONMakeCommand("send-key",
+ "a:keys", keys,
+ "p:hold-time", holdtime,
+ NULL);
}
- cmd = qemuMonitorJSONMakeCommand("send-key",
- "a:keys", keys,
- "p:hold-time", holdtime,
- NULL);
if (!cmd)
goto cleanup;
@@ -3994,17 +4094,17 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
goto cleanup;
if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- VIR_DEBUG("send-key command not found, trying HMP");
+ VIR_DEBUG("send-key (input-send-event) command not found, trying
HMP");
ret = qemuMonitorTextSendKey(mon, holdtime, keycodes, nkeycodes);
} else {
ret = qemuMonitorJSONCheckError(cmd, reply);
}
cleanup:
+ VIR_FREE(reverts);
virJSONValueFree(cmd);
virJSONValueFree(reply);
virJSONValueFree(keys);
- virJSONValueFree(key);
return ret;
}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 3b6159a..1ddbc65 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -294,7 +294,8 @@ int qemuMonitorJSONInjectNMI(qemuMonitorPtr mon);
int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
unsigned int holdtime,
unsigned int *keycodes,
- unsigned int nkeycodes);
+ unsigned int nkeycodes,
+ bool events);
int qemuMonitorJSONScreendump(qemuMonitorPtr mon,
const char *file);
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 1b202fb..fc40d67 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1964,7 +1964,8 @@ testQemuMonitorJSONqemuMonitorJSONSendKey(const void *data)
goto cleanup;
if (qemuMonitorJSONSendKey(qemuMonitorTestGetMonitor(test),
- 0, keycodes, ARRAY_CARDINALITY(keycodes)) < 0)
+ 0, keycodes, ARRAY_CARDINALITY(keycodes),
+ false) < 0)
goto cleanup;
ret = 0;
@@ -1980,6 +1981,10 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void
*data)
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
int ret = -1;
unsigned int keycodes[] = {43, 26, 46, 32};
+ const char *keys =
"[{\"type\":\"number\",\"data\":43},"
+
"{\"type\":\"number\",\"data\":26},"
+
"{\"type\":\"number\",\"data\":46},"
+
"{\"type\":\"number\",\"data\":32}]";
if (!test)
return -1;
@@ -1987,16 +1992,70 @@ testQemuMonitorJSONqemuMonitorJSONSendKeyHoldtime(const void
*data)
if (qemuMonitorTestAddItemParams(test, "send-key",
"{\"return\":{}}",
"hold-time", "31337",
- "keys",
"[{\"type\":\"number\",\"data\":43},"
-
"{\"type\":\"number\",\"data\":26},"
-
"{\"type\":\"number\",\"data\":46},"
-
"{\"type\":\"number\",\"data\":32}]",
+ "keys", keys,
NULL, NULL) < 0)
goto cleanup;
+ if (qemuMonitorTestAddItemParams(test, "send-key",
+ "{\"return\":{}}",
+ "hold-time", "31337",
+ "keys", keys,
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONSendKey(qemuMonitorTestGetMonitor(test),
+ 31337, keycodes,
+ ARRAY_CARDINALITY(keycodes), false) < 0)
+ goto cleanup;
+
if (qemuMonitorJSONSendKey(qemuMonitorTestGetMonitor(test),
31337, keycodes,
- ARRAY_CARDINALITY(keycodes)) < 0)
+ ARRAY_CARDINALITY(keycodes), true) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ qemuMonitorTestFree(test);
+ return ret;
+}
+
+static int
+testQemuMonitorJSONqemuMonitorJSONSendKeyWithEvents(const void *data)
+{
+ virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+ qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+ int ret = -1;
+ unsigned int keycodes[] = {157, 70, 70};
+
+ if (!test)
+ return -1;
+
+ if (qemuMonitorTestAddItemParams(test, "input-send-event",
+ "{\"return\":{}}",
+ "events", "["
+
"{\"type\":\"key\",\"data\":{\"down\":true"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":157}}},"
+
"{\"type\":\"key\",\"data\":{\"down\":true"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":70}}},"
+
"{\"type\":\"key\",\"data\":{\"down\":false"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":70}}},"
+
"{\"type\":\"key\",\"data\":{\"down\":true"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":70}}},"
+
"{\"type\":\"key\",\"data\":{\"down\":false"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":70}}},"
+
"{\"type\":\"key\",\"data\":{\"down\":false"
+
",\"key\":{\"type\":\"number\","
+ "\"data\":157}}}]",
+ NULL, NULL) < 0)
+ goto cleanup;
+
+ if (qemuMonitorJSONSendKey(qemuMonitorTestGetMonitor(test), 0, keycodes,
+ ARRAY_CARDINALITY(keycodes), true) < 0)
goto cleanup;
ret = 0;
@@ -2393,6 +2452,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetCPUInfo);
DO_TEST(qemuMonitorJSONGetVirtType);
DO_TEST(qemuMonitorJSONSendKey);
+ DO_TEST(qemuMonitorJSONSendKeyWithEvents);
DO_TEST(qemuMonitorJSONGetDumpGuestMemoryCapability);
DO_TEST(qemuMonitorJSONSendKeyHoldtime);
DO_TEST(qemuMonitorSupportsActiveCommit);
--
2.1.3