[00:49:34] *** Quits: ziyeyang_ (~ziyeyang@134.134.139.82) (Quit: Leaving) [01:24:46] *** Joins: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) [02:11:37] jimharris: should we still support fio-2.21 for fio_plugin? [02:12:01] could we have some machines on TP using fio-3.x and some 2.x? [02:13:26] we will be able to use some new fio flags for tests that are only run on fio-3.x machines [02:33:33] *** Quits: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [02:43:29] *** Joins: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) [02:43:59] *** Quits: Shuhei (caf6fc61@gateway/web/freenode/ip.202.246.252.97) (Ping timeout: 260 seconds) [02:44:33] *** Quits: sbasierx__ (sbasierx@nat/intel/x-almietdawzfjwnvp) (Ping timeout: 248 seconds) [02:47:19] *** Joins: sbasierx__ (sbasierx@nat/intel/x-rkpfscypibfwsajb) [02:53:01] *** Joins: guerby_ (~guerby@ip165.tetaneutral.net) [02:53:19] *** Quits: guerby (~guerby@april/board/guerby) (Ping timeout: 252 seconds) [02:58:07] *** Quits: guerby_ (~guerby@ip165.tetaneutral.net) (Excess Flood) [02:58:19] *** Joins: guerby_ (~guerby@ip165.tetaneutral.net) [02:59:08] *** guerby_ is now known as guerby [02:59:18] *** Quits: guerby (~guerby@ip165.tetaneutral.net) (Changing host) [02:59:19] *** Joins: guerby (~guerby@april/board/guerby) [04:15:20] *** Joins: tomzawadzki (~tomzawadz@192.55.54.41) [04:18:00] *** Quits: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [04:20:01] *** Quits: tomzawadzki (~tomzawadz@192.55.54.41) (Client Quit) [04:20:17] *** Joins: tomzawadzki (~tomzawadz@134.134.139.83) [04:21:12] *** Parts: tomzawadzki (~tomzawadz@134.134.139.83) () [04:21:33] *** Joins: tomzawadzki (~tomzawadz@192.55.54.41) [04:27:13] *** Parts: tomzawadzki (~tomzawadz@192.55.54.41) ("Leaving") [04:31:41] *** Joins: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) [05:22:10] *** Quits: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [06:40:06] *** Quits: sbasierx__ (sbasierx@nat/intel/x-rkpfscypibfwsajb) (Quit: Going offline, see ya! (www.adiirc.com)) [06:44:13] *** Joins: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) [07:41:58] *** Joins: lhodev (~Adium@inet-hqmc02-o.oracle.com) [07:51:30] darsto: how many ifdefs would we need to support both? [07:53:10] jimharris: probably one per fio file [07:53:24] i'm fine with supporting both then [07:53:50] k [07:54:21] apparently a couple of TP machines have been using fio-3.x for quite some time now [07:57:33] Good morning guys. After my 1st push to GerritHub I noted the "SPDK Automated Test System" indicated an issue. I followed the links to discover (embarrassingly) that if I had run ./configure with "—enable-debug" I would've caught a simple syntax error. So, anyway, I corrected that and re-pushed. Question: should I see another appearance of "SPDK Automated Test System" in the History at the bottom of the page reflecting a new compil [07:57:33] e/verification? I haven't seen such a 2nd automated test entry (though my upload of patch set 2 did appear). Do *I* need to click on some button to nudge one along? [07:58:31] good morning Lance [07:58:49] there's nothing else that you need to do - I just approved your next rev to run through the test pool [07:59:14] Ok, great, and thank you! [08:08:12] darsto: just talking with tomek - I think we can support v3.3 only [08:34:56] *** Quits: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [08:40:14] lhodev: looks like scan-build found some static analysis bugs (memory leaks) - you can find the details here: http://spdk.intel.com/public/spdk/builds/review/e5e712d7a07fa9c9d83f12628de48a5f7e1b1529.1515509888/ [09:10:26] Hey Jim. I'm going through that now. I see the complaint, but am attempting to learn how to resolve it. The allocating function, spdk_push_arg(), performs a free() in the event of a realloc() failure thereby alleviating the caller from needing to do that. The one corner case that remains is for the scenario that the caller of spdk_push_arg() passes in an invalid arg in which case it will return a fail. I'm left wondering if for tha [09:10:26] t scenario if invoking the spdk_free_args() will squelch scan-build, -OR- whether I'll need to do something in the caller, spdk_build_eal_cmdline(), to perform some kind of free()'ing action; i.e. that scan-build may want to see a free in the same scope of the calling function. [09:54:24] maybe spdk_push_arg should just assert(arg != NULL)? [09:55:04] or maybe something like: [09:55:08] if (arg == NULL) { [09:55:17] spdk_free_args(args, *argcount); [09:55:23] assert(false); [09:55:25] return NULL; [09:55:26] } [10:09:43] I'll continue to play around with it. I want to avoid assert() as that could result in an invocation of abort() — something (along with exit()'s) that I'm seeking to eliminate. [10:42:42] lhodev: SPDK uses assert() extensively already though - are you thinking of trying to eliminate those too? [10:47:10] The low-hanging fruit task on Trello, https://trello.com/c/ZB4UEkXe/7-remove-use-of-exit-and-abort-for-error-handling-in-library-code, explicitly called out exit() and abort(). One might extrapolate we pursue assert()'s too in the lib code, but that may be a debatable point. There's the caveat in the Trello task description, "…only potentially use abort() in truly unrecoverable code paths". [10:48:01] assert is a bit different in my mind, because exit/abort is usually for conditions that may occur but aren't recoverage, whereas assert is only for conditions that should never occur [10:48:07] the Trello task should be clarified to indicate indirect aborts through assert() are OK [10:48:16] so we use assert mostly as a sanity check and to guide the static analyzer [10:48:38] recoverage->recoverable [10:48:42] that was a weird typo [10:48:58] thanks for the explanation Lance - I'll update the Trello task [12:59:43] *** Quits: sethhowe (~sethhowe@192.55.54.42) (Remote host closed the connection) [13:00:03] *** Joins: sethhowe (~sethhowe@192.55.54.42) [13:00:10] *** Quits: sethhowe (~sethhowe@192.55.54.42) (Remote host closed the connection) [13:16:16] Hey Jim. I amended the commit that appeared to resolve the mem leak complaint in lib/env_dpdk/init.c. By "appeared", I refer to the analysis output I get when running a version of scan-build on my development system. Curiously, my version of scan-build (clang-analyzer-3.4.2-8) barked about a couple other things which did not appear in the previous report conducted on GerritHub so I'm eager to see the output from my most recent amend. [13:16:48] ok - it's in the run queue now [13:17:20] https://ci.spdk.io/status/ [13:17:22] the queue [13:24:39] Great, thank you. [13:33:39] I see the build summary is a "Pass" across the board for the last amend. And, so at least for now, there was much rejoicing ;-). [13:36:28] and you even added a CHANGELOG entry :) [13:36:58] I aim's t'please ! [13:37:09] *** Joins: sethhowe (~sethhowe@134.134.139.83) [13:38:17] did you do that all on your own or because ./scripts/check_format.sh recommended it? [13:38:26] just for my own data points [13:39:33] I put that check into check_format.sh and I still ignore it half the time [13:39:53] Believe it or not, entirely on my own. I *truly* do value documentation and it was definitely on my mind from the get-go once I knew I was altering publicly-facing interfaces. [13:40:24] I figured as much - I'm not sure that check_format.sh is really modifying behavior much, myself included [13:40:34] oh well, it doesn't hurt [13:40:45] check_format.sh only modifies behavior when it fails your patch :-) [13:41:15] I could definitely make it a failing check if we wanted to go that direction [13:41:32] I don't think that's possible [13:41:39] To be sure, I also ran check_format.sh. Another peeve of mine is consistency in code formatting. I've been on projects where holy wars surrounded that topic. *ugh* [13:41:50] it can check that your patch modified CHANGELOG.md [13:42:01] but it can't check that you wrote something intelligible or related [13:42:13] but how does it know when you're supposed to edit CHANGELOG.md? [13:42:21] it already detects that [13:42:26] if you modified a header file in include/spdk/ [13:42:31] plus a small list of other things [13:42:37] I guess if you just add documentation [13:42:42] you're in trouble [13:42:42] so if I add API documentation? [13:42:50] yeah - that's what I was thinking about [13:43:28] I wonder if I could make gcc strip comments on the before and after and see if there is still any change [13:44:02] maybe some day when I have less to do [13:44:16] our changelogs are already several orders of magnitude better than they used to be [13:44:33] yeah - but then if you fix formatting of the code... [13:44:58] I agree - it's way better - as long as the maintainers are thinking about it, it will get done [13:45:38] When you guys are referring to API documentation, are you specifically talking about the doxygen commenting? [13:45:45] yeah [13:45:46] yes [13:45:57] k. Just wanted to ensure I grok'd that. [13:46:10] the ones in include/spdk/* are the "public" SPDK API [13:46:16] and the documentation ends up here ultimately: http://www.spdk.io/doc/files.html [13:46:25] Right. [13:54:08] Thanks for the reviews guys. Jim, I noted your comment "…let this sit for 24 hours……Otherwise we will commit this." So, to ensure I understand — since this is my first full start-to-fish path thru Gerrit, then I'm "done"? I'm not yet clear on how/when one deals with items appearing in the "Conflicts With" section of the GerritHub UI. [13:54:45] yes - you are done unless one of the following occurs: [13:55:00] 1) someone else marks the patch -1, meaning it needs more revisions [13:55:17] 2) one of the "Conflicts with" patches gets committed before yours, in which case you would need to rebase [13:56:23] How am I notified if #2 happens? [13:56:52] you'll see "Merge Conflict" in red in your Gerrit view [13:57:25] we will also sometimes change a +2 vote to a -1 and request the rebase explicitly [13:57:40] as a review comment [13:58:20] sorry - Merge Conflict is not in red - but it will be in the column between the patch name and the owner [13:59:12] if there is a merge conflict and you click on the patch, you will see "Cannot Merge" in red (I know something related to merge conflicts was in red!) [13:59:18] Regarding the "Merge Conflict" in the Gerrit view, I'm assuming since it's in red I'll probably notice it, but was just generally curious where on the page that appears. Down in the bottom in the History section? Middle of page (near the Code-Review and Verified votes? [13:59:48] type this in the Gerrit search: [13:59:58] projects:spdk status:open [14:00:20] you should see some patches from that search that say "Merge Conflict" in the Status (second) column [14:01:26] Ah, gotcha. And thank you. [15:17:29] *** Joins: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) [15:27:17] PSA: Pausing the build pool to make a copy of a disk image for debugging purposes. [15:27:41] *** Joins: darsto__ (~dstojacx@89-68-135-211.dynamic.chello.pl) [15:28:51] *** Quits: darsto_ (~dstojacx@89-68-135-211.dynamic.chello.pl) (Remote host closed the connection) [15:34:42] PSA: Build pool is back up. [16:00:24] *** Quits: gila (~gila@5ED4D9C8.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [16:04:44] *** Joins: Shuhei (caf6fc61@gateway/web/freenode/ip.202.246.252.97) [16:08:26] Hi Jim, if you are busy it is OK for me to do instead of you because I would like to complete the patch of adding LUN which will follow the patch 390843. [16:23:05] *** Joins: Shuhei_ (caf6fc61@gateway/web/freenode/ip.202.246.252.97) [16:24:44] *** Quits: Shuhei (caf6fc61@gateway/web/freenode/ip.202.246.252.97) (Ping timeout: 260 seconds) [16:25:32] *** Quits: Shuhei_ (caf6fc61@gateway/web/freenode/ip.202.246.252.97) (Client Quit) [16:26:09] *** Joins: Shuhei (caf6fc61@gateway/web/freenode/ip.202.246.252.97) [18:17:50] hi Shuhei [18:18:43] sure - if you would like to do the rebase work for that patch it would be great [18:19:20] you can see we pushed a patch from Pawel (pwodkowx) that removed the lun database - so there is rebasing needed on my patch now [18:19:42] I need to leave the office now but will check on your updates tomorrow [18:41:19] *** Joins: ziyeyang_ (ziyeyang@nat/intel/x-vlcsltthitskywoj)