[libvirt] [PATCH] test_driver: implement virDomainSendKey

[Erik Skultety]

On Mon, Jun 03, 2019 at 08:40:13AM +0200, Pavel Hrdina wrote:
> On Mon, Jun 03, 2019 at 08:23:57AM +0200, Erik Skultety wrote:
> > On Sat, Jun 01, 2019 at 02:46:56PM +0200, Ilias Stamatis wrote:
> > > Validate @keycodes and sleep for @holdtime before successfully
> > > returning.
> > >
> > > Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> > > ---
> > >  src/test/test_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >
> > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > > index 2f58a1da95..51ded256be 100644
> > > --- a/src/test/test_driver.c
> > > +++ b/src/test/test_driver.c
> > > @@ -64,6 +64,7 @@
> > >  #include "virinterfaceobj.h"
> > >  #include "virhostcpu.h"
> > >  #include "virdomainsnapshotobjlist.h"
> > > +#include "virkeycode.h"
> > >
> > >  #define VIR_FROM_THIS VIR_FROM_TEST
> > >
> > > @@ -5980,6 +5981,44 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
> > >      return ret;
> > >  }
> > >
> > > +static int
> > > +testDomainSendKey(virDomainPtr domain,
> > > +                  unsigned int codeset,
> > > +                  unsigned int holdtime,
> > > +                  unsigned int *keycodes,
> > > +                  int nkeycodes,
> > > +                  unsigned int flags)
> > > +{
> > > +    int ret = -1;
> > > +    size_t i;
> > > +    virDomainObjPtr vm = NULL;
> > > +
> > > +    virCheckFlags(0, -1);
> > > +
> > > +    if (!(vm = testDomObjFromDomain(domain)))
> > > +        goto cleanup;
> > > +
> > > +    if (virDomainObjCheckActive(vm) < 0)
> > > +        goto cleanup;
> > > +
> > > +    for (i = 0; i < nkeycodes; i++) {
> > > +        if (virKeycodeValueTranslate(codeset, codeset, keycodes[i]) < 0) {
> > > +            virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +                           _("invalid keycode %u of %s codeset"),
> > > +                           keycodes[i],
> > > +                           virKeycodeSetTypeToString(codeset));
> > > +            goto cleanup;
> > > +        }
> > > +    }
> > > +
> > > +    usleep(holdtime * 1000);
> >
> > I think this API should be merely about translating the value, because
> > this way you're just blocking a synchronous API for no apparent benefit. I
> > believe it should return instantaneously. On the other hand, I'm just wondering
> > what value it brings having an API translating keycodes, but I guess for
> > consistency reasons, we can happily introduce it.
>
> The benefit is to simulate the holdtime as for example hyperv driver is
> doing, in QEMU we just pass everything to the QEMU itself where that one
> will probably do similar operation.

Both hyperv and vbox have to do it because they don't support it, so they do
usleep before sending down and up keycodes respectively, QEMU does it
iternally. In test driver, you're not testing any true codes, so doing plain
usleep si IMHO wrong, from app dev's perspective you're only testing whether
usleep works by providing arbitrary time to wait, which raises the obvious
question "why".

Erik