Re: [PATCH] test_driver: implement virDomainSendKey

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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:
> +    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.

But the QEMU API returns immediately. I don't see a reason to
arbitrarily delay the return of the API if it has no functional purpose.

Also, relying on measuring execution time to figure out whether
the test works or not seems unreliable.

Jano


The translation is just a way how to validate whether the keycode was
correct or not as the code translates to the same codeset.

Pavel



--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux