/srv/irclogs.ubuntu.com/2021/09/28/#snappy.txt

mborzeckimorning05:49
mardymborzecki: hi!06:19
mborzeckimardy: any new findings since yesterday?06:20
mardymborzecki: nope06:28
mupPR 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:51
mborzeckimvo: thanks for landing https://github.com/snapcore/snapd/pull/10845, i see it's already set for 2.5206:55
mupPR #10845: interfaces/seccomp: add clone3 to default template <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10845>06:55
mupPR snapd#10845 closed: interfaces/seccomp: add clone3 to default template <Simple 😃> <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/10845>06:56
pstolowskimorning07:01
mvomborzecki: yeah, I cherry picked it already iirc07:06
mborzeckimvo: thank you!07:08
mardygood morning pstolowski, mvo 07:23
mardymborzecki: 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/7aa7bca91a4a243e06c6a72bd6aacd9cdc12d45707:24
mardyso far appears to be working, but I'll try in the afternoon, when it usually fails07:24
mardymborzecki: 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:28
mborzeckimardy: 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:37
mborzeckiin the meantime, i think we have real trouble with nvidia and new glibc now07:38
mborzeckimardy: 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 not08:21
mborzeckibit ugly but not too complicated08:22
mardymborzecki: ok, though I'm sure it will work (not necessarily because it fixes anything, but just because it adds a delay :-) )08:50
mborzeckihehe, actually the sync with signal is only marginally slower on my system08:52
pstolowskijamesh: hi08:58
jameshpstolowski: hi08:59
pstolowskijamesh: do you have a moment for a quick chat about notifications?09:02
jameshsure09:02
pstolowskijamesh: ok, you should have got an invite via email09:04
mborzeckimardy: busy loop: 1.26ms mean ± 0.3ms, signal wait: 3.2ms mean ± 0.9ms, N=1010:03
mborzeckion my system10:03
mardymborzecki: on the spread the busy loop is a bit more than 4ms10:22
mardymborzecki: 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:32
mardyah, maybe I'm misinterpreting what that flag does10:33
mborzeckimardy: hmm?10:40
mardymborzecki: 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 created10:44
pstolowskijamesh: 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 PR10:46
mardymborzecki: 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-mbpthat seems like a good idea mardy 11:09
mardyzyga-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/7aa7bca91a4a243e06c6a72bd6aacd9cdc12d45711:10
mardyzyga-mbp: of course the devices should not be hardcoded like that, it was just for testing :-)11:10
zyga-mbpremember that hotplug must not regres11:11
zyga-mbp*regress11:11
mborzeckimardy: 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:11
mardymborzecki: mmm... can you give me some additional pointers?11:15
zyga-mbpmardy s-d-h modifies the existing cgroup on hotplug events11:15
mardymborzecki: ah, but then your comment was referring to the "let it all up to systemd" solution, right?11:16
mborzeckimardy: 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
mardymborzecki: yep11:17
mborzeckiso 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:17
mardymborzecki: but we do have a say on the naming of the transient scope, don't we?11:18
mborzeckimardy: yes, we name in a specifiic way, that's what the current code relies on for instance in refresh app awareness11:19
mborzeckimardy: but you don't have a say in the orgranization of parent groups afaik11:19
mardymborzecki: the transient snap scope is created right under /sys/fs/cgroup/devices/11:20
mardyah, 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
mborzeckimardy: yes, can you paste the output of `tree sys/fs/cgroup/devices`?11:21
mardymborzecki: 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:23
mardyoh, the spread machine is stuck :-D11:24
mborzeckihehe11:35
mborzeckimeh, chasing down tests that do something funny with the session11:37
mborzeckibtw. `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 teardown11:38
mborzeckis/teardown/and teardown is enough/11:38
mupPR core#125 closed: Copy dpkg.yaml for LP Buildd <Created by ilasc> <Merged by mvo5> <https://github.com/snapcore/core/pull/125>11:38
mardygot another spread machine, and still it won't accept the password11:42
mupPR core#126 opened: Makefile: refactor to have a common $TARGET_BASENAME <Created by mvo5> <https://github.com/snapcore/core/pull/126>11:43
mardyah, and after I do that, the spread machine gets stuck again :-o11:44
mardymvo: so, the problem was the missing '"'??11:45
mvomardy: I think the missing ";" actually12:29
mvomardy: but let's see, test building this is a bit annoying12:29
mupPR snapd#10853 opened: tests: use the latest cpu family for nested tests execution <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10853>12:47
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 fail13:42
mborzeckifun, our dbustest seems to be racy13:57
mvodoes 10173 need a security review or is that good to go as is? (the polkit security backend)13:57
mupPR 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
mupPR 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>13:58
mborzeckieh and dbustest seems to a bit flaky with injecting messages14:18
mardymborzecki: ok, my fix does not work: sometimes the device cgroup is just set to "/user.slice", even though we are not moving our PID anymore14:23
mardymborzecki: 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:24
mardyis our PID being moved out and in again? Does not make any sense...14:25
mardymborzecki: can you push what you have so far? I'd like to try it, since at this time the bug is relatively easy to trigger14:27
mardymborzecki: 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:46
mborzeckimardy: 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:57
mborzeckimardy: my changes are in this branch: https://github.com/bboozzoo/snapd/tree/bboozzoo/snap-run-wait-systemd-group-job-done14:58
mborzeckimardy: well, it does the right thing, we're asking it for the tracking group, aren't we?15:05
mardymborzecki: yes, from the POV of the master branch; not for my modifications (but that's my problem :-D)15:12
mupPR snapd#10854 opened: spread: use `bios: uefi` for uc20 <Simple 😃> <Skip spread> <Created by mvo5> <https://github.com/snapcore/snapd/pull/10854>16:03
mardymborzecki: still there? I've been running the tests on your branch for about 2 hours, it never failed17:37
mborzeckimardy: yay, that's good news then 😉 let's sync tomorrow morning17:51
mupPR snapd#10855 opened: tests: reset some mount units failing on ubuntu impish <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/10855>21:29

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!