Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added initial support for SPDK v19.07 #85

Closed
wants to merge 2 commits into from

Conversation

tbykowsk
Copy link
Contributor

@tbykowsk tbykowsk commented Nov 7, 2019

This pull request introduces to Linstor a possibility to create a storage pool from SPDK lvol store, construct SPDK lvol bdev and expose it as SPDK nvmf subsystem.

How to start using it:

  1. Download and compile SPDK v19.07
  2. Setup SPDK
    spdk-19.07/scripts/setup.sh
  3. Start SPDK nvmf_tgt
    spdk-19.07/app/nvmf_tgt/nvmf_tgt
  4. Create SPDK nvme bdev from a drive
    spdk-19.07/scripts/rpc.py construct_nvme_bdev -b NVMe1 -t PCIe -a 0000:3e:00.0
  5. Create SPDK lvol store on the nvme bdev
    spdk-19.07/scripts/rpc.py construct_lvol_store NVMe1n1 sample-lvol-store
  6. Create a symlink to SPDK rpc.py script
    ln -s spdk-19.07/scripts/rpc.py /usr/bin/rpc.py
  7. Start Linstor and create a SPDK storage pool and a resource from it
linstor node create sample-node 10.1.0.1 --node-type Combined
linstor storage-pool-definition create spdk-pool
linstor storage-pool create spdk sample-node spdk-pool sample-lvol-store
linstor resource-definition create sample-resource
linstor volume-definition create sample-resource 10G
linstor resource create --layer-list nvme,storage --storage-pool spdk-pool sample-node sample-resource

Signed-off-by: Tomasz Bykowski <tomasz.bykowski@intel.com>
Copy link
Contributor

@ghernadi ghernadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

Here are some things that I discovered while quickly reviewing the code (not tested yet)

Major:
satellite/src/main/java/com/linbit/linstor/storage/layer/adapter/nvme/NvmeLayer.java: 133-140
getSingleChild() might return LuksRscData or other *Data from layers between nvme and storage
(in this case it might be a bit confusing that VlmLayerObject is extending VlmProviderObject).
To solve this issue you might want to use LayerRscUtils.getRscDataByProvider(rscLayerObject, DeviceLayerKind.STORAGE). However, usually it makes sense to at least briefly think about cases where one NVME-volume is composed of multiple STORAGE-volumes (things like these will come in future). It is OK if you say this layer only works if ALL NVME-volumes are SPDK (if not, fall back to normal NVMe logic). I dont think there is right now a "correct" solution - just keep things like this in mind.

Major:
/satellite/src/main/java/com/linbit/linstor/storage/layer/adapter/nvme/NvmeUtils.java: 56-57
Agreed, but we still cannot simply change that prefix that easily as that would break compatibility with already deployed nvme devices after upgrading linstor-satellite and restarting the satellite where the nvme devices still having the old names.

Minor:
/satellite/src/main/java/com/linbit/linstor/storage/layer/adapter/nvme/NvmeSpdkUtils.java: 160
error message states "Failed to create subsystem!", whereas the method just tried to delete the subsystem

@tbykowsk
Copy link
Contributor Author

Thank you very much for the review!
Please verify if the issues have been resolved as you requested

@ghernadi
Copy link
Contributor

ghernadi commented Dec 4, 2019

This PR has been rebased on top of our master - the merge conflicts above can therefore be ignored.
SPDK support is included in v1.3.0

@ghernadi ghernadi closed this Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants