[01:17] soooo === joed_ is now known as RogueOmega7 [01:20] anyone here wanna bs about ubuntu / unity convergence? [07:17] add "ipv6.address: none" in the edit "lxc network edit lxdbr0" gives you [09:14] bdmurray: see man autopkgtest, could be lots of things === jamespag` is now known as jamespage [10:39] can anyone with more debian-packaging-fu than me understand what's wrong in this autopkgtest log? https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-zesty-ci-train-ppa-service-2362/zesty/amd64/u/unity8/20170113_095859_4ba42@/log.gz [11:25] pete-woods: you need to ask for retrying the tests with all-proposed=1 until Qt 5.7.1 transition is over (which is currently waiting for archive admin action) [11:26] pete-woods: ...retried both === _salem is now known as salem_ === marcusto_ is now known as marcustomlinson [11:55] Mirv: thanks! hopefully that works === hikiko is now known as hikiko|ln [13:53] doko, i have a fix for cinder (well kombu) re: http://qa.ubuntuwire.org/ftbfs/rebuilds/test-rebuild-20161216-updates-python2712-trusty.html [13:56] doko, i'm not sure how best to provide it to you though [13:56] doko, it might fix up glance too [14:00] doko, here's the debdiff: http://paste.ubuntu.com/23792290/ === davmor2_ is now known as davmor2 [14:11] coreycb: does it work with the old python 2.7 as well? [14:12] if yes, then maybe SRU it? [14:13] doko, good question, I'll try it out [14:47] mdeslaur: hi, sudo needs a merge.. 1.8.16 is buggy with freeipa/sssd, fixed in .17 (#1607666). do you have plans to merge 1.8.19 for zesty? [14:48] tjaalton: 1.8.19 has a syslog regression I believe [14:48] we need 1.8.19p1 [14:48] ok [14:49] tjaalton: looks like mvo touched it last, would have to ask him what his plans are [14:49] stgraber: we're seeing some new regressions on ubuntu-image promotion in zesty on armhf. are you aware of or can think of any changes that might have broken devmapper? https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-zesty/zesty/armhf/u/ubuntu-image/20170111_171929_431b1@/log.gz [14:50] tjaalton: take it if you have the time [14:56] we'll see :) [14:56] nacc, do we expect to get upstream tags on usd imported things now ? [14:56] might just fix that bug so that sru is possible === scottt is now known as Guest72306 [15:03] nacc, and, when you're about... is there a way to do a clean import even if the thing has already been imported? [15:04] this seems to work...but wondering if there was a more direct way [15:04] usd import -v --director=cloud-utils --lp-user=smoser --lp-owner=NONE cloud-utils [15:21] smoser: I discussed that with nacc yesterday. He's being doing what you did. I have a branch I've added --no-fetch to, but it's not ready yet. [15:22] *been [15:26] rbasak, for this specific need, i just deleted the branch :) [15:26] on lpusp [15:26] i know i shoudlnt do that generally, but i think it was fine here and i wanted the newly imported thing with tags and such. [15:28] ddstreet: can you help me with reviewing bug 1642903 please? I haven't yet figured out what the specification of udev_event_apply_format is supposed to be :-/ [15:28] bug 1642903 in systemd (Ubuntu Trusty) "introduce disk/by-id (model_serial) symlinks for NVMe drives" [Wishlist,Fix committed] https://launchpad.net/bugs/1642903 [15:29] And trying to follow string manipulation in raw C makes my head hurt :-( [15:29] So reverse engineering its intention is difficult. [15:30] I understand that SUBST_* seems to match udev rule syntax roughly. [15:31] I mean bug 1647485, sorry. [15:31] bug 1647485 in systemd (Ubuntu Zesty) "NVMe symlinks broken by devices with spaces in model or serial strings" [High,Confirmed] https://launchpad.net/bugs/1647485 [15:32] rbasak: [15:32] rbasak: doesn't appear to be any C involved in that at all. Just adding some udev rules (https://launchpad.net/bugs/1642903) [15:32] Launchpad bug 1642903 in systemd (Ubuntu Trusty) "introduce disk/by-id (model_serial) symlinks for NVMe drives" [Wishlist,Fix committed] [15:32] sladen: sorry, wrong bug. I meant bug 1647485 [15:32] bug 1647485 in systemd (Ubuntu Zesty) "NVMe symlinks broken by devices with spaces in model or serial strings" [High,Confirmed] https://launchpad.net/bugs/1647485 [15:40] doko, kombu tested and uploaded to trusty review queue [15:47] ddstreet: never mind. I think I've figured it out. [15:51] rbasak sorry, just saw this, let me know if i can help [15:52] and yeah, that udev tangle is not easy to follow === karstensrage_ is now known as karstensrage [16:04] ddstreet: reading that patch, I [think] it is possible to reach the end of udev_event_apply_format() with an uninitialised sbuf[] [16:04] sladen sbuf should only ever be used if replace_ws is true [16:05] where are you seeing a problem? [16:05] ddstreet: https://git.launchpad.net/~ddstreet/+git/systemd/tree/src/udev/udev-event.c?id=a135967a#n216 [16:05] ddstreet: followed by eg. https://git.launchpad.net/~ddstreet/+git/systemd/tree/src/udev/udev-event.c?id=a135967a#n228 [16:05] ddstreet: meaning that the for(;;) loop is would be exited with nothing written into sbuf [16:06] I'm still on udev-rules.c. Not dived deep into the changes in udev_event_apply_format yet. [16:06] ddstreet: then finally https://git.launchpad.net/~ddstreet/+git/systemd/tree/src/udev/udev-event.c?id=a135967a#n410 [16:07] ddstreet: [16:07] oops [16:10] sladen those break; commands don't break out of the for loop, they break out of the switch statement [16:10] is that what you're talking about? [16:10] ddstreet: correct === hikiko|ln is now known as hikiko [16:12] sladen i'm not quite following where the bug happens? [16:13] line 216, s = &sbuf, then it enters the switch(), then eventually exits the switch(), and restores s after replacing whitespace... [16:13] the switch ends at line 399 [16:14] the if statement at line 402 is outside, after, the switch [16:14] sladen is there something i'm missing? [16:17] ddstreet: minimal example is char sbuf[UTIL_PATH_SIZE]; s = &sbuf; switch(SUBST_RESULT) { case SUBST_RESULT: if (event->program_result == NULL) break; tmplen = util_replace_whitespace(sbuf, s, MIN(tmplen, l)); [16:26] smoser: right, we don't (can't in the current algorithm) go back and add history (e.g., upstream data) for already imported stuff [16:27] smoser: that could be a reasonable feature, but the alternative, presuming no one is yet using your repository, is to delete and re-import [16:27] sladen that util_replace_whitespace is called with length 0 [16:27] as tmplen will be 0 if sbuf isn't used [16:28] nacc, so i did that [16:28] why is there only one upstream/ [16:28] I am planning to file an "upgrade-software-version" bug for xkeyboard-config and maybe freetype. But I am confused. xkeyboard-config has the string ubuntu1 at the end of the version string. So it's not automatically imported until DebianImportFreeze. So if I want to get the version bumped to the version that's in debian sid/unstable I have to file a bug with the tag "upgrade-software-version", probably at least two or three we [16:29] smoser: which src package? [16:29] https://git.launchpad.net/~usd-import-team/ubuntu/+source/cloud-utils/refs/tags?h=debian/jessie [16:29] smoser: wait you deleted a branch or a repository? [16:29] i deleted the lusd-import and then re-imported [16:30] at least i thought i9 did. [16:30] smoser: hrm, the newest objects in the repo are just 3 tags [16:31] yeah, that is confusing [16:31] i think i try again [16:31] smoser: seems reasonable [16:31] so i'm going to 'delete repository' at https://code.launchpad.net/~usd-import-team/ubuntu/+source/cloud-utils/+git/cloud-utils [16:31] right ? [16:32] nacc, hm... why do we not default to patches-applied? [16:33] that was going to be my next question, "where are my patches-applied" branches [16:33] then i see that it doesnt have them by default. i suspect due to possible failure of applying due to lack of a tool ? [16:33] ddstreet: where is 'l' (saved to '_l') initialised? [16:34] smoser: see the git commits [16:34] sladen: "l = size;" [16:34] smoser: disabled becasue it doesn't always work (for certain src packages) and the importer can't fail (yet) [16:34] smoser: you can pass --patches-applied [16:34] sladen line 131 [16:35] sladen but if replace_ws, then l is set to UTIL_PATH_SIZE which is the size of sbuf [16:35] nacc, yeah, but that wont be "sticky" [16:35] This code is horrible :-( [16:36] then tmplen is UTIL_PATH_SIZE - l, so if the switch statement doesn't do anything tmplen is 0 and nothing is copied [16:36] rbasak, you're in a friend-making mood :) [16:36] smoser: that is true, but nothing is 'sticky' in the importer (there's no config stored in the repository) [16:36] It might as well be in assembly! [16:36] rbasak i agree [16:37] nacc, and thats fine... i'm just saying that i can do it , you're right, but then as the importer goes on i dont have it. [16:37] ddstreet: yeah, not your fault :) [16:37] so its not terribly helpfu. [16:37] i wonder if we could --patches-applied=attempt [16:37] and that be the default [16:38] smoser: this is mostly temporary while we wait for debian to tell me what policy they want for dgit [16:38] and if it failed, then warn and skip [16:38] smoser: I hope that eventually --patches-applied will not break and be the default. [16:38] smoser: for the cases where it fails [16:38] smoser: so in the meantime, it's just a feature flag awaiting feature specification finialization really. [16:38] smoser: because we have to do something withe branch pointers [16:42] ddstreet: I think I'd have preferred you to have modified the destination in-place instead of temporarily redirecting everything else. [16:42] Though then I suppose that util_replace_whitespace couldn't be called directly. Hmm. [16:44] * sladen just reading util_replace_whitespace() [16:44] and util_replace_whitespace strips leading/trailing whitespace [16:46] hmm i supppose util_replace_whitespace could operate on the same memory as src and dest... [16:46] but that's a bit more clever than is safe for this code i think [16:49] ddstreet: I (think) this code (probably) works. It has taken 45 minutes to understand the implications of the patch. The UTIL_PATH_SIZE - UTIL_PATH_SIZE == 0 step is quite fragile if another patch gets added later. There may well be a much simpler way to implement it. [16:50] The entire switch statement should be in its own ******* function :-/ [16:50] rbasak +1000 [16:50] nacc, so you're saying you would not want --patches-applied right now if it always worked ? [16:50] ddstreet: if it's purely a post-processing step, it should be possible to do it all at the end, with (probably) no additional variables being added before [16:50] as in you're not sure if the implementation is right here ? [16:50] sladen: I appreciate your additional review. Thanks. [16:50] sladen i'm not sure how it's fragile, it calculates the amount of chars added [16:51] sladen what *really* should be done is for strpcpy() to return the number of chars copied, not the new length remaining! [16:52] strpcpy is a function that udev didn't need to re-invent [16:52] but i digress. my patch was done with minimal changes to the existing code in udev, which is why it looks weird. [16:52] udev's existing code is already weird. [16:52] ddstreet: bool replace_whitespace, could probably be const to aid readability, and then could be referenced directly, which would be easier than the 'replws' abbreviation [16:52] smoser: the implementation is correct. The need for the flag is temporary [16:53] smoser: hopefully [16:53] ddstreet: to be fair, you could have refactored it in your upstream patchset. [16:53] sladen it's not possible to do all at the end, because only the variable substitution whitespace should be removed - not any whitespace that was already in the string [16:53] (that isn't my requirement, see the pull req for details) [16:54] rbasak fixing strpcpy? it's used all over udev [16:54] ddstreet: the redirection around the switch statement is pretty hacky and fragile. Better to pull the switch into a separate function. Then you can give the switch statement different pointers. [16:54] nacc, http://paste.ubuntu.com/23793031/ [16:55] ddstreet: that would also ensure that nothing in the switch statement can reach out to other variables in the scope of udev_event_apply_format that might impact the redirection. [16:55] that is what i'm suggesting, that way we get it imported for everything that *does* work, and for stuff that doesnt we warn rather htan fail [16:55] rbasak yeah but i think a separate func is gonna need a lot of params passed in [16:55] smoser: that will break those packages where it doesn't work [16:55] ? [16:55] smoser: in a way that is not fixable later [16:55] i dont follow [16:55] ddstreet: perhaps, but I have to review each of those parameters individually for unintended interactions this way anyway. [16:56] it will just not create those commits [16:56] which is what you're doing now [16:56] no, it will break the historical import algorithm [16:56] for that series [16:56] ddstreet: to check the redirection for correctness, I have to know what other variables the contents of the switch reaches out to. [16:56] how is that different than not doing it? [16:57] rbasak sure i can create a new patch to pull that function out, but the upstream bug is already closed [16:57] smoser: yes. [16:57] ddstreet: understood. This is a passing comment really. I haven't come to a conclusion with my SRU hat yet. [16:57] smoser: because if it's not done, i can go back and 'catch-up' the applied branches [16:57] smoser: if history is *wrong*, I can't. [16:58] rbasak yep, i understand, i'll see what i can do upstream to pull the function out [16:59] i'm missing something. if the patch application fails, you dont create the history, so its just as if you ran without patches-applied [16:59] are you just saying that the commit_patches_applied has side affects ? [17:05] nacc, ? what would be un-correctable ? [17:08] i think you're suggesting that you could potentially import incorrect state and succeed. i'm not sure. if thats the concern then you can never be sure that running of tools provided the same code they did a decade ago. [17:09] ie, samba's failure. although, as i understand it , that was a fialure to apply the patch, and thus my suggestion would just go on. === scottt is now known as Guest19740 [17:20] ddstreet: I think you should use sizeof(x)/sizeof(x[0]) instead of UTIL_PATH_SIZE, etc. Again, maybe not important for this SRU. [17:23] smoser: i don't understand what you mean by 'commit_patches_applied has side effects'? it creates/moves branch pointers [17:23] rbasak yeah i thought of that, but was concerned if sbuf is changed from stack array to a * the sizeof would silently break [17:24] nacc, it does that only if it succeeds to apply a patch [17:24] OTOH, if the size of sbuf is changed, then we'll silently break. [17:24] smoser: no, i'm saying that if make patch-application non-fatal by default, which (you by default are doing), then historical patches-applied imports will 'succeed' but will not be imported anywhere (potentially) [17:24] IMHO, I'd expect someone changing a variable's type to check how something is used. [17:24] More so than the the length of an array. [17:25] smoser: i think you misunderstand what your new 'commit_patches_applied' does [17:25] smoser: it loops over the patches applying each [17:25] smoser: that can fail anywhere in the queue [17:26] nacc, right. so yeah,j it has side affects. [17:26] we'd have to revert the branch [17:26] which honestly woudl not be that hard [17:26] just take the head before attempt and then reset it on fail [17:26] right ? [17:26] what do you do with the *next* one? [17:26] this is not some trivial problem :) [17:30] i dont know. i understand your point. [17:31] smoser: as in, fine, you fail to apply patches, so you leave the current pointer at the prior [17:31] but then FF branches get messy [17:31] and you're dropping state without tagging or anything else [17:31] FF ? [17:31] fast-forward [17:31] right, then the next version tries and succeeds [17:31] and determining its 'parents' are where we're currently undecided [17:32] and that decision is policy, so rather than fake it, i'm avoding the choice [17:32] it really doesnt matter all that much. [17:32] at some point, when we swtich the default for patched-applied, i'll switch the algorithm to also do historical applied-patches imports separate from unapplied [17:32] you do lose some history in taht you dont have those in the middle, but it is history that could not be created. [17:33] and woudlnt be used anyway [17:33] really.. [17:33] because people are only really going to use the tip for applied branches [17:33] well.. that might not be the case, but missing a historic commit doenst matter that much [17:34] it breaks some of our assertions [17:34] that every publish in launchpad is imported, e.g. [17:36] smoser: i agree there are solutions out there for this problem. But they all imply a policy, which once done, needs to become stable and supported. === JanC is now known as Guest43962 === JanC_ is now known as JanC [17:42] smoser: if what you are asserting is true, then i would suggest a *better* choice is a usd sub-command (perhaps a checkout) which takes a series (+pocket?) and checks out the latest publish in that series (+pocket?) and applies the patches to it. [17:42] smoser: the patches-applied state is always deriviable and if users don't care about history, let's not provide history [17:42] smoser: but for dgit, they *do* care about history, and so we have to do the right thing in the importer [17:43] nah. thats not what i was saying [17:43] ddstreet: how feasible would it be to add a test case for this particular bug? [17:43] even in dgit [17:43] they're only really primarily interested in the tip [17:43] its for development, to make changes. [17:43] if a historical point was unable to be created, that is what it is. [17:43] same as if there was no download available for a thing in launchpad or something [17:44] its less than perfect for sure and obnoxious, and i'd probably complain about it later [17:44] :) [17:47] heh [17:47] note that a failure to find a download in launchpad is fatal to the importer [17:47] right. the difference i see is only in that the next import would possibly succeed and then go stopmping on the history. [17:48] in my suggested solution [17:48] rbasak i can do that, there's a big test case perl script built into systemd/udev that's run on compile, i'll add some test cases [17:48] nacc, if i wanted to pull in a new upstraeam release [17:48] when working with this branch... ie, to upload. [17:48] test/udev-test.pl [17:49] what would you do to do that ? [17:49] ie, i'm on ubuntu-devel [17:49] and i want a new upstream tarball [17:49] smoser: does uupdate/uscan work? [17:49] note this might be the same question as the bug cyphermox filed :) [17:50] smoser: LP: #1649940 [17:50] Launchpad bug 1649940 in usd-importer "can't prepare new upstream releases using gbp" [Medium,New] https://launchpad.net/bugs/1649940 [17:51] that's one 'answer', in that we don't actually use the upstream data we have [17:51] smoser: but if you have uupdate/uscan working, what i would suggest (and it's gross) is to do a uupdate/uscan, and then overwrite the working tree to be exactly what is in the new upstream [17:55] nacc, yeah, that is basically what i was asking [17:56] smoser: i think this is where we hit the bodge that is you asking for imported orig tarballs :) [17:56] smoser: in that, we don't really import them -- they exist, but are off to the side [17:56] nacc, so... this is what actually brought me here.. [17:56] heh [17:57] i was tying to use the lpusp branch to do development on cloud-utils now [17:57] my plan is to leave upstream in bzr for now [17:57] and do the usd for working with git [17:57] right [17:57] but then wanted to upll in a new 0.30 that is really the same as what was in ubuntu due to the applied patches [17:57] but i wanted git to tell me that [17:58] and so i wanted to diff against lpusip/applied/0.27-0ubuntu9 [17:58] to see that that was the case [17:59] and that didn't exist [18:01] yeah. [18:01] so that is a non-dgit use of the applied branches [18:01] right [18:01] but my prior point of a catch-up mode for the applied branches would have solved your problem [18:02] correct [18:02] smoser: i can probably get that coded up today, honestly -- we'll need it anyways at some point, probably [18:03] that allows us to continue using the current importer for now and then we can catchup all the repos at some point, change the default on the flag, and then restart the importer with patches-applied? [18:04] sure. one other thing you coulddo.... [18:05] so, i'm concerned that reality will be that we never get to the point that we have these applied branches [18:05] because discussion or whatever policy will drag on and on [18:06] for now we could do as i suggested, and then correctly reset the branch to the last good state. [18:06] and then add a commit "APPLIED_PATCHES_FAILED" [18:06] and then if that was ever seen, dont go on. [18:08] LP: #1649832 [18:08] Launchpad bug 1649832 in usd-importer "Problems in applied patches imports cause import failures which are not ignored" [Wishlist,Confirmed] https://launchpad.net/bugs/1649832 [18:08] smoser: --^ [19:35] stgraber: re-ping [20:21] is eatmydata broken ? [20:21] $ grep LD_LIBRARY_PATH /usr/bin/eatmydata [20:21] LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+"$LD_LIBRARY_PATH:"}/usr/lib/libeatmydata [20:21] export LD_LIBRARY_PATH LD_PRELOAD [20:21] $ ls -l /usr/lib/libeatmydata [20:21] ls: cannot access '/usr/lib/libeatmydata': No such file or directory [20:59] barry: hey there, sorry, in South Africa this week so not the most compatible timezone [21:00] barry: so that's running inside a LXC container and is attempting to mknod /dev/mapper/control. If the container is unprivileged, then that sure won't be allowed by the kernel. If the kernel is privileged, then it'd depend on the lxc.cgroup.devices configuration. [21:01] barry: I'm not aware of any change we made that would explain a change in behavior there, so a change in the adt container configuration seems more likely to be. Or something was at /dev/mapper/control before which has since disappeared and triggers the mknod? [21:01] stgraber: is that something that changed recently? i'm fairly positive that worked original in ubuntu-image dated 10-oct [21:02] stgraber: yeah, *something* has changed, just not sure what. do you know, does autopkgtest.u.c run priv'd or unpriv'd containers? [21:03] barry: I don't know about our adt test environment's config. We haven't changed anything on our side that'd explain that. mknod has always failed in unprivileged containers and always succeeded or failed in privileged containers based on lxc.cgroup.devices config [21:03] barry: no idea [21:04] barry: and unfortunately autopkgtest doesn't log what it used to create the lxd container (like it does for nova) [21:05] stgraber: let me poke around in the apt source and see if i can find something, otherwise i'll email ubuntu-devel@ and see if anybody else has an idea. thanks, this is starting to narrow it down [21:13] barry: note that devmapper itself isn't namespaced, so if the adt container is allowed to interact with it, you may well conflict with another container and run into surprises [21:14] stgraber: i wonder if the apt infra used to run priv'd containers and now runs unpriv'd containers? === salem_ is now known as _salem [21:16] stgraber: that's good to know. maybe the right answer is to skip the mount test if `dmsetup ls` fails. i should at least set isolation-machine restriction in d/tests/control for that test [21:16] that might do it in a more principled way [21:16] barry: I know that the lxc adt backend was using privileged containers with barely any security restriction applied to the container. I suspect the lxd adt backend attempts to be somewhat more secure and may run unprivileged. [21:17] It certainly can run unprivilege. [21:17] d [21:17] I forget whether it does in production. [21:17] cjwatson: maybe Laney knows if he's still online [21:17] barry: yeah and if you set isolation-machine for that test, then adt will just skip it for you when running on lxc/lxd [21:17] But it's what adt-build-lxd(1) and adt-virt-lxd(1) will set up unless you muck about. [21:18] stgraber: given that devmapper isn't namespaced, that's probably the right thing to do anyway, even if it doesn't explain what's changed [21:19] cjwatson: ack [21:19] ok, testing and if it works, yay! but i may still send the email cause i'm still curious [21:20] cjwatson, stgraber: oh, but isolation-machine will prevent the test from running on amd64 too, so then i'm not so sure what the use of the test is ;/ [21:21] Yeah, dunno, my use of lxd is (a) openssh autopkgtests and (b) Launchpad/OLS stuff at the moment :-) [21:21] :D [21:26] barry: no it won't [21:26] * stgraber triple checks [21:27] barry: right, isolation-machine is satisfied by VMs [21:27] barry: we set it for the lxc/lxd/lxcfs/... packages and we do see our tests running on everything but armhf and s390x [21:28] stgraber: then a test that kpartx's and mounts the partitions in a built image is never going to be *safely* tested on current apt infra. would you agree? [21:29] barry: ? I'm saying that if you set isolation-machine, your test will run on amd64, i386 and ppc64el [21:30] stgraber: oh, yes, i see what you're saying. [21:31] stgraber: unfortunately, i usually use schroot for local tests because qemu and until recently lxd wasn't working locally for me. qemu still doesn't for some reason (a crash in kvm.c iirc) [21:49] barry, there is isolation-vm and isolation-container too i think [21:50] to be more specific than just "machine" [21:51] xnox: isolation-container yes, that's to isolate services and tcp ports. isolation-machine looks to be more appropriate for kernel interactions (e.g. devmapper). no isolation-vm restriction that i can see [23:58] smoser: still around?