> On March 9, 2016, 12:03 a.m., Ben Mahler wrote: > > src/slave/flags.hpp, line 92 > > <https://reviews.apache.org/r/44365/diff/1/?file=1280277#file1280277line92> > > > > Can you put this in an ifdef?
Done. I also moved it out of the `#ifdef __linux__ `directive. Although we only support gpus on linux at the moment, there's nothing preventing us from supporting them on other platforms in the future. This should be treated as a generic flag in that respect (i.e. nvidia-smi is also a command that can be run on windows). > On March 9, 2016, 12:03 a.m., Ben Mahler wrote: > > src/slave/flags.cpp, lines 597-607 > > <https://reviews.apache.org/r/44365/diff/1/?file=1280278#file1280278line597> > > > > -s/nvidia_gpus/nvidia_gpu_devices/ > > -capitalize NVML > > -s/as a resource/as resources/ > > -maybe put the nvidia-smi comment in a parenthetical? > > -The third sentence seems to be already expressed in the first? Perhaps > > clarify that the isolator will be used only if the `--isolation` flag > > contains the right value. > > -We can probably omit or re-pharse the last sentence since this flag is > > omitted from the program if the configure flag is not present. Done. I also updated the string in `docs/configuration.md`. In the `.md` file, I left the (slightly reworded) last line, since there is no way of `#ifdef`ing this flag out in the documentation. > On March 9, 2016, 12:03 a.m., Ben Mahler wrote: > > src/slave/flags.cpp, lines 606-607 > > <https://reviews.apache.org/r/44365/diff/1/?file=1280278#file1280278line606> > > > > I was going to suggest we add a validator here, but even better would > > be to add a parse specialization to src/common/parse.hpp and just avoid the > > need for the extra validation altogether. Because it is so generic, I decided to add this parser to stout instead of mesos. It should appear as a a dependency of this patch once I push it to reviewboard. As part of this, I now set `vector<const unsigned int>` as the return type on the `nvidia_gpu_devices` flag. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44365/#review122628 ----------------------------------------------------------- On March 14, 2016, 7:38 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44365/ > ----------------------------------------------------------- > > (Updated March 14, 2016, 7:38 a.m.) > > > Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya. > > > Bugs: MESOS-4864 > https://issues.apache.org/jira/browse/MESOS-4864 > > > Repository: mesos > > > Description > ------- > > Added flag to specify available Nvidia GPUs on an agent's command line. > > > Diffs > ----- > > docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b > src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 > src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd > > Diff: https://reviews.apache.org/r/44365/diff/ > > > Testing > ------- > > ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,string > Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,string': > Expecting all elements to be unsigned integers > > ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2.0,3 > Failed to load flag 'nvidia_gpu_devices': Failed to load value '1,2,3.0': > Expecting all elements to be unsigned integers > > ./bin/mesos-slave.sh --master=127.0.0.1:5050 --nvidia_gpu_devices=1,2,3 > SUCCESS > > > Thanks, > > Kevin Klues > >
