martinpitt / umockdev

Mock hardware devices for creating unit tests and bug reporting
https://launchpad.net/umockdev
GNU Lesser General Public License v2.1
312 stars 58 forks source link

Server side ioctls #118

Closed benzea closed 3 years ago

benzea commented 3 years ago

This is somewhat work-in-progress. But, it is working now, so opening an MR as there is something that can be discussed.

I think that ideally we would share more code paths between recording and replay. i.e. use a umockdev testbed for recording purposes. But I have not looked into the implications that this may have for the testbed API.

martinpitt commented 3 years ago

Thanks @benzea for your great work here! This is quite a mouthful, so I suggest to spoonfeed this in several smaller PRs. The first for the first five commits, which should be relatively easy. Then one for the commits related to thread → mainloop. And finally one for the ioctl.

FTR, ignore the ubuntu:devel test failure -- this is a glibc bug which can't be worked around at the moment, so I disabled that test for now in commit 0aab5f82fa56 -- so if you rebase your smaller PRs, they should be green.

martinpitt commented 3 years ago

@benzea : Want to rebase this and look at the issues above, so that we don't let this bitrot too long? Thanks!

martinpitt commented 3 years ago

The CentOS 8 failure looks like a too old Vala API (The namenew' does not exist in the context of GLib.Signal'). Unless that's trivial to fix, I'd be happy to stop supporting CentOS 8 now, and move to CentOS 9 stream (once there is a container image).

The valgrind error on Alpine looks more serious though, some portability issue?

martinpitt commented 3 years ago

I tested this branch interactively with evtest, on my "standard AT keyboard":

sudo LD_LIBRARY_PATH=. UMOCKDEV_DEBUG=all ./umockdev-record -i /dev/input/event3=/tmp/i -e /dev/input/event3=/tmp/e -- evtest /dev/input/event3 

This works fine on master (despite issue #96 it does write a tree and evemu file), but it crashes immediately with this branch:

[...]
ioctl_simplestruct_init_from_bin: EVIOCGREP(80084503): size is 8 bytes
ioctl_tree_insert: node of type EVIOCGREP ptr 0x94c320 already exists
ioctl fd 3 request 80084503: emulated, result 0
    Repeat code 0 (REP_DELAY)
      Value    250
    Repeat code 1 (REP_PERIOD)
      Value     33
ioctl_simplestruct_init_from_bin: EVIOCGPROP(80F84509): size is 248 bytes
ioctl_tree_insert: node of type EVIOCGPROP ptr 0x94c320 already exists
ioctl fd 3 request 80F84509: emulated, result 8
Properties:
Testing ... (interrupt to exit)
ERROR: libumockdev-preload: emulation code requested invalid read from 0x1 + 4
ERROR: libumockdev-preload: Error communicating with ioctl socket, errno: 14

@benzea , does this work or fail for you?

benzea commented 3 years ago

Ugh, ok …

So:

martinpitt commented 3 years ago

Whee, nice work @benzea ! As I said, I'm happy to disable centos-8 CI at this point. RHEL 8.4 is out of the door, there is not terribly much demand for newer umockdev versions there IMHO. I'll disable it tomorrow, so ignore that test case now.

I'll give this another review tomorrow, but I'd actually like to merge this RSN before it becomes stale again, so that you can actually use it in fprint. (I don't want to divert your attention even longer :grin: ). Unless you still have some blocker on your list?

benzea commented 3 years ago

Good stuff!

Yeah, it should be quite helpful for libfprint (in particular with the corresponding follow up MRs to add pcap USB replay and eventually also SPI replay hopefully).

benzea commented 3 years ago

And no, no blocker from my side. Unless you see an issue with the exposed API at this point.

Note that for SPI I am planning to also allow overriding write/read (with a separate handle-{write,read} signals). Just in case that is relevant for (API) review.

benzea commented 3 years ago

Of course, feel free to just force push into my branch.

benzea commented 3 years ago

Wait, right, you already merged it! Thanks! :-)