[06:58] morning [07:02] good morning [07:02] mvo: I've shut down half of the workers now [07:02] let me know if there is any disruption [07:06] zyga: hey [08:01] morning [08:01] hey pstolowski [08:02] zyga: thank you, excellent [08:05] PR snapd#9724 closed: boot: observe successful command line update, provide a default [08:10] PR snapd#9718 closed: secboot,devicestate: add scaffoling for "fde-reveal-key" support

[08:18] pstolowski: mvo; hey [08:29] mborzecki: good morning! [08:55] #9730 needs 2nd reviews [08:55] PR #9730: hookstate: refactor around EphemeralRunHook [09:17] the apparmor support situation on opensuse leap is confusing, on the one hand we downgrade the policy template, on the other snap-confine is built without apparmor support, so why bother with any support at all? [09:18] and there's a special case for tw, where it's built with apparmor support and no policy downgrade [09:19] so on leap we generate some profiles, but never transition to them [09:27] Bug #1906621 opened: System doesn't do FDE when installing with secure boot enabled [09:30] Bug #1906621 changed: System doesn't do FDE when installing with secure boot enabled [09:36] Bug #1906621 opened: System doesn't do FDE when installing with secure boot enabled [09:42] Bug #1906621 changed: System doesn't do FDE when installing with secure boot enabled [09:49] pedronis: bug filed here: https://bugs.launchpad.net/snapd/+bug/1906622 [09:49] Bug #1906622: UC20 system fails to seed when model contains snaps requiring experimental.user-daemons or dbus-activation [09:50] * mvo hugs mborzecki for reviewing 9695 [09:51] Bug #1906621 opened: System doesn't do FDE when installing with secure boot enabled [10:01] jamesh: thx [10:44] pedronis: hey, does this structure look sensible? https://pastebin.ubuntu.com/p/tCSc7wznby/ [10:56] PR snapd#9738 opened: secboot: use `fde-reveal-key` if available to unseal key [10:58] mborzecki: do you remember why we have EffectiveRole and EffectiveLabel, they don't to be used very consistently (especially for new UC20 stuff) ? [10:58] *dont' seem [11:00] pedronis: iirc it was some backward compatibity hacks where things were left unspecified ing adget yaml [11:00] mborzecki: it's a bit of a mess [11:00] new code seems ignore them [11:01] which seems not to provoke immediate problems, but long term is not a good state of things [11:02] pedronis: it should be used wherever we check Role, but i agree that it's not used and should be fixed [11:02] same holds for label [11:02] or we should set things to the value [11:02] having Label and EffectiveLabel is just asking to get it wrong [11:03] or have only the accessor [11:03] anyway bit of a tangle atm [11:03] * pstolowski going to the dentist [11:03] though there will probably be some weird undocumented cases where it's supposed to be explicit [11:03] pedronis: i can put it my queue [11:03] mborzecki: well, I'm looking into it right now because of the issue from yesterday [11:03] mborzecki: I'll see how far I can get [11:03] pedronis: ok [11:06] pstolowski: let's chat this afternoon, sorry, I was chasing something else [11:31] PR snapd#9739 opened: interfaces/apparmor: fix generating of extended s-c AppArmor profiles with /usr/libexec [11:41] PR snapd#9740 opened: packaging/opensuse: enable AppArmor on Leap [11:51] mborzecki at the time this was a single decision [11:51] mborzecki we should probably just build s-c with apparmor now [11:51] zyga: yup, there's 9740 for that [12:41] re [13:57] PR snapd#9409 closed: cmd/snap: implement 'snap validate' command [14:14] * zyga looks at export manager now [14:18] mborzecki https://github.com/snapcore/snapd/pull/9740#pullrequestreview-543996730 [14:18] PR #9740: packaging/opensuse: enable AppArmor on Leap [14:19] My snap has a need to access `/usr/lib/libGLESv2.so.2` from the host, I have the opengl interface connected. However it seems that my snap's `/usr/lib` view is actually of the core20 snap and *not* the host's rootfs, which actually has the nvidia's libGL* libraries. [14:21] zyga: well, that requires a differnt fix then, because right now snapd acts silly when there's no s-c apparmor profile, and is completely oblivious to s-c not supporting aa at all [14:21] l [14:21] om26er: can you run the app with SNAP_CONFINE_DEBUG=1 SNAPD_DEBUG=1 and post the logs somewhere? btw. is it a tegra device? [14:21] om26er try /var/lib/snapd/lib/gl [14:22] tegra is not supported AFAIK [14:22] as in, no gl support for tegra [14:22] via snapd [14:22] mborzecki, yes its a Nvidia Jetson Xavier NX, running a yocto based image. [14:23] mborzecki why does snapd get confused when s-c has no apparmor enabled? [14:23] om26er no snapd support for nvidia on that platform [14:23] om26er if you want to know more I can explain but it's a long road to fix t hat [14:23] zyga: use has home on nfs: https://forum.snapcraft.io/t/snapd-doesnt-start-on-opensuse-leap-15-1/13710/10 and snapd tries to update s-c profile, which does not exist, so it just fails and exits [14:24] mborzecki that's different [14:24] we should have the profile [14:24] and not use it [14:24] zyga I was able to "fix" that by copying all the required libGL* files to `/var/lib/snapd/lib/gl`, maybe a patch to snapd would fix? [14:24] IMO that's safe [14:24] zyga: why have it when s-c has no aa? [14:24] that of course is a hack [14:24] om26er a dirty hack might work but that's not the right way [14:24] mborzecki for consistency and simplicity [14:24] mborzecki it's one to not use aa [14:24] mborzecki: it's something else to special case lots of places [14:25] om26er there are the following concerns: [14:25] om26er: hosts provides some files for "GL" support, those differ across hardware and across OSes for the same hardware [14:25] zyga: shipping apparmor profile without aa is a special case on its own [14:25] mborzecki I disagree, many packages ship unused profiles [14:25] and offer admins to enable them [14:25] I see it as lower cost than special fixes in code [14:26] om26er: apps need to load those libraries and find appropriate support files [14:26] om26er that's the general problem [14:26] now for nvidia specifically [14:26] we have two pieces of code, compile time choice, of how apparmor is "provided" [14:26] one is assuming everything is in a directory that can be bind mounted [14:26] the other assumes that the files are in /usr/lib or similar, and a symlink farm is constructed [14:27] those differ in sandbox description required to use them [14:27] before I left Canonical I wanted to build a different way that's no longer ugly [14:27] and also allow snaps to provide those libraries on some systems [14:27] either from snap to host or from snap to other snaps [14:28] but also allow the host to describe the files and allow classic host to provide the libraries to snaps [14:28] there's some work towards that [14:28] notably the export manager is nearly everything required to make that possible, there's still more work required after the manger is merged [14:28] but it gives you a way to tell that the host can just export the libraries and the snaps consume them [14:28] then snap-confine code can go away [14:29] and systems with other packaging, like debian even, can be made to work by adjusting packaging in the os (in debian) without patching snapd [14:29] the idea is that the export manager creates a tree of libraries [14:29] and the provided no longer matters [14:29] it could be provided from snaps later on [14:30] but can, most importantly, provided by the host in a way that is not hard-coded in snapd [14:30] om26er let me know if you are interested in pursuing that [14:30] In our case all the required libraries do live in /usr/lib, these are the one's that I had to copy https://gist.github.com/om26er/b4aaf190f5ce3629067bd18b5ff502f2 [14:30] om26er you probably want to build snap-confine with different config option then [14:30] it's an acceptable stop-gap even though the code was never tested on tegra [14:31] or !x86 "fork" of nvidia [14:31] some arm builds are more like x86 [14:31] tegra is not IIRC [14:32] zyga since I "build" the image, the do have the option to copy the required so files to a location from where snapd could read them. All in all we are looking for a solution that wouldn't break easily [14:32] well, as long as you upgrade everything in lockstep that's possble [14:32] I would still call it a hack though [14:32] So how would a change to snap-confine look like ? [14:32] om26er there's a binary choice of nvidia support mode [14:32] use the other one [14:33] it looks for libs in /usr/lib [14:33] the first one bind mounts the whole /usr/lib/nvidiasomething directory [14:33] how do you configure snap-confine today? [14:34] zyga nothing special, here is the build code that we stole from morphis's project https://github.com/crossbario/meta-snappy/blob/master/recipes-support/snapd/snapd_2.47.1.bb [14:34] hmm [14:34] I forgot which value is default [14:34] you're not explicitly using any [14:35] https://github.com/crossbario/meta-snappy/blob/master/recipes-support/snapd/snapd_2.47.1.bb#L68 you need more services [14:39] pedronis_: would you like to discuss validation set error i pasted earlier? [14:39] I think I should probably look at snapd' deb packaging ? [14:39] pedronis_: or shall I just push to the PR for re-review? [14:40] also apart from /usr/lib, the libglvnd vendor directory is also needs to be "read", namely `/etc/glvnd/egl_vendor.d` === pedronis_ is now known as pedronis [14:41] pstolowski: if you have time now, yes I would like to discuss a couple of things about it [14:41] and `/etc/egl/egl_external_platform.d/` [14:41] pstolowski: they relate also to local vs store snaps [14:42] zyga re more services: Which one's do you have in mind ? [14:43] om26er: reminds me i have this outstanding pr to meta-snappy to review :/ [14:43] pedronis: ok, coming to standup HO [14:44] mborzecki I was planning to pursue that. Especially now that we have apparmor kernel patches for full confinement for multiple platforms. [14:45] qemu, linux-raspberrypi-5.4 and linux-tegra-4.9 [14:46] ^ of course, those patches are 99% Canonical's, we only fixed minor conflicts etc ;-) [14:46] om26er: let me take a look at that tomorrow morning and we can thing about taking that further [14:47] PR snapd#9741 opened: boot: add sealKeyToModeenvUsingFdeSetupHook() [14:47] sure, sounds good :+1: [14:48] zyga: kida meh https://paste.ubuntu.com/p/cnRQ7xMgpJ/ i would like to avoid having another configure switch [14:49] zyga: so in theory distro could decide to not ship the file at all, but then snapd behaves silly again if it finds sufficient kernel & userspace support [14:50] i think those 2 should be separate, if there's no aa profile for s-c snapd should not try to fix it even if aa is supported by userspace and the kernel [14:53] heh, type=AVC msg=audit(1607007092.399:196): apparmor="DENIED" operation="exec" profile="/usr/lib/snapd/snap-confine" name="/usr/lib/snapd/snap-update-ns" pid=24802 comm="snap-confine" requested_mask="x" denied_mask="x" fsuid=0 ouid=0 [14:55] zyga: there's no x in s-c profile for s-u-n :/ so one canno really ship the profile for s-c without enabling other bits [14:59] * cachio lunch [15:00] I wonder what would happen if I add /var/lib/snapd/hostfs/usr/lib/ to LD_LIBRARY_PATH ? The opengl interface already allow "read" access to quite a few so files there. [15:01] om26er tip, use SNAP_LIBRARY_PATH [15:02] pedronis: thinking about it, for that cloud-init thing, we probably want to _add_ that config key to existing zzzz_snapd.cfg's right? or do you think it's sufficient to just add it to new ones given that this new feature from multipass is the only entity that seems to require it enough to file a bug for us? [15:03] ijohnson: let's start with a fix for new install and then see [15:03] ack [15:03] I think I should be able to also write a spread test for this case too given what they have in multipass is for a VM [15:13] zyga that kind of "worked". There are still a few so files that the opengl interface does not allow: [15:13] libnvdc.so libnvimp.so libnvos.so libnvrm.so libnvrm_gpu.so libnvrm_graphics.so [15:13] Would a PR for opengl interface be acceptable ? [15:13] om26er that's another aspect why this is hacky and annoying [15:13] om26er perhaps, I dont' see why not [15:13] it's just not scalable in general [15:16] Yeah, I understand it's not scalable, so would definitely love to have a real fix and even help where I can. Wouldn't be fun to have our UI break because Nvidia decided there is now a new .so file required as well [15:17] btw: That confined snap, would run on Weston's Kiosk Shell as full screen. (It kind of already works, with a few minor hacks) [15:20] erm ... nvidia libs are mapped to /var/lib/snapd/lib/gl usually (and fully accessible by the opengl interface this way) ... you dont need to access hostfs at all [15:20] mvo FYI: all workers are shut down,I'll kill the instancenow [15:20] ogra not exactly right [15:20] it depends [15:20] on what ? [15:20] on build config [15:21] there are two separate nvidia support versions [15:21] one does depend on hostfs [15:21] the desktop launchers definitely map these libdirs for you automatically [15:21] and that usually works very well for desktop and kiosk apps [15:21] no, that's not true either [15:21] I mean [15:21] snapd provides the libraries [15:21] helpers just use them [15:21] (explicitly) [15:21] right and i.e. the gnome extension auto-maps them into the library path [15:22] cachio: zyga shut down the other instances now [15:22] with its desktop-launch script [15:22] ogra that's correct [15:22] the only thing i found missing yet was VDPAU in some exotic setups [15:23] (interestingly intel ... not nvidia actually ... ) [15:23] mvo, nice, I'll start adding more in some minutes [15:24] hw support bits are a bit missing in snapd [15:24] I hope the export manager unlocks that [15:24] as custom hacks cannot scale [15:25] mborzecki interesting [15:25] yeah, sadly apparmor is used in two ways [15:25] implicitly [15:25] and explicitly [15:26] the explicit part we can control [15:26] the implicit one we cannot [15:26] but [15:26] can you compile snap-confine without apparmor support [15:26] so that when snap-update-ns runs, it is not confined either? [15:27] mborzecki looking at your diff [15:27] do not load profiles [15:27] if disabled, just keep them off [15:27] otherwise snap-confine _is_ confined anyway [15:27] and we don't want it to [15:27] out of our set, snap-confine is the only program that uses implicit path-based profile association [15:28] mvo: ijohnson: mborzecki: proposed https://github.com/snapcore/snapd/pull/9742 [15:28] PR #9742: gadget,o/devicestate: hybrid 18->20 ready volume setups should be valid [15:29] pedronis: ack thanks I'll have a look today [15:29] it's just the fix plus some gadget update tests [15:29] to be clear [15:29] but it's a baseline to then try to untangle things [15:32] zyga: apparmor.service will load the profile when restarted anyway [15:32] mmmm [15:32] indeedd [15:32] PR snapd#9742 opened: gadget,o/devicestate: hybrid 18->20 ready volume setups should be valid [15:32] PR snapd#9743 opened: daemon: split unsupported buy implementation to its own api_*.go files [15:32] hmm [16:16] ijohnson: btw let me know if you have questions about my lk comments [16:16] pedronis: yes I was going through them, I will ask questions in a couple minutes [16:18] pedronis: instead of Go build tags what do you think of trying to type cast *os.PathError to Unwrap() and if it returns nil, then we do the compat thing, and if it does return non-nil just return it as-is ? [16:19] ijohnson: that also works [16:19] essentially what Go source is doing here: https://golang.org/src/errors/wrap.go?s=372:400#L4 [16:19] ok, I think that is more managable [16:19] it just feels a bit heavy to have an entire set of multiple files per go versions that just have one method [16:20] mvo: cachio: I'm seeing the cla check fail sometimes like this: https://github.com/snapcore/snapd/pull/9736/checks?check_run_id=1493790248 are we missing deps on some agents? [16:20] PR #9736: o/devicestate: save model with serial in the device save db

