[02:03:12] *** Quits: cunyinch_ (~cunyinch@134.134.139.77) (Remote host closed the connection) [04:43:34] *** Quits: tsuyoshi (b42b2067@gateway/web/freenode/ip.180.43.32.103) (Quit: Page closed) [06:53:36] jimharris, see my comments in response to your comments and let me know whether you'd prefer I just submit one larger patch or continue with the series as broken up. With my limited editing capabilities right now (fixing that today w/dedicated machine) it was simply easier to make one patch with all the small changes in scattered files and then a series of patches all against one file as opposed to a series of patches touching multiple files each. [07:47:54] *** Quits: peluse_ (~peluse@2600:8800:140a:ae18:ecbc:32da:506e:6aa5) (Quit: Leaving...) [10:19:25] peluse: please continue with the series as broken up [10:19:47] i think that patch #1 should be broken into smaller chunks (once your dedicated machine is set up) [10:20:07] for example the .gitignore and test/lib/scsi/lun/Makefile changes are not related at all to the NVMe unit testing [12:48:56] can do! [12:49:22] should be today unless my laptop catches fire, or I get in a car accident next or some shit like that... [13:47:23] jimharris, FYI the only change I can yank from the first patch is .gitignore. The makefile example you mentioned will cause a link failure without it [13:47:27] LINK test/lib/scsi/lun/lun_ut [13:47:29] collect2: error: ld returned 1 exit status [13:47:30] Makefile:51: recipe for target 'lun_ut' failed [13:48:33] jimharris, so I can either just yank .gitignore or redo the whole patchset and have set #1 include everything pluse like one function in nvme_ut.c and add the rest of the functions separately or I can keep it as is. Or I can make the whole thing one big patch. [13:48:59] jimharris, but I'm learning a ton using ben's instructions for keeping a series on one branch and updating old commits, etc. [13:49:30] the .gitignore stuff should go in with the local unittest coverage patch, but I think the Makefile part is necessary for setting up the wrapped functions, right? [13:49:58] so just pulling .gitignore seems like the right thing to me [13:50:00] why does the lun_ut makefile require those changes though? [13:50:16] drv, the .giignore is in that other set. I jsut accidentally had it in here due to patch munging and left it but will pull it as its obivously no big deal [13:50:25] jimharris, beat the hell out of me :) [13:50:25] oh, I see, I didn't notice it was in the SCSI tests makefile [13:50:28] hmmmm [13:50:44] probably because that is the only other thing that currently links to spdk_cunit [13:50:52] oh [13:51:00] but on my deathbed, I hear I will receive total consciousness... [13:51:03] you had that JSON output stuff wired up there at some point [13:51:07] and nothing else uses it yet [13:51:33] yeah - I think we want a central place to hold these wrap/real functions [13:51:38] drv, yeah, I think you're right. the other makefile changes I had to make were both the ldflgs and linking spdk_cunit both [13:52:07] yeah, I think it's fine (although could maybe use a mention in the commit message so we remember what we were thinking later) [13:52:21] jimharris, the question is - do we do that in spdk_cunit or somewhere else? That seemed like the initial most logical place but I'm open to suggestions, it's not really cunit stuff afterall [13:52:34] can you post the full link error for lun_ut? [13:52:53] yeah, I did, here it is again though: [13:53:06] LINK test/lib/scsi/lun/lun_ut [13:53:08] collect2: error: ld returned 1 exit status [13:53:09] Makefile:51: recipe for target 'lun_ut' failed [13:53:20] hmm, that didn't paste everything for some reason [13:53:20] that's it? it doesn't say why ld returned 1 exit status? [13:53:37] it does, I'll try pasting the specific line [13:54:00] heh, it won't paste it. one sec I'll gist it [13:55:02] jimharris: did you see the RocksDB env change? it is a prereq for the final spdk_bdev patch (no rush, just didn't want you to miss it) [13:55:07] https://gist.github.com/peluse/4f68fd48517bae6f3275ab4843a3f44b [13:56:00] ah, of course [13:56:43] * jimharris is thinking... [13:57:42] maybe we create a new test library specific to these wrappers? drv is right - originally spdk_cunit was meant to just do special json stuff with cunit data [13:57:52] yeah, we could make an equivalent to test_env.c [13:57:59] or a real library if that is preferable [13:58:20] (test_env.c is meant to mock up the env layer) [13:58:24] i'd also like those ldflags in some kind of mk/???.unittest.mk file [13:59:00] so that as we start using this new library in more makefiles, and add 50-100 wrapper functions, we don't duplicate these ldflags everywhere [13:59:28] yeah, I looked for the best place to put all that shit for the first patch but didn't want to discuss to much w/o a patch to drive the discussion and didn't want to change a ton of little files w/o some agreement [13:59:42] i *think* it is harmless to always include those ldflags, even if you are not actually linking in whatever library/file has the implementations [13:59:42] so yeah, agree for sure [13:59:55] wait - no, what am I thinking? [14:00:07] * jimharris puts down the pipe [14:00:20] * peluse asks for his turn [14:00:48] can we make those LDFLAGS conditional on CONFIG_DEBUG somewhere? [14:01:10] we don't want to build the real apps/examples with those LDFLAGS even if debug is on [14:01:13] just the unit tests [14:01:23] so the unittest.mk file is probably the way to go [14:01:27] peluse: are you still in the office or are you at home? [14:01:58] i'd like to have a whiteboard discussion on the cmocka push/pop idea [14:02:08] I"m at home for another 15 min or so then I'll be back. Plan on being in the lab at a little after 3 after meeting upstairs for 30 min [14:02:18] maybe i'll just write it up and we can iterate on it [14:03:02] jimharris, if you're still there I'll be there but yeah lets do it now and I'll work it into the series [14:03:53] I'll try to come down a little earlier, will head up that direction now [15:49:09] *** Quits: edwinrodriguez (d8f01e19@gateway/web/freenode/ip.216.240.30.25) (Ping timeout: 260 seconds) [15:50:27] drv: next steps for the dpdk submodule? [15:51:33] the status page only seems to be showing Release builds [16:08:41] we are hooking up the sync script for the public server, under construction [16:09:23] let me grab the DPDK patch and do some testing, but it looks OK to me at first glance [16:14:34] we probably want to take out the "force using internal submodule for testing for now" [16:18:12] other than that, I think it's good to go [16:43:45] yeah - I'm thinking I remove that part and then we can check it in [16:44:01] then for general usage (outside of test pool) people can start using it [16:47:37] once it's merged and everybody has a chance to rebase, we can remove the local DPDK dir from some of the test pool machines [18:56:58] can we put a WIP button on gerrit? I think its supported somewhere/somehow. I just -1'd my own patches for now to indicate they are WIP [19:19:11] and drv, jimharris can you pls look at landing http://spdk.intel.com/gerrit/#/c/7716/ before moving any unit tests around in the tree? [20:13:07] *** Joins: johnmeneghini (~johnmeneg@pool-173-76-8-123.bstnma.fios.verizon.net) [23:04:02] *** Quits: johnmeneghini (~johnmeneg@pool-173-76-8-123.bstnma.fios.verizon.net) (Quit: Leaving.)