[libvirt] [PATCH v3 4/5] hyperv: support virDomainSendKey

[Matthias Bolte]

2017-04-24 20:19 GMT+02:00 Sri Ramanujam <sramanujam at datto.com>:
> This commit adds support for virDomainSendKey. It also serves as an
> example of how to use the new method invocation APIs with a single
> "simple" type parameter.
> ---
>  src/hyperv/hyperv_driver.c            | 85 ++++++++++++++++++++++++++++++++++
>  src/hyperv/hyperv_wmi.c               |  7 +++
>  src/hyperv/hyperv_wmi.h               |  3 +-
>  src/hyperv/hyperv_wmi_generator.input | 86 +++++++++++++++++++++++++++++++++++
>  4 files changed, 180 insertions(+), 1 deletion(-)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 0ca5971..9562d5a 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -35,6 +35,7 @@
>  #include "hyperv_wmi.h"
>  #include "openwsman.h"
>  #include "virstring.h"
> +#include "virkeycode.h"
>
>  #define VIR_FROM_THIS VIR_FROM_HYPERV
>
> @@ -1373,6 +1374,89 @@ hypervConnectListAllDomains(virConnectPtr conn,
>  #undef MATCH
>
>
> +static int
> +hypervDomainSendKey(virDomainPtr domain, unsigned int codeset,
> +        unsigned int holdtime ATTRIBUTE_UNUSED, unsigned int *keycodes,
> +        int nkeycodes, unsigned int flags)
> +{
> +    int result = -1;
> +    size_t i = 0;
> +    int keycode = 0;
> +    int *translatedKeycodes = NULL;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    char *selector = NULL;
> +    Msvm_ComputerSystem *computerSystem = NULL;
> +    Msvm_Keyboard *keyboard = NULL;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    hypervInvokeParamsListPtr params = NULL;
> +
> +    virCheckFlags(0, -1);
> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0)
> +        goto cleanup;
> +
> +    virBufferAsprintf(&query,
> +            "associators of "
> +            "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> +            "Name=\"%s\"} "
> +            "where ResultClass = Msvm_Keyboard",
> +            uuid_string);
> +
> +

Unnecessary extra empty line.

> +    if (hypervGetMsvmKeyboardList(priv, &query, &keyboard) < 0)
> +        goto cleanup;
> +
> +    /* translate keycodes to xt and generate keyup scancodes. */
> +    translatedKeycodes = (int *) keycodes;

You cannot translate the keycodes in-place. For the VBox and QEMU
drivers this is kind of okay, because they are always behind the
remote driver. But for the Hyper-V driver this is different. You're
directly modifying user provided memory here. This is not okay, you
need to make a copy of the keycodes before translating them.

> +    for (i = 0; i < nkeycodes; i++) {
> +        if (codeset != VIR_KEYCODE_SET_WIN32) {
> +            keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_WIN32,
> +                    translatedKeycodes[i]);
> +
> +            if (keycode < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("Could not translate keycode"));
> +                goto cleanup;
> +            }
> +            translatedKeycodes[i] = keycode;
> +        }
> +    }
> +
> +    if (virAsprintf(&selector,
> +                "CreationClassName=Msvm_Keyboard&DeviceID=%s&"
> +                "SystemCreationClassName=Msvm_ComputerSystem&"
> +                "SystemName=%s", keyboard->data.common->DeviceID, uuid_string) < 0)
> +        goto cleanup;
> +
> +    /* type the keys */
> +    for (i = 0; i < nkeycodes; i++) {
> +        char keycodeStr[sizeof(int) * 3 + 2];

Use "char keycodeStr[INT_BUFSIZE_BOUND(int)];" instead.

> +        snprintf(keycodeStr, sizeof(keycodeStr), "%d", translatedKeycodes[i]);
> +
> +        /* params obj takes ownership of selector */
> +        params = hypervInitInvokeParamsList(priv, "TypeKey", selector,
> +                Msvm_Keyboard_WmiInfo);
> +        if (hypervAddSimpleParam(params, "keyCode", keycodeStr) < 0)
> +            goto cleanup;
> +
> +        if (hypervInvokeMethod(priv, params, NULL) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not press key %d"),
> +                           translatedKeycodes[i]);
> +            goto cleanup;
> +        }

This is not correct. If virDomainSendKey is called with more than one
keycode than all those keys have to be pressed at once. This allows to
send key combination e.g. Alt+F4. But the way you invoke the TypeKey
function here will just send them one after another. Also you
currently don't handle holdtime.

I think you can make both things work by using
PressKey/Sleep/ReleaseKey instead of TypeKey. See how the VBox driver
does it:

http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/vbox/vbox_common.c#l7551


-- 
Matthias Bolte
http://photron.blogspot.com