----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63387/#review192946 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/storage/provider.cpp Lines 310 (patched) <https://reviews.apache.org/r/63387/#comment271334> Can you add a comment about why `sequence` is needed? src/resource_provider/storage/provider.cpp Lines 668 (patched) <https://reviews.apache.org/r/63387/#comment271314> attach the failure? src/resource_provider/storage/provider.cpp Lines 696-698 (patched) <https://reviews.apache.org/r/63387/#comment271315> Can you comment on why this is not a fatal error and it is safe to ignore? src/resource_provider/storage/provider.cpp Lines 749-752 (patched) <https://reviews.apache.org/r/63387/#comment271321> Why do you need this? Can you add a `default` and set it to UNREACHABLE? src/resource_provider/storage/provider.cpp Lines 932-933 (patched) <https://reviews.apache.org/r/63387/#comment271336> What if there are multiple resources (e.g, PATH) disk that points to the same volume id? Can you add some comments about that? src/resource_provider/storage/provider.cpp Lines 933 (patched) <https://reviews.apache.org/r/63387/#comment271332> Can you add a CHECK here asserting that the resource is a disk resource with an ID? src/resource_provider/storage/provider.cpp Lines 962-965 (patched) <https://reviews.apache.org/r/63387/#comment271337> Can this just be `default`? src/resource_provider/storage/provider.cpp Lines 1578-1582 (patched) <https://reviews.apache.org/r/63387/#comment271325> Do you need this? src/resource_provider/storage/provider.cpp Lines 1636-1640 (patched) <https://reviews.apache.org/r/63387/#comment271326> DO you need this? src/resource_provider/storage/provider.cpp Lines 1704-1708 (patched) <https://reviews.apache.org/r/63387/#comment271327> Do you need this? - Jie Yu On Dec. 5, 2017, 5:59 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63387/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2017, 5:59 a.m.) > > > Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht. > > > Bugs: MESOS-8143 > https://issues.apache.org/jira/browse/MESOS-8143 > > > Repository: mesos > > > Description > ------- > > Storage local resource provider can now handle PUBLISH events. > Although we don't do UNPUBLISH for now, the unpublish function is > still required for deleting volumes. > > > Diffs > ----- > > src/Makefile.am d5ca797c5a57a2763374d391a7e9358fe29f5e06 > src/csi/state.hpp PRE-CREATION > src/csi/state.proto PRE-CREATION > src/csi/utils.hpp 54cf34b40bd48282bbe3f8bcfe664540d9ce79b5 > src/csi/utils.cpp 590e5f4f763734748932584e383287bf94a5ec18 > src/resource_provider/storage/provider.cpp > d35b0d02992e3730ca47906b34c21e1ba9c653e7 > > > Diff: https://reviews.apache.org/r/63387/diff/6/ > > > Testing > ------- > > make > > > Thanks, > > Chun-Hung Hsiao > >
