[05:49] <mborzecki> morning
[06:19] <mardy> mborzecki: hi!
[06:20] <mborzecki> mardy: any new findings since yesterday?
[06:28] <mardy> mborzecki: nope
[06:51] <mup> PR snapd#10848 closed: interfaces/u2f-devices: add GoTrust Idem Key (https://launchpad.net/bugs/1945182) <Created by oSoMoN> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10848>
[06:55] <mborzecki> mvo: thanks for landing https://github.com/snapcore/snapd/pull/10845, i see it's already set for 2.52
[06:55] <mup> PR #10845: interfaces/seccomp: add clone3 to default template <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10845>
[06:56] <mup> PR snapd#10845 closed: interfaces/seccomp: add clone3 to default template <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10845>
[07:01] <pstolowski> morning
[07:06] <mvo> mborzecki: yeah, I cherry picked it already iirc
[07:08] <mborzecki> mvo: thank you!
[07:23] <mardy> good morning pstolowski, mvo 
[07:24] <mardy> mborzecki: this is another debug attempt, I want to see if it would work if systemd handled it all itself: https://github.com/mardy/snapd/commit/7aa7bca91a4a243e06c6a72bd6aacd9cdc12d457
[07:24] <mardy> so far appears to be working, but I'll try in the afternoon, when it usually fails
[07:28] <mardy> mborzecki: actually, even not going that far: since our snap is run in a scope, we do not need to move the PID and create a subgroup, right? (opposed to what I was doing yesterday, when I enabled delegation and continued creating a subgroup)
[07:37] <mborzecki> mardy: hmm actually the docs aren't clear, groups upper in the hierarchy are managed by systemd, below that unit are managed by you, but how about the unit then?
[07:38] <mborzecki> in the meantime, i think we have real trouble with nvidia and new glibc now
[08:21] <mborzecki> mardy: out of curiosity i've added the synchronization with JobRemoved signal, I can propose a branch, so that you can pull it, rebase your changes on top and check whether it's behaving better or not
[08:22] <mborzecki> bit ugly but not too complicated
[08:50] <mardy> mborzecki: ok, though I'm sure it will work (not necessarily because it fixes anything, but just because it adds a delay :-) )
[08:52] <mborzecki> hehe, actually the sync with signal is only marginally slower on my system
[08:58] <pstolowski> jamesh: hi
[08:59] <jamesh> pstolowski: hi
[09:02] <pstolowski> jamesh: do you have a moment for a quick chat about notifications?
[09:02] <jamesh> sure
[09:04] <pstolowski> jamesh: ok, you should have got an invite via email
[10:03] <mborzecki> mardy: busy loop: 1.26ms mean ± 0.3ms, signal wait: 3.2ms mean ± 0.9ms, N=10
[10:03] <mborzecki> on my system
[10:22] <mardy> mborzecki: on the spread the busy loop is a bit more than 4ms
[10:32] <mardy> mborzecki: do you know the reason why we are not using SC_DEVICE_CGROUP_FROM_EXISTING when setting up the cgroup from udev-support.c?
[10:33] <mardy> ah, maybe I'm misinterpreting what that flag does
[10:40] <mborzecki> mardy: hmm?
[10:44] <mardy> mborzecki: I thought that the flag was used to decide if a separate cgroup had to be created, but it appears that it rather decides only if the directory must be created
[10:46] <pstolowski> jamesh: i forgot to address the point about GIcon (what you described under the original forum post), I should probably address this in the already opened PR
[11:09] <mardy> mborzecki: I'm running the tests with another possible fix, which so far seems to be working: setup the devices cgroup in the snap scope, without creating another subgroup. In this way the PID is not moved to another group by snap-confine.
[11:09] <zyga-mbp> that seems like a good idea mardy 
[11:10] <mardy> zyga-mbp: the other change I tested this morning was more drastic, that is leaving it all up to systemd: https://github.com/mardy/snapd/commit/7aa7bca91a4a243e06c6a72bd6aacd9cdc12d457
[11:10] <mardy> zyga-mbp: of course the devices should not be hardcoded like that, it was just for testing :-)
[11:11] <zyga-mbp> remember that hotplug must not regres
[11:11] <zyga-mbp> *regress
[11:11] <mborzecki> mardy: that seems ok and is more in line with v2, I see problems with snap-device-helper though, as right now for v1 and v2 we modify just one resource (group in case of v1, or a map for v2)
[11:15] <mardy> mborzecki: mmm... can you give me some additional pointers?
[11:15] <zyga-mbp> mardy s-d-h modifies the existing cgroup on hotplug events
[11:16] <mardy> mborzecki: ah, but then your comment was referring to the "let it all up to systemd" solution, right?
[11:17] <mborzecki> mardy: no the snap scope thing, i understand you let systemd create /sys/fs/cgroup/devices/..../snap....<uuid>.scope and then modify devices.allow there, right?
[11:17] <mardy> mborzecki: yep
[11:17] <mborzecki> so in this snap-device-helper would have to traverse the whole /sys/fs/cgroup/devices tree looking for groups that are named like snap scopes (which we already do for v2 but for other purpose)
[11:18] <mardy> mborzecki: but we do have a say on the naming of the transient scope, don't we?
[11:19] <mborzecki> mardy: yes, we name in a specifiic way, that's what the current code relies on for instance in refresh app awareness
[11:19] <mborzecki> mardy: but you don't have a say in the orgranization of parent groups afaik
[11:20] <mardy> mborzecki: the transient snap scope is created right under /sys/fs/cgroup/devices/
[11:21] <mardy> ah, I see, you mean that globs don't exist in syscalls, so we'd have to list the contents of /sys/fs/cgroup/devices/ and pick the one which starts with the right prefix; is this what you mean?
[11:21] <mborzecki> mardy: yes, can you paste the output of `tree sys/fs/cgroup/devices`?
[11:23] <mardy> mborzecki: uh... unfortunately not: I'm running the tests, and for some reason I cannot open a secondary ssh connection (it claims that the password is wrong, even though I'm copy-pasting it...)
[11:24] <mardy> oh, the spread machine is stuck :-D
[11:35] <mborzecki> hehe
[11:37] <mborzecki> meh, chasing down tests that do something funny with the session
[11:38] <mborzecki> btw. `su - .. test` enters some silly mode if the test user session isn't up, but if it's up, things work fine, i'm not even sure we need to use `tests.session -u test exec` to run commands, i started fixing tests to use that but noticed along the way that just earlier session prepare is teardown
[11:38] <mborzecki> s/teardown/and teardown is enough/
[11:38] <mup> PR core#125 closed: Copy dpkg.yaml for LP Buildd <Created by ilasc> <Merged by mvo5> <https://github.com/snapcore/core/pull/125>
[11:42] <mardy> got another spread machine, and still it won't accept the password
[11:43] <mup> PR core#126 opened: Makefile: refactor to have a common $TARGET_BASENAME <Created by mvo5> <https://github.com/snapcore/core/pull/126>
[11:44] <mardy> ah, and after I do that, the spread machine gets stuck again :-o
[11:45] <mardy> mvo: so, the problem was the missing '"'??
[12:29] <mvo> mardy: I think the missing ";" actually
[12:29] <mvo> mardy: but let's see, test building this is a bit annoying
[12:47] <mup> PR snapd#10853 opened: tests: use the latest cpu family for nested tests execution <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10853>
[13:42] <ijohnson[m]> cachio_: does debian-sid no longer support NTP for some reason ? https://pastebin.ubuntu.com/p/qCZ3YzwnMP/
[13:42] <ijohnson[m]> first time I've seen that one fail
[13:57] <mborzecki> fun, our dbustest seems to be racy
[13:57] <mvo> does 10173 need a security review or is that good to go as is? (the polkit security backend)
[13:58] <mup> PR snapd#9773 closed: interfaces/apparmor: do not fail during initialization when there is no AppArmor profile for snap-confine <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/9773>
[13:58] <mup> PR snapd#10571 closed: daemon: implement access checkers for themes API <Needs Samuele review> <Created by jhenstridge> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10571>
[14:18] <mborzecki> eh and dbustest seems to a bit flaky with injecting messages
[14:23] <mardy> mborzecki: ok, my fix does not work: sometimes the device cgroup is just set to "/user.slice", even though we are not moving our PID anymore
[14:24] <mardy> mborzecki: what I don't understand is why the loop check we have in tracking.go is not enough. It checks the cgroup that our PID belongs to, and only loops out when it's the correct one.
[14:25] <mardy> is our PID being moved out and in again? Does not make any sense...
[14:27] <mardy> mborzecki: can you push what you have so far? I'd like to try it, since at this time the bug is relatively easy to trigger
[14:46] <mardy> mborzecki: I now notice that cgroup.ProcessPathInTrackingCgroup() returns true if the name=systemd controller matches (for cgroups v1), which is probably not enough (we also want the "devices" controller to be setup)
[14:57] <mborzecki> mardy: the check only ensures that the process is in the right group in the systemd hierarchy, which isn't associated with any controller (at least not in v1)
[14:58] <mborzecki> mardy: my changes are in this branch: https://github.com/bboozzoo/snapd/tree/bboozzoo/snap-run-wait-systemd-group-job-done
[15:05] <mborzecki> mardy: well, it does the right thing, we're asking it for the tracking group, aren't we?
[15:12] <mardy> mborzecki: yes, from the POV of the master branch; not for my modifications (but that's my problem :-D)
[16:03] <mup> PR snapd#10854 opened: spread: use `bios: uefi` for uc20 <Simple 😃> <Skip spread> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10854>
[17:37] <mardy> mborzecki: still there? I've been running the tests on your branch for about 2 hours, it never failed
[17:51] <mborzecki> mardy: yay, that's good news then 😉 let's sync tomorrow morning
[21:29] <mup> PR snapd#10855 opened: tests: reset some mount units failing on ubuntu impish <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10855>