Introduce a new pidfd based set of D-Bus APIs #173
Conversation
|
Sorry for the delay. I added a few comments on one of the commits, could you check those? Other than that, it seems to work for me on both kernel 5.2 (without pidfd_open) and 5.3. |
Either we are in a flatpak or not, this doesn't change, so we can just remember the result.
Separate the D-Bus messaging code from gamemode_request().
Comments were not reflecting the what they were describing.
Specify the include directory in the link_daemon_common dependency and thus everything that includes that as a dependency will get the proper include directory automatically.
Much like the auto-closing helper for file descriptors, add a new auto-free helper that is meant to be used with dynamically allocated memory, a la: autofree char *data = NULL; ... data = malloc(size); When data goes out of scope, cleanup_free will be called with &data, i.e. cleanup_free(&data), which in turn will call free(3) data. In order to work with all types, e.g. 'char *' (resulting in char ** being passed to cleanup_free) or 'int *' (resulting in int ** being passed to cleanup_free), cleanup_free is defined to work with void *, hence the odd-looking cast: void *target = *(void **) ptr.
Add functions to open pidfds, i.e. file descriptors representing processes, for process ids and vice versa. Both functions work an array of fds/pids, stop on error and return the number of successfully handled items.
Provide a new set of APIs with identical semantics as the existing ByPID family of calls but instead of working with process ids, they take pidfds, file descriptors representing processes, instead. The fds can be translated back to pids (in the correct namespace) and also be monitored via select/poll/epoll. The current implementation translates them directly back to pids, but in the future the monitoring code that watches processes (if they are still alive) could be converted be event driven via pidfds.
Use a small subset of the existing common files to create another static library to be used from the client library.
So that lib can depend on the new library from common.
|
I just found that the supervisor tests ( |
I see it too, let me have a look |
Try to make API requests using the new pidfd based APIs. If getting the pidfds fails or if the remote (daemon) does not support the new pidfd based D-Bus API, transparently fall back to the old API.
|
Ok, I had |
|
That's working for me now, merged. Thanks! I was also seeing an occasional failure on "Gamemoderun and reaper thread tests", if that's what you were getting. I've increased the timeout for that. |
Great. I did that too locally but did not push the commit apparently. Thanks for that and the review! |
Introduce a new pidfd based set of D-Bus APIs
Pidfds are process file descriptors representing processes. Since Linux 5.2 the corresponding pid in the callers pid namespace can be obtained via the fdinfo in procfs; this by reading the "Pid:" field in '/proc/self/fdinfo/'. In Linux 5.3 the
pidfd_opensystem call was added, allowing us to easily open a pidfd for a given pid. Additionally select/poll/epoll can be used to monitor the status of the process, which can be used to watch for process termination.Add a new set of APIs, analogous to the existing *byPID APIs, that work with pidfds instead of pids and convert the client library to default to using those, if supported by the system. The two main advantages are:
pidfds drastically simplify the container use case, because there is no need to translate the pids between different namespaces anymore
it prepares the use of pidfds in the daemon, which in the future could be used to monitor process terminations and thus removes the need of the reaper background thread.