[01:29] PR snapd#9558 opened: cmd/snap, snapmgr, tests: cleanups after #9418 [02:17] PR snapcraft#3342 opened: unit tests: mock os.environ.copy() for deb tests [02:35] PR snapd#9489 closed: daemon,client: write and read a maintenance.json file for when snapd is shut down [02:40] PR snapd#9542 closed: interfaces: deny connected x11 plugs access to ICE <⚠ Critical> [02:40] PR snapd#9559 opened: client, daemon, cmd/snap: cleanups from #9489 + more unit tests [03:50] PR snapd#9560 opened: gadget/many: drop usage of gpt attr 59 for indicating creation of partitions [06:40] good morning [06:59] hey mvo [07:00] hey zyga [07:00] hey mbeierl [07:00] mborzecki, [07:00] morning [07:00] zyga: heya [07:03] hey mborzecki [07:03] mborzecki, last nigth I was cursing at selinux [07:04] mborzecki, I totally failed to fix that denial, after hours of wasted effort trying things [07:07] zyga: heh, i'm pretty sure you're not the only one unhappy with selinux :P [07:08] I felt so lost because the denial and the rules are so far away from each other [07:08] I give up on that, [07:08] I want to actually spend my days on productive progress [07:08] maybe you could look when you have a moment [07:47] hmm not good `13 files changed, 891 insertions(+), 86 deletions(-)` [07:54] need to chop it up into more reviewable bits [08:06] hey pawel [08:06] heya [08:06] PR snapd#9561 opened: secboot: version bump, unlock volume with key [08:06] good morning pstolowski [08:07] mvo: can you take a look at #9561? [08:07] PR #9561: secboot: version bump, unlock volume with key [08:07] pstolowski: hey [08:08] zyga: btw. yday i ran a loop with this test https://github.com/bboozzoo/spread-mini/blob/master/mini/vendor-sync/task.yaml on tumbleweed, 27 iterations, all good [08:09] wonder whteher we're doing something wrong there [08:09] mborzecki: sure [08:14] mborzecki, that's great [08:15] mborzecki, so is that using our images? [08:15] zyga: yes [08:15] that is even more surprising [08:17] * zyga grabbed small breakfast [08:21] PR snapd#9562 opened: tests: improve uc20-create-partitions-reinstall test [08:22] mvo: thanks ;) now i need to get +1 from pedronis and a green merge button [08:32] pedronis, o/ [08:32] good morning [08:32] I've replied to some questions and sent a few small patches in response to others [08:32] let's discuss the rest at 10 [08:32] I will try to make all the changes as fast as possible [08:43] pedronis: hi, can you take a look at #9561? [08:43] PR #9561: secboot: version bump, unlock volume with key [08:43] yes, in a bit [08:48] pedronis: thanks [08:55] mborzecki: can you look at https://github.com/snapcore/snapd/pull/9560 it seems to simple to me [08:55] PR #9560: gadget/many: drop usage of gpt attr 59 for indicating creation of partitions <⛔ Blocked> [08:55] pedronis: yeah, just left a comment there [08:56] heh, ok see you left one too [08:57] * zyga grabs laptop [08:57] mborzecki: I think we use the offset indirectly because we run ensureLayoutCompatibility first [08:57] but is still all a bit fragile [08:59] pedronis: hm it should probably identify the role of the partition based on gadget info from laid out volume and then double check it's the same partition (i.e. look at offset, though a bit of belt & suspenders maybe?) [09:02] loos like ensure layout compatibility runs later [09:05] zyga, hello! [09:05] hi all [09:27] PR snapd#9558 closed: cmd/snap, snapmgr, tests: cleanups after #9418 [09:38] zyga: pasted https://github.com/snapcore/snapd/pull/9546#issuecomment-717813825 [09:38] PR #9546: overlord: add inert export manager [09:38] pedronis, thank you! [09:45] pedronis, one more thing that I just thought of is gadget/kernel copy-exporting assets\ [09:45] pedronis, like device tree files or kernel images copied to FAT [09:45] pedronis, but I think this is all in sync with this approach [10:01] pedronis, changes look good, I think this will be over quickly :-) [10:09] * zyga coffee or tea, depending on what's made upstairs, brb [10:25] re [10:25] pedronis: i have the code that passes luks2 format options, but i wonder whether we should just pick something in the snapd/secboot package rather than expose it all the way to gadget/install, the patch here: https://github.com/snapcore/snapd/commit/603fee44875e73a9c129e06574bf41bcad0eeadd [10:47] PR snapd#9557 closed: tests/snap-advise-command: re-enable test [10:50] mborzecki: I moved something in the TODO list [10:50] mborzecki: should we quickly sync about your question and other bits? [10:51] pedronis: sure, let me jump into standup HO [11:16] pedronis, I ran into one bump, Manifest stores SnapName and ExportedVersion but we mangle core_revision or revision as both "snapd". When you look at ExportedFile.SourcePath you cannot use /snap/$snapName/$revision directly, instead we must unpack that special case there [11:17] I was wondering if this would warrant a distinction between (true) SnapName, SnapRevision and ExportedSnapName and ExportedVersion [11:18] if we were to store SnapName: "core", SnapRevision: "1234", ExportedSnapName: "snapd", ExportedVersion: "core_1234" then we would not have to parse anything [11:18] we might even make ExportedSnapName and ExportedVersion omitempty, and fall back to SnapName/SnapVersion [11:19] mborzecki, pedronis 9561 is green, ok if I merge? [11:19] I think this is better than trying to bake the logic there [11:19] for clarity, I am trying to write a pair of functions ExportedFilePathName and ExportedFileSourceName [11:19] that both take the following arguments: manifest *Manifest, set *ExportSet, exported *ExportedFile [11:31] pedronis, I made those modifications, I will post this soon [11:31] it's much more natural now [11:32] sorry, I was syncing with Maciej about something else [11:32] PR snapd#9561 closed: secboot: version bump, unlock volume with key [11:33] for reference https://paste.ubuntu.com/p/vt7Xg4frcH/ [11:34] mmh, I'll need to look, I worry a bit if it's confusing or not confusing [11:35] I need to go have lunch [12:09] zyga: it's probably fine but we need to be careful how we name things and where we put the fields [12:10] mvo: updated #9553 [12:10] PR #9553: tests: add spread test for refreshing from an old snapd and core18 [12:12] pstolowski: thank you! and sorry that I did not think about this earlier [12:12] np! [12:14] zyga, hey [12:14] zyga, I left a comment https://github.com/snapcore/snapd/pull/9556#issuecomment-717890459 [12:14] PR #9556: tests: testing new fedora 33 image [12:14] do you know how to fix that? [12:24] pedronis, ack [12:24] cachio, looking [12:25] oh that's serious [12:25] but again related to how we test things IMO [12:25] mborzecki, ^ fedora bumped libc? [12:25] new libc baked into snap-exec [12:25] I thougth we made it static [12:25] but perhaps not really? [12:25] hm maybe?, it should be static, there's a check in snapd.spec for it [12:26] : /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by /usr/lib/snapd/snap-exec) [12:32] * mvo hugs mborzecki for 9528 [12:34] zyga: I didn't notice this yesterday, bit confused: https://github.com/snapcore/snapd/pull/9546/files#r513405431 [12:34] PR #9546: overlord: add inert export manager [12:36] mmmm, replied [12:36] do you want to re-think? [12:36] should I pause the changes? [12:37] I think, as a first instinct, that this _is_ fine [12:37] especially with what we said about a higher level mapper for things like aliases/man pages [12:37] zyga: i can take a look a bit later [12:37] mborzecki, thank you! [12:38] pedronis, as one more data point, this is exactly why you designed core_$rev as the exported name before [12:43] How do I completely reset a part in snapcraft that pulls in stage-packages? I'm testing which packages are required for a cmake-based build; is snapcraft clean my-part && apt autoremove enough? [12:44] * zyga doesn't know, sorry [12:45] * ogra just uese a fresh lxd container for such cases [12:45] *uses [12:46] ogra: That would be my go-to approach if it would't need ~ 10min for each run to get to the part I'm working on 🙈 [12:51] zyga: do you have some time for #9535? [12:51] PR #9535: o/snapstate: generate snapd snap wrappers again after restart on refresh [12:51] yes, I remember that [12:55] pstolowski, why is the new logic only applied to core? [12:55] zyga: only on core we generate wrappers. this check is repeated anyway in AddSnapdServices in wrappers [12:55] ah, I see [12:59] zyga: two things come to mind, in general what is in .../export/* is really the input for another round of indirection in most cases, this is a bit hidden right now because we have only use case. because of that though there is really no good reason to do the dance we are doing conflating snapd/core and host for anything but snapd [12:59] *only 1 use case [12:59] zyga: heh, so snap-exec is not statis, wtf is our check doing even [12:59] mborzecki, ;D [12:59] mborzecki, it's on strike today [13:00] haha, i think i know [13:01] pedronis, if we ever want to remove that special case we could create one more layer which updates snapd-tools -> core/tools or host/tools or otherwise, so I think we are safe here [13:01] mborzecki, ldd is naive [13:01] mborzecki, readelf FTW [13:01] zyga: ldd is good enough, just that it prints to stderr now apparently [13:01] zyga: yes, we could do that. but my point here is more that we need to make clear that the strange bits are really only for this special case [13:02] zyga: and the check is ldd .. | grep withouth 2>&1 [13:02] without [13:02] mborzecki, readelf has issues with some things in my experience but I'm happy the issue is easier this time :) [13:02] zyga: there's no reason to abuse them later for something else, it's probably a bad idea even [13:02] pedronis, ack [13:03] pstolowski, approved [13:04] zyga: thank! [13:04] *thanks [13:08] PR snapd#9535 closed: o/snapstate: generate snapd snap wrappers again after restart on refresh [13:08] PR snapd#9563 opened: secboot: set metadata and keyslots sizes when formatting LUKS2 volumes [13:12] zyga: fun fact, i had a fix for f33, but not proposed yet, it's a sync between fedora packaging and our snapd.spec in tree [13:14] mborzecki, related https://forum.snapcraft.io/t/apps-wont-run-and-crash-on-gnome-wayland/20322? [13:14] mborzecki, or is that for the x11 socket? [13:14] ah sorry [13:14] ignore em [13:14] *me [13:23] PR snapd#9564 opened: secboot: fix doc comment on helper for unlocking volume with key [13:23] pedronis: NewRequestWithContext is available from go 1.13 :( they actually show version numbers, see https://golang.org/pkg/net/http/#NewRequestWithContext (it's on right-hand side) [13:24] sad :( [13:25] pedronis: is upgrade out of question [13:25] ? [13:25] we need to talk with mvo [13:26] I think there's a older way to do what we want [13:26] is just messier [13:27] I didn't notice the versions on the right, I suppose is recent-ish change [13:30] pstolowski: btw I think we do set some other timeouts via httputil/transport.go [13:31] pstolowski: the old way is to use cli.Transport.CancelRequest [13:33] pedronis: mborzecki: perhaps we can chat sometime this morning re gpt attrs ? [13:33] I guess I'm a bit confused how to implement it using offsets given that we essentially already do that exact check in verifyLayoutCompatibility [13:33] ijohnson: i'm ok with after/before standup [13:35] ijohnson: I don't think the issue is strictly the offsetts, you reasoning is not wrong, the issue is more about what is the marker/what we correlate with, the fs label might not be set [13:35] pedronis: hmm, i missed that, got confused by &ClientOptions{} and Timeout: opts.Timeout in NewHTTPClient [13:36] pedronis: ok, yeah I suppose it makes sense to not use the fs label in that case and more rely on the partition label, but again we won't have the partition label on mbr [13:36] let me have a look at the code again then perhaps we can chat after the SU [13:36] ijohnson: we could use offset everywhere then? [13:36] anyway my other issue with the PR as is, is that the flag seems more and more of a danger [13:36] ijohnson: offet will work for gpt and mbr [13:37] pedronis: I think as you said we need to just get rid of that CreatedDuringInstall flag entirely and then just compare the offsets after we have both the ondisk and the laidout from the gadget [13:37] mborzecki: right [13:38] ijohnson: you'll probably need to thing about fitting that with the rest of the code, iirc ensureLayoutsCompatible runs later and looks at IsCreatedDuringInstall flag [13:38] does it? [13:38] it says it does [13:38] but I missed where [13:38] mborzecki: I think we should just drop that IsCreatedDuringInstall flag because yes ensureLayoutsCompatible runs after we build up the on disk layout (obviously) [13:39] I think I know what to do now [13:39] I just need to see if I know _how_ to do it :-) [13:39] mmh, it does [13:39] so I'm confused indeed [13:40] hip hip hippo! [13:42] shave it ! [13:43] pedronis: CancelRequest is marked deprecated in current go docs, not a huge problem but means we may need to redo this in the future [13:44] yea, I know [13:46] * ijohnson is slightly sad my suggestion to use hyrax was not taken [13:46] hyrax? [13:46] * zyga googles [13:46] it's like a little african prairie dog [13:46] oh wow [13:46] all the new words :) [13:46] I guess my comparison requires you know what a prairie dog looks like :-) [13:46] * ogra would have voted for homunculus [13:46] hasty hummus [13:47] (given it is the first one after an LTS that seemed to fit) [13:47] * ijohnson plans to try again in 13 years [13:47] haha [13:50] mborzecki: you bumped secboot to current tip in your PRs? [13:50] pedronis: yes [13:51] pedronis: hmmm [13:53] pedronis: not quite the tip apparently, it's a this commit https://github.com/snapcore/secboot/commit/7c5ed6888746f5a798e475c02d23c08678a5ede8 [13:53] ah, so will need another bump [13:53] yup, the activation with multiple keys isn't there [14:00] cannot join [14:00] one sec [14:09] zyga, LOL ... did you see the journal at LP#1898869 ? [14:09] in a call [14:09] its an atom laptop 4GB that runs boinc ! [14:10] +with 4GB [14:17] ogra, I'm running focal on a core2 duo with 3GB ram and it runs, pretty great actually [14:18] zyga, yeah, but you didnt install BOINC ... and a ton of broken "screen share" gnome-shell extensions [14:19] and your focal surely also doesnt have ifupdown installed (which seems to confise NM) [14:19] i'm refreeing to https://bugs.launchpad.net/bugs/1898869 [14:19] Bug #1898869: System slow on booting

