----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63388/#review192955 -----------------------------------------------------------
src/resource_provider/storage/provider.cpp Lines 1901-1931 (patched) <https://reviews.apache.org/r/63388/#comment271347> See my other comments. Let's move the resource generation/conversion part into a separate helper and make `createVolume` a pure CSI operation. src/resource_provider/storage/provider.cpp Lines 1947-1949 (patched) <https://reviews.apache.org/r/63388/#comment271345> Hum, i like to keep `deleteVolume` helper simple to just CSI related operations (like you did for publich/unpublish related operations). The generation of the resources conversion should be in `applyOfferOperation`. Ditto for `createVolume` src/resource_provider/storage/provider.cpp Lines 1971 (patched) <https://reviews.apache.org/r/63388/#comment271343> typo? src/resource_provider/storage/provider.cpp Lines 2038 (patched) <https://reviews.apache.org/r/63388/#comment271342> using iterator in the interface is something we never do in other places. I'd suggest we keep track of a hashmap with key being the operation uuid. - Jie Yu On Dec. 5, 2017, 2:19 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63388/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2017, 2:19 a.m.) > > > Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht. > > > Bugs: MESOS-8108 > https://issues.apache.org/jira/browse/MESOS-8108 > > > Repository: mesos > > > Description > ------- > > For legacy operations, we just call Resources::apply(). New operations > CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are > implemented through CSI. Specially, DESTROY_* requires unpublishing > the resources first. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > d35b0d02992e3730ca47906b34c21e1ba9c653e7 > > > Diff: https://reviews.apache.org/r/63388/diff/5/ > > > Testing > ------- > > make > > > Thanks, > > Chun-Hung Hsiao > >
