----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22898/#review46482 -----------------------------------------------------------
Two things: (1) How does this surface to the operator? Will we dump a stack trace when it's not supported or will we exit cleanly with a message? I think we should do the latter. (2) Do we need the error case, since we can assume linux? (See my comments below). src/linux/perf.hpp <https://reviews.apache.org/r/22898/#comment81867> // Returns whether the given events are accepted by `perf stat`. src/linux/perf.hpp <https://reviews.apache.org/r/22898/#comment81866> I think it's important to describe what this is returning, since it's telling us whether it's supported: // Returns whether perf is supported on this host. If we say "check if perf is supported" that reads like a block of code that decides to exit or bail if perf is not supported, no? src/linux/perf.cpp <https://reviews.apache.org/r/22898/#comment81868> else if? src/linux/perf.cpp <https://reviews.apache.org/r/22898/#comment81869> ditto src/linux/perf.cpp <https://reviews.apache.org/r/22898/#comment81870> period src/linux/perf.cpp <https://reviews.apache.org/r/22898/#comment81871> Is this error possible if we're on a linux machine? I'm wondering if we can just CHECK_SOME and avoid the error / failure propagation altogether. On linux, I only see: ERRORS EFAULT buf is not valid. - Ben Mahler On June 23, 2014, 11:25 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22898/ > ----------------------------------------------------------- > > (Updated June 23, 2014, 11:25 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-1394 > https://issues.apache.org/jira/browse/MESOS-1394 > > > Repository: mesos-git > > > Description > ------- > > Adds a supported() check for perf that requires version >= 2.6.39. > > > Diffs > ----- > > src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db > src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa > src/slave/containerizer/isolators/cgroups/perf_event.cpp > 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90 > > Diff: https://reviews.apache.org/r/22898/diff/ > > > Testing > ------- > > make check # Linux > > > Thanks, > > Ian Downes > >