[14:20] in the light of what he installed it is actually booting pretty fast [14:24] ogra, wait, what is boinc? [14:24] I thought you said "bionic" [14:24] the successor of "seti at home" [14:25] https://en.wikipedia.org/wiki/Berkeley_Open_Infrastructure_for_Network_Computing [14:26] LOL [14:27] so someone installs all possible 3rd party SW that puts high load on the machine on boot ... uses the lowest end CPU, 4GB RAM and a rotary disk and complains about 1.5min boot time 🙂 [14:27] i'd say that system actually flies given what it has installed 🙂 [14:32] * pstolowski lunch [14:48] PR snapd#9553 closed: tests: add spread test for refreshing from an old snapd and core18 [14:49] Issue core20#93 opened: forward port things to 22 branch [14:49] PR core20#91 closed: hooks: add /var/lib/snapd/save [14:59] * cachio lunch [15:04] xnox: thanks! [15:14] pedronis, I've pushed the big change, I'll look at the SetStatus part now but it seems to be somewhat problematic [15:15] the task has now DoneStatus even though it is undone [15:15] on disk behavior is correct [15:15] but I wanted to ask if that is expected [15:20] pedronis, a few misc comments remain but I think the bulk of it is good now [15:21] I'll break for an hour and rebase the rest on top to ensure that it still works end-to-end [15:48] PR snapd#9564 closed: secboot: fix doc comment on helper for unlocking volume with key [15:49] rebased, running export smoke test now [15:54] zyga: there' something off about instances (because of how we do current), there's is probably a bit more to simplify [15:56] mmm [15:58] found a trivial one liner bug, will add some tests and push again [15:59] I will look at instances after that [16:00] pedronis, if we do .../export/instance_key/revsion directly we can indeed simplify [16:00] pedronis, neither bases nor snapd allow instances so that's compatible with the goal [16:00] pedronis, we could also eventually allow that, for whatever reason, by having the mapping layer we discussed [16:00] so I think that yes, we can further simplify this [16:02] zyga: when we discussed this I didn't consider that most other cases will need another indirection anyway [16:02] yeah, this turns the export manager mostly into a "mechanical" layer [16:02] which is good [16:04] I wonder if we should do the snapd special case differently, perhaps by using a different symlink which picks current tool provider, pointing at snapd/current/tools, core/current/tools or host/current/tools [16:04] then the whole special case would go away and only one, distinct symlink would be custom [16:06] zyga: that would need more design [16:09] pedronis, I've rebased the other branch, tests are passing, [16:09] pedronis, this change was entirely self-contained, good :) [16:15] ugh gadget tests are very difficult to write - they require doing arithmetic! [16:16] ijohnson, easy as pi [16:16] it's as easy as finding a rational representation of pi [16:17] you are being irrational! [16:17] * zyga shoots upstairs to check up on kids [16:17] more export manager push tomorrow [16:59] mvo: still around? can you merge #9528 ? the failure on 18.04 is mount-ns:inherit [16:59] PR #9528: cmd/snap-bootstrap: mount ubuntu-save during boot if present [16:59] that test has been failing more consistently recently [16:59] for whatever reason the cgroup mountsh ave different options [17:01] heh [17:07] mborzecki: sure, let me do this [17:08] mborzecki: done, I guess that means that 9563 is now also unblocked? [17:09] PR snapd#9528 closed: cmd/snap-bootstrap: mount ubuntu-save during boot if present [17:09] mvo: thanks, yes, i'll merge and push [17:10] mborzecki: \o/ [17:14] PR snapd#9565 opened: [RFC] overlord/devicestate: bind mount ubuntu-save under /var/lib/snapd/save on startup [17:14] mvo: pedronis: ^^ [17:15] and #9563 has been updated too [17:15] PR #9563: secboot: set metadata and keyslots sizes when formatting LUKS2 volumes [17:16] mborzecki: thanks [17:17] pedronis: it does a mount via systemd-mount, felt it may be a bit cleaner and easier to test than fiddling with syscall.Mount again [17:17] anyways, need to wrap it up and help kids with the homework [17:17] sounds good [17:22] pedronis: I'm working on the new location for lockout/policy-auth. should the recovery key also change location (I guess not but wanted to double check) [17:28] mvo: they are in /var/lib/snapd/device/fde right now ? [17:29] pedronis: correct [17:29]