[16:20] pedronis, let me check [16:21] pedronis, this ran in canonistack [16:21] the old env [16:22] I'll monitor the other action jobs to see where it is happening [16:22] thanks for the ifno [16:34] mvo: #9730 can be squash merged [16:34] PR #9730: hookstate: refactor around EphemeralRunHook

[16:35] #9705 needs 2nd reviews [16:35] PR #9705: devicestate: add runFDESetupHook() helper [16:35] pedronis: the error struct we discussed still needs to be slightly sophisticated, reason is that AFAIU a snap may be required by multiple validation sets (if they are not conflicting); so, something like map[string]map[string]bool is needed, or map[string][]string ? [16:36] (for each of: missing, required, wrongrevision) [16:36] []string seems ok [16:40] pstolowski: ^ [16:40] pedronis: yes, ok [17:12] pedronis: ok, so I think I have most of your comments handled, but you said something about "maybe we need two level of helpers", that bit is unclear to me, I eliminated backupEnvFile and removed the `if` from partLabelForRoleAndTime so it's now just partLabelForRole and the callers of that function do the appending of ".bin" themselves [17:16] ijohnson: the two level helper is because of my wondering about envFileForPartName, it seems to be used for the actually boot partitions as well [17:16] so the name is off [17:16] ah I see so that relates to your comment about using envFileForPartName being used in ExtractKernelAssets [17:17] yes, but I had probably a different org in mind, you said you removed backupEnvFile which I didn't quite expect [17:17] but that's ok [17:17] so I don't know if we need two helper or not [17:17] but that name is off [17:17] given usage [17:17] pedronis: what about devFileForPartName ? [17:18] ijohnson: that sounds ok in principle, I just wonder a bit how envFile looks like atm [17:18] https://www.irccloud.com/pastebin/bS2PX05E/ [17:19] mvo, today is comming 2.48.1 right? [17:19] envFileForPartName would become maybe devFileForPartName [17:23] pedronis: thoughts ? [17:24] cachio: yes, to beta in ~1h for snapd [17:24] cachio: I'm not sure we will do a core release but probably for symetry [17:24] cachio: but less urgent [17:24] mvo, nice [17:24] sure [17:26] * cachio afk 20mins [17:26] ijohnson: that looks ok, the bool doesn't read great though [17:26] yeah I have constants for that though [17:26] const ( [17:26] backupEnvFile = true [17:26] primaryEnvFile = false [17:26] ) [17:26] but agreed it's clunky [17:27] devPathForPartName ? [17:28] I think we might also want to acknowledge the reality that envFile is not always/primarely a file [17:28] now that is not exposed anymore because we have Present [17:28] might that should be called envBackstore() [17:30] s/might/maybe/ [17:30] ijohnson: ^ [17:30] mmm that sounds better [17:30] let me add that [17:43] PR snapd#9744 opened: OpenGL interface: Support more Tegra libs [17:46] pedronis: oh wait actually I have more more question, you mentioned `validate()` in one of your last comments about checking if PrepareImageTime is true that Role is Sole or Recovery, but we don't have a validate() method? [17:46] did you mean to return an internal error from Present() [17:46] ? [17:48] fyi @zyga https://github.com/snapcore/snapd/pull/9744 [17:48] PR #9744: OpenGL interface: Support more Tegra libs [17:49] looking [17:49] om26er replied [17:51] ijohnson: we do on opts [17:51] oh right I see at that level [17:52] I think it would be always correct, is just that only lk cares [17:52] unless I'm confused [17:53] PR snapd#9745 opened: [RFC] seed: enable uc20 devmode snaps in dangerous models [17:57] om26er look again please :) [17:58] sure ;-) [18:08] PR snapd#9746 opened: bootloader: add check for prepare-image time and more tests validating options [18:10] cachio: CLA check failed again https://github.com/snapcore/snapd/pull/9695/checks?check_run_id=1494398856 [18:10] PR #9695: bootloader/lk: add support for UC20 lk bootloader with V2 lkenv structs === ijohnson is now known as ijohnson|lunch [18:13] PR snapd#9730 closed: hookstate: refactor around EphemeralRunHook

[19:01] ijohnson|lunch: I'm seeing core20/degraded failing on one of my PRs but it passed before on it and nothing related to it changed, is it flaky? [19:32] ijohnson|lunch, updated all the canonistack and prodstack agents and installed the cla dependency [19:33] it is really weird it didn't fail before === ijohnson|lunch is now known as ijohnson [20:08] pedronis: hmm I haven't seen it fail, I don't know that it is any more flaky than the nested tests are in general [20:08] if you have logs that would be good to look at [20:09] cachio: thanks, I'll let you know if I see it fail anywhere else again [20:13] ijohnson, nice ,thanks [21:59] PR snapd#9747 opened: interfaces/builtin/log_observe.go: allow controlling apparmor audit levels [22:03] ijohnson: it failed here: https://github.com/snapcore/snapd/pull/9736/checks?check_run_id=1494221544 it's the saving serial one [22:03] PR #9736: o/devicestate: save model with serial in the device save db

[22:03] pedronis: thanks I'll have a look [22:08] ijohnson: thx