[06:06] morning [06:52] zyga: hey [06:52] you around? [07:05] mborzecki sure, can I help you somehow? [07:05] zyga: nvidia biarch is borked [07:05] let me show you where [07:07] zyga: https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support-nvidia.c#L276 the last thing that sc_mkdir_and_mount_and_glob_files() does is remount the target directory ro, so sc_glob_files_only() then fails with filesystem is read only https://paste.ubuntu.com/26523173/ === zyga_ is now known as zyga [07:12] zyga: https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support-nvidia.c#L276 the last thing that sc_mkdir_and_mount_and_glob_files() does is remount the target directory ro, so sc_glob_files_only() then fails with filesystem is read only https://paste.ubuntu.com/26523173/ [07:14] aha [07:14] this makes sense, feel free to move that outside so that things work correctly [07:15] ok [07:15] thank you for spotting that [07:15] it feels like a RC3 though [07:17] yeah, built a new rig, put an old nvidia card inside since prices are absurd, tried to debug the spotify issue some guy raised in a forum and ended up debugging nvidia issues instead :) [07:23] :-) [07:23] it's good that you are using nvidia though, most of the laptop developers don't notice that [07:29] hey mvo [07:29] hwy zyga [07:29] hey [07:34] mvo: morning [07:34] good morning mborzecki [07:49] PR snapd#4603 opened: cmd/snap-confine: fix read-only filesystem when mounting nvidia files in biarch [07:49] zyga: ^^ [07:49] yep [07:49] looking [07:49] sadly there are no tests for this code, so i just tested it locally [07:50] the good news is that i can run snaps again :) [07:51] ohmygiraffe works [07:54] good morning, snappers [07:56] kalikiana: morning [07:59] mborzecki: oh, nice. something for 2.31? [08:02] mborzecki +1 [08:03] we need to ensure that some round of testing on proprietary nvidia driver is done for 2.31 [08:04] PR snapd#4601 closed: tests: update kill-timeout focused on making tests pass on boards [08:06] zyga: is this 2.31 cherry-pick material? [08:06] yes [08:06] definitely [08:07] i have an old 9600gt card, so can check with 340xx driver, no vulkan though [08:07] mvo note that it affects _biarch_ systems [08:07] zyga: so fedora/solus? [08:07] mvo: yes, otherwise it's not possible to assemble a mount on on my arch box [08:07] mvo yes [08:07] ta [08:07] * mvo looks [08:11] mborzecki: 4598 is ready right? it has a +1 from neal [08:11] mvo: yes [08:13] PR snapd#4598 closed: packaging: create /var/lib/snapd/lib/{gl,gl32,vulkan} as part of packaging [08:17] mborzecki: fwiw, once the tests are green I will merge 4603, my commment about comments can always added later [08:19] mvo: ok [08:20] mborzecki: I also noticed this is already used in the code without much comment so :) [08:20] * mvo has a wtf moment when he noticed that unsquashfs will return "0" if the unpack fails [08:21] .... [08:21] well [08:21] yeah, that is weird [08:21] zyga: my thoughts exactly :) [08:22] zyga: I will send a patch to upstream and to our ppa [08:22] PR snapd#4602 closed: tests: use root path to /home/test/tmp to avoid lack of space issue [08:22] zyga: but holly cow [08:22] mvo: is there anything special about the file? [08:23] maybe it's _just_ a bug [08:23] mborzecki: which file? the squashfs file that is unpacked? I unpacked it into a tmpfs [08:23] mborzecki: and that had not enough space [08:23] mvo: right, the squashfs file [08:23] oh [08:23] mborzecki: sergio figured out this is what broke the prepare-image test on arm64 [08:24] mborzecki: it uses a tmpfs on /tmp which is not big enough to hold the tmp files we need to prepare the image (the armhf kernel must be unpacked there) [08:24] zyga: it probably is [08:24] mvo: btw. does it try to clean up the fiels it extracted? [08:25] s/fiels/files/ [08:25] mborzecki: I doubt it, let me check [08:25] mborzecki: no [08:27] right, don't know what i was expecting [08:28] it'd be funny to put squashfs through afl and see what comes out [08:29] afl? [08:29] american fuzzy loop [08:30] http://lcamtuf.coredump.cx/afl/ [08:30] oh, cool [08:32] anyone wants to do a quick HO test with me [08:38] mornings! [08:38] hey hey [08:50] tak, tak [08:50] wrong window :) [08:50] pstolowski: morning [08:51] mborzecki tak tak, wrong window [08:51] mvo: restarted the build in 4603, looked like an unrelated failure [08:51] ok === d__ed is now known as d_ed [09:17] mvo: o/ [09:17] mvo: looking at 4600, do config changes always end in a *json.RawMessage? [09:18] Chipaca: I'm not 100% certain, that sounds like something pstolowski may know [09:18] pstolowski: ^? [09:19] pstolowski: in config's transaction.go, do config changes always finish in a *json.RawMessage? [09:19] Chipaca: fwiw, the PR is a bit of a RFC for gustavo if this is validation we want or not [09:19] mvo: ack [09:19] ogra_: did things work once the typo in the core.service.rsyslog.disable=true was fixed btw? [09:19] Chipaca: I mean, don't kill yourself over the review :) it might be in vain :( [09:20] Chipaca: even though I think it would be good to have this validation [09:21] Chipaca, mvo yes [09:21] pstolowski: ack [09:21] pstolowski: why are they *json.RawMessage, btw? instead of just json.RawMessage i mean [09:22] ogra_: you might be right, but that whole ubuntu sso thing seems fishy to me. i dont want to be dependant on any service that is not hosted on my own devices :/ [09:22] Chipaca, dunno. I haven't implemented that [09:22] zyga: I pushed a PR upstream for unsquashfs, I wonder what we should do in the meantime, we could do crzay stuff like compare the targetdir with the unsquahfs -ls output but that sounds slightly crazy. but maybe patience is a virtue and I wait for upstream [09:22] Chipaca, kyrofa might remember ;) [09:23] pstolowski: blame says gustavo [09:23] :-) [09:23] i'll ask [09:23] Chipaca, okay. blame has better memory than me, that's for sure [09:23] Chipaca: ta [09:25] mvo: what is the worst thing that can. happen if we use broken unsquashfs? [09:26] zyga: we unpack a kernel on arm, no space left on /boot but we don't detect the failure, continue and the boot fails (and reverts) [09:26] zyga: so hopefully its not too bad [09:27] zyga: but still fugly that we don't detect it when it happens [09:27] mvo: zyga: we found a bug in unsquashfs? [09:27] mvo: shall i merge 4603? [09:27] Chipaca: bug/feature/missing-thing - unsquashfs does not report anything on unsqushfs failures [09:27] Chipaca: like exit(0) [09:28] Chipaca: is what it will always do [09:28] ah [09:28] Chipaca: which is slightly annoying, we had this bug on arm64 that prepare-image failed and there was a kernel missing [09:28] Chipaca: and it turns out, tmpfs was too small and unsquashfs told the world: "failed..." on stdoutbut never reported it via exitcode [09:29] mborzecki: let me look [09:29] mborzecki: yes please, I will cherry-pick [09:29] afterwards [09:29] mvo: could we bundle fixed unsquashfs in the package? [09:29] zyga: yeah, I think that would work [09:29] PR snapd#4603 closed: cmd/snap-confine: fix read-only filesystem when mounting nvidia files in biarch [09:30] zyga: I was just thinking we could do a strings.Contains("failed", strings.ToLower(string(unsquashfsOutput)))" [09:30] I mean, the risk is rather small because we check for hashes and the store does validation as well [09:30] zyga:as a first approximation [09:30] mvo, a simple "failed" thing works too, yeah [09:30] zyga: I think I will do that and hope for upstream, once upstram merges it I can SRU it at least into the ubuntu-world [09:31] zyga: anyway, thanks for the brainstorm :) [09:31] :-) [09:31] * mvo is also slightly shocked that there is no open upstream bugreport about this [09:32] PR snapd#4604 opened: cmd/snap-confine: create lib/{gl,gl32,vulkan} under /var/lib/snapd and chown as root:root [09:33] zyga: ^^ this a workaround for the problem with missing /var//lib/snapd/lib that we had on fedora [09:33] reading [09:34] mborzecki one comment [09:36] zyga: uhh right :/ silly me [09:43] pstolowski: want me to add the roundrobin writer to strutil? [09:45] zyga: mvo: is 4604 a 2.31 material too? === ufee1dead is now known as ikey [09:45] yes [09:45] Chipaca, no, thanks, I'm on it [09:45] pstolowski: ok [09:46] * Chipaca carries on down the PRs [09:46] mborzecki: that is only needed for re-exec? [09:46] mborzecki: and bi-arch? or more generic? [09:48] mvo: bi- and multi- arch, same thing as in #4598 but works around packaging errors [09:48] PR #4598: packaging: create /var/lib/snapd/lib/{gl,gl32,vulkan} as part of packaging [09:53] mvo, i only tesetd on a pi and it was fine there (on the actual board i'm currently trying to get a grub-uboot implementation going, so have no booting OS atm) ... it was just the typo (and i checked, it wasnt there in a former local iteration of the gadget which explains why it worked before) === chihchun_afk is now known as chihchun [09:54] ogra_: thanks [09:59] ogra_: fwiw, we are exploring if we can add error reporting for this easily (the general case is harder but we might get away with somehting core specific to catch unknown options) [09:59] mvo, that would be awesome [09:59] ogra_: pr #4600 iirc [09:59] PR #4600: configstate: validate known core.* options [10:00] nice round number [10:00] * mvo back to squashfs [10:00] ogra_: heh, indeed [10:00] heh. you too ? [10:00] * ogra_ is banging his head against grubs squashfs implementation on arm [10:01] ogra_: I am just fixing silly issue around unsquashfs [10:01] sadly the u-boot api and grub dont want to play together well .. [10:01] ogra_: autsch! [10:01] would be so nice to have the arm boards read from the snaps diurectly as well .. [10:02] (beyond being able to use secureboot from grub) [10:03] ogra_: oh yes! there were patches for pre-v4 squashfs for u-boot, I guess this never went anywhere? or is the plan to use uboot->grub anyway? [10:04] mvo, well, i have to implement a secureboot solution, one option thats known to work is using a uboot.FIT image, but that means essentially all contents of system-boot need to go into a blobby signed image (doesnt work well with split-initrd or de-coupling of any snaps if tehy ship separate parts for system-boot) ... [10:05] the other option is grub ... and use signed single files directly from the snaps [10:05] ogra_: so uboot chainloads to grub? [10:06] ogra_: that sounds interessting for the main borads as well if this works well [10:06] well ... kind of ... uboot keeps running and provides an api to grub ... so it does all the HW heavylifitng ... grub then just uses syscalls to talk to uboot [10:07] the prob is the api ... it is a) very young and b) the board i'm currently using has a 2015 u-boot tree as base ... grub works very well until i try to load anything from any disk [10:08] so sadly i might have to turn down the idea :/ [10:08] (i can actually use the loopback feature and list contents of snaps etc ... just the loading explodes completely) [10:09] * mvo nods [10:10] * Chipaca ~> afk (physio) === chihchun is now known as chihchun_afk [10:25] PR snapd#4605 opened: snap: detect unsquashfs write failures [10:31] mvo one comment there [10:42] * Chipaca tries again to go to physio [10:44] zyga: ta [10:44] mvo nothing strong, just could save (perhaps) some memory [10:45] zyga: easier to read this way too, I like it [10:45] zyga: also Chipaca comment is a good one, the amount of output can be huge (because it does not stop and will report and error for every file). so a special writer seems like the better option === lool- is now known as lool [10:50] Cloning into '/home/gopath/.cache/govendor/golang.org/x/crypto'... [10:50] error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function. [10:50] I saw this a few times today [10:56] brb === chihchun_afk is now known as chihchun [11:18] mvo, can we merge 4576? [11:36] PR snapd#4596 closed: osutil: allow using many globs in EnsureDirState [11:46] PR snapd#4606 opened: snap: use custom unsquashfsStderrWriter for unsquashfs error detection [11:46] Chipaca: I pushed a potential fix using the custom writer -^ [11:46] any idea why snap app services are enabled (but not started) in doLinkSnap() while app socket services are enabled and started in statSnapServices() ? [11:47] sergiusens, hey, I was talking to Ken about building snaps on 18.04 and he said there's a bug in snapcraft that makes those not run on other ubuntu releases [11:47] is there a bug report I can subscribe to? [11:49] sergiusens, he also said you stated you would have it fixed by past-thursday :) === chihchun is now known as chihchun_afk [11:58] mborzecki: socket services are recent, where do we start then app services? [11:59] PR snapd#4589 closed: many: remove "content" argument from snaptest.MockSnap() [12:00] Chipaca, I think we should have fewer implementations of mkdirallchown [12:00] +1 on the patch there but I think we could use the secure variant instead [12:00] chipaca we could make osutil/secure/ package and stick them there [12:01] pedronis: in 'start-snap-services', but they get enabled in link-snap [12:01] pedronis: the sockets are enabled and started in start-snap-services [12:02] PR snapd#4586 closed: cmd/snap: add completion conversion helper to increase DRY [12:04] Chipaca question on 4581 [12:05] zyga: what do you mean with empty there? [12:05] empty byte array [12:06] I don't think I understand the question, maybe John will [12:06] pedronis maybe I misunderstood how that works, I'll talk to john [12:07] it's a shortcut, in the case there's no config at all (for any snap) and we are setting to the nil config for this one [12:07] there's nothing to do [12:07] basically [12:08] I think I understand that now, snapcfg is the value we want to set, not the data store [12:14] mvo is 2.31 something that is now living in a branch? [12:14] I'd like to merge 1st 2.32 PRs that are not ready for 2.31 rc3 [12:21] zyga: 2.31 is in a branch [12:21] zyga: only cherry-picks [12:22] excellent [12:22] I'm chopping various things for sun profiles [12:22] and trying to figure out how to avoid some duplication and annoying maintenance [12:22] and since it's not a 2.31 thing anymore I'm doing some improvements to how apparmor backend works [12:24] zyga: yeah, you can go wild in master [12:25] cool [12:26] jdstrand, 4590 needs your +1 then, I'll land the follow-ups on top so that we can see I didn't break what we got working last week [12:26] and the branches can be nicer when we're not after minimal diffs anymore [13:02] PR snapd#4607 opened: wrappers: cleanup enabled service sockets [13:29] * Son_Goku sighs [13:30] I wish people would stop counting on LSM stacking to save their butts [13:30] hey Son_Goku [13:31] zyga: hey === chihchun_afk is now known as chihchun === chihchun is now known as chihchun_afk [13:51] * kalikiana going to have lunch, back in a bit [14:01] Chipaca: not super urgent but a review of #4497 would be appreciated [14:01] PR #4497: many: make rebooting of core on refresh immediate, refactor logic around it [14:01] niemeyer: for later, the PR I mentioned is: #4582 [14:01] PR #4582: many: at seeding try to capture cloud information into core config under "cloud" [14:09] pedronis: Thanks! [14:23] zyga: Who can we work with to get snapd updated in openSUSE? [14:24] Sadly most desktop application snaps are wearing their sad face when run on openSUSE right now. [14:27] "lets turn that frown upside down" [14:27] flexiondotorg me or anyone who will do the packaging [14:27] * ikey remembers his motivational training [14:28] zyga: Any chance you could find the time to rev snapd in openSUSE to 2.30? [14:29] It's currently 2.27 so doesn't know about the desktop and desktop-legacy interfaces. [14:29] ikey: o/ [14:29] \o [14:30] flexiondotorg let me try tomorrow and if not let's find somoene [14:33] flexiondotorg I have opensuse ready and booted, just some higher priority things [14:34] flexiondotorg we don't have anyone on it that's dedicated [14:34] flexiondotorg I can review PRs to that repo [14:35] Thanks zyga [14:45] mvo: indeed partial writes make this a lot harder [14:45] mvo: sorry :-) i'll still get something up for you to look at in case you like it [14:46] Chipaca: yeah, its a bit annoying [14:46] Chipaca: sure, happy to have a look how you solved it [14:47] Chipaca: that reminds me, my test case needs an upper case "Failed to..." [14:51] PR snapd#4582 closed: many: at seeding try to capture cloud information into core config under "cloud" [14:54] re [15:01] mvo: https://pastebin.ubuntu.com/26524851/ -- i'm not at all sure this is actually simpler (but i know how to make it more complex :) ) [15:01] * Chipaca -> school run [15:09] Chipaca: in a meeting right now, let me read it but thanks a bunch for looking into this in any case [15:10] Chipaca: it looks a bit shorter at least [15:15] jdstrand hello, how was your flight home/ [15:17] zyga: uneventful :) [15:17] zyga: I saw the request for 4590 re-review [15:17] just as it should be [15:17] zyga: exactly :) [15:17] I didn't do the full thing there, just added your FIXME comments [15:18] and confirmed with mvo that release is branched off and can merge things [15:18] I'm chopping my sun profile work into smaller pieces now [15:20] cool [15:20] let me look at 4590 real quick then [15:25] cool, thanks! [15:27] oh ... cooooool :) [15:27] * zyga just realised something very useful :) [15:36] Chipaca: did you do anything about saving screenshots on install? or still pending [15:36] pedronis: that's phase 2 [15:36] pedronis: and requires answers to Questions [15:36] Chipaca: phase 2 of what? [15:36] pedronis: of snapshots [15:37] pedronis: phase 1 is just manual [15:37] Chipaca: I said screenshots, not snapshots [15:37] something discussed in NYC [15:37] pedronis: questions such as "if you snapshot rev2 and upgrade to rev3 and restore what do you even restore" [15:37] pedronis: /o [15:37] pedronis: I did nothing about any metadata [15:37] ok, thx [15:37] icons, screenshots, title, description, etc etc etc [15:38] all the same bag imo [15:38] Chipaca: because we had some discusions about saving more, saving differently, right? [15:38] but hasn't happend, ok [15:39] pedronis: we decided those for all attributes that were per snap not per rev the store is authoritative, and we should keep a cache locally [15:39] correct, no progress [15:40] pstolowski, Chipaca, yeah I got nothing on the *json.RawMessage [15:40] kyrofa: thank you for reminding me :-D [15:40] :) [15:40] niemeyer: in configstate/config we use *json.RawMessage everywhere instead of json.RawMessage. Why? [15:40] niemeyer: (asking you because git blames you) [15:44] Chipaca: he is in a meeting [15:44] mvo: i am in no rush :-) [15:56] * cachio__ lunch [16:06] PR snapd#4608 opened: cmd/snap-confine: allow snap-update-ns to chown things [16:06] jdstrand can you please look at ^^ [16:06] it's a two liner [16:09] zyga: done [16:09] mvo ^ [16:10] I'll work on updates to the tests to check relevant new features as a user, just ran into this by dogfooding [16:12] Chipaca: Heya [16:12] Chipaca: There is/was a serious bug about it in the json package [16:12] Chipaca: It misbehaves wildly in some situations [16:12] ah [16:13] niemeyer: i see [16:13] Chipaca: Let me see if I can find some quick info [16:13] niemeyer: i use json.RawMessage directly in snapshots; let me know if i'll get bitten [16:13] niemeyer: or taht :-D [16:14] Chipaca: https://github.com/golang/go/issues/14493, probably [16:18] ah, that [16:36] + systemctl stop snapd.service snapd.socket [16:36] Job for snapd.service canceled. [16:50] niemeyer: it's fun reading through that issue how people flip from "meh" to "WTF LETS FIX THIS" as they forget and then get bitten and waste time trying to figure it out [16:52] niemeyer: OTOH, it's fixed, so ¯\_(ツ)_/¯ [16:53] so "it will be out in debian 10" [16:53] in X years [16:54] zyga: it's not go's fault ubuntu always ships a go that is no longer supported by them [16:55] much as it irks me, it really isn't :-) [16:55] and this particular bug has been fixed since 1.8, which is a year old already [17:06] zyga: what do you mean with "fewer implementations of mkdirallchown"? [17:07] Chipaca we have one in snap-update-ns that's tailored for security [17:07] zyga: ahh... i forget [17:07] zyga: I can take on unifying those two [17:07] i see no reason not to use the securer one everywhere :-) [17:07] yeah [17:08] how does osutil/secure sound? [17:08] those would require jamie review to change [17:08] but we could just move the current code there first [17:08] have a look, maybe something about it is silly and unreasonable, maybe not [17:08] zyga: sounds like a good, reasonable plan [17:09] zyga: but, going back a bit, you said "+1 to my changes there", but didn't actually +1 :-D [17:09] I will add secure symlink next, definitely this week [17:09] oh, sorry [17:09] looking [17:09] :-) [17:09] zyga: also your comment on #4581 feels reviewish but doesn't have a +1 either [17:09] PR #4581: overlord/configstate/config: make SetSnapConfig delete on empty [17:09] zyga: #4587 is the other one fwiw [17:09] PR #4587: osutil: make MkdirAllChown clean the path passed in [17:10] PR snapd#4581 closed: overlord/configstate/config: make SetSnapConfig delete on empty [17:10] PR snapd#4587 closed: osutil: make MkdirAllChown clean the path passed in [17:11] yuss [17:12] Chipaca: Yeah, it's fixed.. we just need to check if all the versions of Go we care about are fixed too [17:12] Chipaca: I would guess not [17:12] niemeyer: 1.8+ [17:12] Yeah, so risky still [17:13] I'll update snapshots to use *json.RawMessage for now then [17:13] as part of the rebase [17:13] just one more pr to land for that to happen :-) [17:13] Chipaca one question posted on 4585 [17:17] zyga: i confess to not understanding your question :-( [17:18] zyga: care to expand it a little bit? [17:18] sure [17:18] sorry about tyhat [17:18] is there any practical difference between using `inst` there? (instead of the current code that does `&inst`) [17:18]