Storage: Add NVMe and SDC storage connectors #14710
Conversation
|
As previously discussed with @roosterfish, I have also replaced nvme "disconnect-all" and "connect-all" by connecting to just a specific target. This prevents disconnecting all nvme volumes, which would affect other storage drivers as well. However, needs to be checked that NQN is correctly parsed and passed to the connector. |
There was a problem hiding this comment.
Thanks for this, looking great already.
I'll grab your branch and make some tests on the PowerFlex cluster for validation. I think the only change that has to be performed upfront is using the actual (Connect|Disconnect)All funcs to resemble the original behavior.
When we have merged the PR I can take care of switching to the Connect/Disconnect instead.
MusicDin
force-pushed
the
feat/storage-connector
branch
from
67f9bf3 to
4846572
Compare
|
@roosterfish Thanks for the comments. I've addressed all of them, and reverted fixes for |
Thanks, I'll run it through some tests on the PowerFlex env and report back here. |
lxd/storage/connectors/utils.go
Outdated
| } | ||
|
|
||
| // Skip volumes that do not contain the desired name suffix. | ||
| if !strings.HasSuffix(diskPath, diskNameSuffix) { |
There was a problem hiding this comment.
Before this was using !strings.Contains() because there are characters after the PowerFlex volume ID.
This breaks the disk discovery. I have just tested it in our PowerFlex lab and this is the first issue I saw.
Maybe should findDiskDevicePath accept a func(string) bool instead so the caller can pass an arbitrary validator that either checks Contains or HasSuffix depending on the individual format of the disk name under by-id/?
There was a problem hiding this comment.
As an example when creating a disk in PowerFlex it's name will be /dev/disk/by-id/nvme-eui.b526f9710000000064b94e07b19c1c0f.
In this case b526f97100000000 is the volume id.
There was a problem hiding this comment.
Good point, accepting an arbitrary function also seems versatile solution for any potential upcoming storage drivers
There was a problem hiding this comment.
I've added the devicePathFilter which is a function that accepts device path, and returns bool indicating whether the path matches the required criteria.
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
…or lock Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
…name Signed-off-by: Din Music <[email protected]>
…system Signed-off-by: Din Music <[email protected]>
…inter) Signed-off-by: Din Music <[email protected]>
MusicDin
force-pushed
the
feat/storage-connector
branch
from
4846572 to
0e0ad2b
Compare
This PR contains cherry-picks from #14700 to include only connectors and PowerFlex changes (without Pure Storage bits)
The aim of the connector is to handle connections with the target storage. Currently, connectors support NVMe, and SDC.
Closes #14708