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:56:31AM +0200, Ján Tomko wrote:
> 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.

If you are both strongly against that sure, I personally don't care.

Pavel

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