Skip to content

Commit 218c870

Browse files
committed
nvmeof: do not pass mutable parameters to RBD CreateVolume
Do not pass any mutable parameters to the RBD CreateVolume call. NVMe-oF manages its own mutable parameters (QoS and host list) separately and these should not be passed to RBD. The NVMe-oF mutable parameters are defined in a shared slice: - nvmeofRWIOsPerSecond - nvmeofRWMbytesPerSecond - nvmeofRMbytesPerSecond - nvmeofWMbytesPerSecond - allowHostNQNs This shared slice is used in CreateVolume, ControllerModifyVolume, and validateCreateVolumeRequest to avoid duplication. Assisted-by: AskBob <askbob@ibm.com> Signed-off-by: Niels de Vos <ndevos@ibm.com>
1 parent 190a06f commit 218c870

1 file changed

Lines changed: 43 additions & 33 deletions

File tree

internal/nvmeof/controller/controllerserver.go

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ import (
4141
"github.com/ceph/ceph-csi/internal/util/log"
4242
)
4343

44+
// nvmeofMutableParams lists all NVMe-oF specific mutable parameters.
45+
var nvmeofMutableParams = []string{
46+
nvmeof.RwIosPerSecond,
47+
nvmeof.RwMbytesPerSecond,
48+
nvmeof.RMbytesPerSecond,
49+
nvmeof.WMbytesPerSecond,
50+
nvmeof.AllowHostNQNs,
51+
}
52+
4453
type Server struct {
4554
csi.UnimplementedControllerServer
4655

@@ -99,6 +108,8 @@ func (cs *Server) ValidateVolumeCapabilities(
99108
}
100109

101110
// CreateVolume creates a new RBD volume and exposes it through NVMe-oF Gateway.
111+
//
112+
//nolint:gocyclo,cyclop // TODO: reduce complexity
102113
func (cs *Server) CreateVolume(
103114
ctx context.Context,
104115
req *csi.CreateVolumeRequest,
@@ -148,9 +159,33 @@ func (cs *Server) CreateVolume(
148159
defer cs.volumeLocks.Release(sourceVolumeID)
149160
}
150161

151-
// Step 1: Create RBD volume through backend. if exists, it is ok.
162+
// Step 1: Extract NVMe-oF mutable parameters before passing to RBD
163+
extractedMutableParams := make(map[string]string)
164+
if req.GetMutableParameters() != nil {
165+
// Extract NVMe-oF specific mutable parameters
166+
for _, key := range nvmeofMutableParams {
167+
if value, exists := req.GetMutableParameters()[key]; exists {
168+
extractedMutableParams[key] = value
169+
}
170+
}
171+
}
172+
173+
// Create a modified request without any mutable parameters for RBD
174+
// NVMe-oF handles its own mutable parameters separately
175+
rbdReq := &csi.CreateVolumeRequest{
176+
Name: req.GetName(),
177+
CapacityRange: req.GetCapacityRange(),
178+
VolumeCapabilities: req.GetVolumeCapabilities(),
179+
Parameters: req.GetParameters(),
180+
Secrets: req.GetSecrets(),
181+
VolumeContentSource: req.GetVolumeContentSource(),
182+
AccessibilityRequirements: req.GetAccessibilityRequirements(),
183+
// MutableParameters intentionally not set - NVMe-oF manages these separately
184+
}
185+
186+
// Step 2: Create RBD volume through backend. if exists, it is ok.
152187
// RBD backend automatically handles cloning when VolumeContentSource is present.
153-
res, err := cs.backendServer.CreateVolume(ctx, req)
188+
res, err := cs.backendServer.CreateVolume(ctx, rbdReq)
154189
if err != nil {
155190
log.ErrorLog(ctx, "failed to create RBD volume: %v", err)
156191

@@ -183,7 +218,7 @@ func (cs *Server) CreateVolume(
183218
// can be empty. if it was defined in config-map the rbd csi driver would have set it already
184219
rbdRadosNameSpace := res.GetVolume().GetVolumeContext()["radosNamespace"]
185220

186-
// Step 2: Setup NVMe-oF resources
221+
// Step 4: Setup NVMe-oF resources
187222
var nvmeofData *nvmeof.NVMeoFVolumeData
188223
// Defer: Cleanup NVMe-oF on any error (BEFORE the call!)
189224
defer func() {
@@ -204,13 +239,13 @@ func (cs *Server) CreateVolume(
204239

205240
return nil, status.Errorf(codes.Internal, "NVMe-oF setup failed: %v", err)
206241
}
207-
// step 3: Populate volume context for NodeServer
242+
// Step 5: Populate volume context for NodeServer
208243
err = populateVolumeContext(backend, nvmeofData)
209244
if err != nil {
210245
return nil, status.Errorf(codes.Internal, "failed to populate volume context: %v", err)
211246
}
212247

213-
// Step 4: Store NVMe-oF metadata in the volume context
248+
// Step 6: Store NVMe-oF metadata in the volume context
214249
err = cs.storeNVMeoFMetadata(ctx, req, volumeID, nvmeofData)
215250
if err != nil {
216251
return nil, err // Error already formatted with proper status code
@@ -372,28 +407,14 @@ func (cs *Server) ControllerModifyVolume(
372407
defer cs.volumeLocks.Release(volumeID)
373408

374409
// Step 2: Validate that only known parameters are provided
375-
knownParams := []string{
376-
nvmeof.RwIosPerSecond,
377-
nvmeof.RwMbytesPerSecond,
378-
nvmeof.RMbytesPerSecond,
379-
nvmeof.WMbytesPerSecond,
380-
nvmeof.AllowHostNQNs,
381-
}
382-
383410
for param := range params {
384-
if !slices.Contains(knownParams, param) {
411+
if !slices.Contains(nvmeofMutableParams, param) {
385412
return nil, status.Errorf(codes.InvalidArgument,
386413
"unknown mutable parameter: %s", param)
387414
}
388415
}
389416

390417
// Step 3: Parse QoS parameters from mutable_parameters
391-
hasRBDQoS := rbd.HasQoSParams(params)
392-
if hasRBDQoS {
393-
log.ErrorLog(ctx, "Cannot set RBD QoS parameters on NVMe-oF volumes")
394-
395-
return nil, status.Error(codes.InvalidArgument, "cannot set RBD QoS parameters on NVMe-oF volumes")
396-
}
397418
nvmeofQoS, err := nvmeof.NewNVMeoFQosVolumeFromParams(params)
398419
if err != nil {
399420
log.ErrorLog(ctx, "failed to parse NVMe-oF QoS parameters: %v", err)
@@ -509,27 +530,16 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
509530
mutableParams := req.GetMutableParameters()
510531

511532
// Validate that only known mutable parameters are provided
512-
knownMutableParams := []string{
513-
nvmeof.RwIosPerSecond,
514-
nvmeof.RwMbytesPerSecond,
515-
nvmeof.RMbytesPerSecond,
516-
nvmeof.WMbytesPerSecond,
517-
nvmeof.AllowHostNQNs,
518-
}
519-
520533
for param := range mutableParams {
521-
if !slices.Contains(knownMutableParams, param) {
534+
if !slices.Contains(nvmeofMutableParams, param) {
522535
return fmt.Errorf("unknown mutable parameter: %s", param)
523536
}
524537
}
525538

526-
// check for RBD QoS parameters in both params and mutableParams
539+
// Check for RBD QoS parameters in regular params (not mutableParams - already validated above)
527540
if hasRBDQoS := rbd.HasQoSParams(params); hasRBDQoS {
528541
return errors.New("setting RBD QoS parameters on NVMe-oF volumes is not supported")
529542
}
530-
if hasRBDQoS := rbd.HasQoSParams(mutableParams); hasRBDQoS {
531-
return errors.New("setting RBD QoS parameters on NVMe-oF volumes is not supported")
532-
}
533543

534544
// It take the mutableParams value from the volumeAttributesClassName in the PersistentVolumeClaim yaml.
535545
_, err = nvmeof.NewNVMeoFQosVolumeFromParams(mutableParams)

0 commit comments

Comments
 (0)