snapd
snapd copied to clipboard
many: implementation file monitoring for snap cgroups
cgroup: Add Snap/Cgroup monitoring capabilities (alternative MR)
This is an alternative MR for adding a Snap/Cgroup monitor that allows other parts of the code to wait until all the instances of a Snap have ended and it can be safely refreshed. It is the first step for a bigger MR that will allow to automagically refresh a snap after the system has notified the user that there is a refresh available and that they must close it.
@mvo5 Ok, this should be ready for another round...
@jhenstridge Ok, after testing the complete snapd I found (and remembered) why I used InDelete instead of InDeleteSelf: for some reason, InDeleteSelf isn't triggered in /sys/fs, so we must monitor the parent folder.
@jhenstridge Ok, after testing the complete snapd I found (and remembered) why I used InDelete instead of InDeleteSelf: for some reason, InDeleteSelf isn't triggered in /sys/fs, so we must monitor the parent folder.
Looking at the cgroups(7) man page, it seems to suggest monitoring the cgroup.events file. If the file system behaves weirdly, then maybe it'd be worth structuring the code to use that logic. Something like:
- For each cgroup
$dir
, add a watch for$dir/cgroup.event
forIM_MODIFY | IM_DELETE_SELF
. - When a notification is received for the watch, the cgroup should be considered gone if (a) the "populated" line in the file rrrreads 0, or (b) the file is not present.
Looking at the cgroups(7) man page, it seems to suggest monitoring the cgroup.events file. If the file system behaves weirdly, then maybe it'd be worth structuring the code to use that logic. Something like:
The problem is that the cgroup.events file seems to be exclusive of cgroups V2...
@jhenstridge Can you check if all is fine? It seems that there is a pending change request from you, but I can't find it...
@jhenstridge Ok, changes done to the inotify part, and added an extra test for them. Ready for another review round.
Squashed all the commits, as requested.
Thanks for this. I did an incomplete pass and have some questions. I'll go over the parts I didn't read once I re-review
Hey @sergio-costas, is this ready for a re-review? I see a few unanswered comments so I'm just checking before going through it again
@MiguelPires Ops! I clearly forgot some of them. Give me some minutes to end another thing, and I will fix them.
@MiguelPires Ops! I clearly forgot some of them. Give me some minutes to end another thing, and I will fix them.
No worries, take your time :slightly_smiling_face: I'll re-review once you're ready
@MiguelPires Fixed the missing error check in AddWatcher.