-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72779/#review221636
-----------------------------------------------------------




src/slave/csi_server.cpp
Lines 130-132 (patched)
<https://reviews.apache.org/r/72779/#comment310706>

    I see this method will always be called with name specifed in 
`CSIServerProcess::publishVolume()` and `CSIServerProcess::unpublishVolume()`. 
So in which case will it be called with no name specified?



src/slave/csi_server.cpp
Line 127 (original), 145 (patched)
<https://reviews.apache.org/r/72779/#comment310707>

    When is this hashmap populated? I see what is populated in 
`CSIServerProcess::initializePlugin()` is a local variable with the exactly 
same name.



src/slave/csi_server.cpp
Line 132 (original), 149 (patched)
<https://reviews.apache.org/r/72779/#comment310709>

    So basically this method does two things: load the plugin config files and 
initialize the plugins. I would suggest we still do the former in 
`CSIServer::create()` so if there is any errors (like `JSON::parse()` or 
`protobuf::parse()` fails) in the plugin config files we can error out eariler, 
otherwise users may only find the error in the runtime and they have to fix the 
error with restarting agent.



src/slave/csi_server.cpp
Line 136 (original), 155 (patched)
<https://reviews.apache.org/r/72779/#comment310708>

    If `name` is specified, do we need to do this `os::ls`? I think we can just 
load and initialize the specified plugin directly without traversing the 
entries in `pluginConfigDir`, right?


- Qian Zhang


On Aug. 19, 2020, 2:23 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72779/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2020, 2:23 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch updates the CSI server to attempt to
> initialize unknown plugins when it receives publish
> or unpublish calls meant for plugins that it does
> not recognize.
> 
> 
> Diffs
> -----
> 
>   src/slave/csi_server.hpp f5ec7663926731483589c5e30e7060751ed01e55 
>   src/slave/csi_server.cpp 2ba4f22b92370b019722d845d6113fb37f1b876a 
> 
> 
> Diff: https://reviews.apache.org/r/72779/diff/2/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*CSIServerTest*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Reply via email to