diff --git a/src/control/server/ctl_storage_rpc.go b/src/control/server/ctl_storage_rpc.go index 52a1f97cdb29..bee4531239f5 100644 --- a/src/control/server/ctl_storage_rpc.go +++ b/src/control/server/ctl_storage_rpc.go @@ -113,7 +113,7 @@ func bdevScanToProtoResp(scan scanBdevsFn, bdevCfgs storage.TierConfigs) (*ctlpb return nil, err } - if bdevCfgs.HaveBdevs() { + if bdevCfgs.HaveRealNVMe() { // Update proto Ctrlrs with role info for offline display. for _, c := range pbCtrlrs { pciAddrStr, err := ctrlrToPciStr(c) @@ -148,11 +148,11 @@ func bdevScanEngines(ctx context.Context, cs *ControlService, req *ctlpb.ScanNvm instances := cs.harness.Instances() resp := &ctlpb.ScanNvmeResp{} - for _, ei := range instances { + for _, engine := range instances { eReq := new(ctlpb.ScanNvmeReq) *eReq = *req if req.Meta { - ms, rs, err := computeMetaRdbSz(cs, ei, nsps) + ms, rs, err := computeMetaRdbSz(cs, engine, nsps) if err != nil { return nil, errors.Wrap(err, "computing meta and rdb size") } @@ -161,9 +161,9 @@ func bdevScanEngines(ctx context.Context, cs *ControlService, req *ctlpb.ScanNvm // If partial number of engines return results, indicate errors for non-ready // engines whilst returning successful scanmresults. - respEng, err := scanEngineBdevs(ctx, ei, eReq) + respEng, err := scanEngineBdevs(ctx, engine, eReq) if err != nil { - err = errors.Wrapf(err, "instance %d", ei.Index()) + err = errors.Wrapf(err, "instance %d", engine.Index()) if errLast == nil && len(instances) > 1 { errLast = err // Save err to preserve partial results. cs.log.Error(err.Error()) @@ -859,28 +859,48 @@ type formatNvmeReq struct { func formatNvme(ctx context.Context, req formatNvmeReq, resp *ctlpb.StorageFormatResp) { // Allow format to complete on one instance even if another fails - for idx, ei := range req.instances { + for idx, engine := range req.instances { _, hasError := req.errored[idx] _, skipped := req.skipped[idx] if hasError || (skipped && !req.mdFormatted) { - // if scm failed to format or was already formatted, indicate skipping bdev format - ret := ei.newCret(storage.NilBdevAddress, nil) - ret.State.Info = fmt.Sprintf(msgNvmeFormatSkip, ei.Index()) + // If scm failed to format or was already formatted, skip bdev format. + ret := engine.newCret(storage.NilBdevAddress, nil) + ret.State.Info = fmt.Sprintf(msgNvmeFormatSkip, engine.Index()) resp.Crets = append(resp.Crets, ret) continue } + respBdevs, err := scanEngineBdevs(ctx, engine, new(ctlpb.ScanNvmeReq)) + if err != nil { + if errors.Is(err, errEngineBdevScanEmptyDevList) { + // No controllers assigned in config, continue. + continue + } + req.errored[idx] = err.Error() + resp.Crets = append(resp.Crets, engine.newCret("", err)) + continue + } + + // Convert proto ctrlr scan results to native when calling into storage provider. + pbCtrlrs := proto.NvmeControllers(respBdevs.Ctrlrs) + ctrlrs, err := pbCtrlrs.ToNative() + if err != nil { + req.errored[idx] = err.Error() + resp.Crets = append(resp.Crets, engine.newCret("", err)) + continue + } + // SCM formatted correctly on this instance, format NVMe - cResults := ei.StorageFormatNVMe() + cResults := formatEngineBdevs(engine, ctrlrs) if cResults.HasErrors() { req.errored[idx] = cResults.Errors() resp.Crets = append(resp.Crets, cResults...) continue } - if err := ei.GetStorage().WriteNvmeConfig(ctx, req.log); err != nil { + if err := engine.GetStorage().WriteNvmeConfig(ctx, req.log, ctrlrs); err != nil { req.errored[idx] = err.Error() - cResults = append(cResults, ei.newCret("", err)) + cResults = append(cResults, engine.newCret("", err)) } resp.Crets = append(resp.Crets, cResults...) @@ -1024,8 +1044,9 @@ func (cs *ControlService) StorageNvmeAddDevice(ctx context.Context, req *ctlpb.N } cs.log.Debugf("updated bdev list: %+v", tierCfg.Bdev.DeviceList) + // TODO: Supply scan results for VMD backing device address mapping. resp = new(ctlpb.NvmeAddDeviceResp) - if err := engineStorage.WriteNvmeConfig(ctx, cs.log); err != nil { + if err := engineStorage.WriteNvmeConfig(ctx, cs.log, nil); err != nil { err = errors.Wrapf(err, "write nvme config for engine %d", engineIndex) cs.log.Error(err.Error()) diff --git a/src/control/server/ctl_storage_rpc_test.go b/src/control/server/ctl_storage_rpc_test.go index fb6c700b15e8..dda35321de13 100644 --- a/src/control/server/ctl_storage_rpc_test.go +++ b/src/control/server/ctl_storage_rpc_test.go @@ -168,8 +168,12 @@ func TestServer_bdevScan(t *testing.T) { { storage.NewTierConfig(). WithStorageClass(storage.ClassNvme.String()). - WithBdevDeviceList(test.MockPCIAddr(1), - test.MockPCIAddr(2)), + WithBdevDeviceList(test.MockPCIAddr(1)). + WithBdevDeviceRoles(storage.BdevRoleWAL), + storage.NewTierConfig(). + WithStorageClass(storage.ClassNvme.String()). + WithBdevDeviceList(test.MockPCIAddr(2)). + WithBdevDeviceRoles(storage.BdevRoleMeta | storage.BdevRoleData), }, }, provRes: &storage.BdevScanResponse{ @@ -184,14 +188,20 @@ func TestServer_bdevScan(t *testing.T) { func() *ctlpb.NvmeController { c := proto.MockNvmeController(1) c.SmdDevices = []*ctlpb.SmdDevice{ - {Rank: uint32(ranklist.NilRank)}, + { + Rank: uint32(ranklist.NilRank), + RoleBits: uint32(storage.BdevRoleWAL), + }, } return c }(), func() *ctlpb.NvmeController { c := proto.MockNvmeController(2) c.SmdDevices = []*ctlpb.SmdDevice{ - {Rank: uint32(ranklist.NilRank)}, + { + Rank: uint32(ranklist.NilRank), + RoleBits: uint32(storage.BdevRoleMeta | storage.BdevRoleData), + }, } return c }(), @@ -980,7 +990,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { bClass storage.Class bDevs [][]string bSize int - bmbc *bdev.MockBackendConfig + bmbcs []*bdev.MockBackendConfig awaitTimeout time.Duration getMemInfo func() (*common.MemInfo, error) expAwaitExit bool @@ -992,6 +1002,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sMounts: []string{"/mnt/daos"}, sClass: storage.ClassRam, sSize: 6, + bmbcs: []*bdev.MockBackendConfig{{}}, expResp: &ctlpb.StorageFormatResp{ Crets: []*ctlpb.NvmeControllerResult{}, Mrets: []*ctlpb.ScmMountResult{ @@ -1006,6 +1017,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sMounts: []string{"/mnt/daos"}, sClass: storage.ClassDcpm, sDevs: []string{"/dev/pmem1"}, + bmbcs: []*bdev.MockBackendConfig{{}}, expResp: &ctlpb.StorageFormatResp{ Crets: []*ctlpb.NvmeControllerResult{}, Mrets: []*ctlpb.ScmMountResult{ @@ -1023,14 +1035,16 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sSize: 6, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1058,13 +1072,15 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { bClass: storage.ClassFile, bDevs: [][]string{{"/tmp/daos-bdev"}}, bSize: 6, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - "/tmp/daos-bdev": new(storage.BdevDeviceFormatResponse), + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + "/tmp/daos-bdev": new(storage.BdevDeviceFormatResponse), + }, }, }, }, @@ -1089,14 +1105,16 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sDevs: []string{"dev/pmem0"}, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1124,9 +1142,11 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sSize: 6, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, }, }, expAwaitExit: true, @@ -1163,9 +1183,11 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sSize: 6, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, }, }, expResp: &ctlpb.StorageFormatResp{ @@ -1197,14 +1219,16 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sSize: 6, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1235,14 +1259,16 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sSize: 6, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1269,9 +1295,11 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sDevs: []string{"/dev/pmem1"}, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, }, }, expResp: &ctlpb.StorageFormatResp{ @@ -1303,14 +1331,16 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sDevs: []string{"/dev/pmem1"}, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, - }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1340,9 +1370,11 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sDevs: []string{"/dev/pmem1"}, bClass: storage.ClassNvme, bDevs: [][]string{{mockNvmeController0.PciAddr}}, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0}, + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, }, }, expAwaitExit: true, @@ -1365,14 +1397,29 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { {mockNvmeController0.PciAddr}, {mockNvmeController1.PciAddr}, }, - bmbc: &bdev.MockBackendConfig{ - ScanRes: &storage.BdevScanResponse{ - Controllers: storage.NvmeControllers{mockNvmeController0, mockNvmeController1}, + // One for each engine. + bmbcs: []*bdev.MockBackendConfig{ + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController0}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, + }, + }, }, - FormatRes: &storage.BdevFormatResponse{ - DeviceResponses: storage.BdevDeviceFormatResponses{ - mockNvmeController0.PciAddr: &storage.BdevDeviceFormatResponse{ - Formatted: true, + { + ScanRes: &storage.BdevScanResponse{ + Controllers: storage.NvmeControllers{mockNvmeController1}, + }, + FormatRes: &storage.BdevFormatResponse{ + DeviceResponses: storage.BdevDeviceFormatResponses{ + mockNvmeController1.PciAddr: &storage.BdevDeviceFormatResponse{ + Formatted: true, + }, }, }, }, @@ -1417,6 +1464,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { t.Fatal("expResp test case parameter required") } test.AssertEqual(t, len(tc.sMounts), len(tc.expResp.Mrets), name) + test.AssertEqual(t, len(tc.sMounts), len(tc.bmbcs), name) for i := range tc.sMounts { // Hack to deal with creating the mountpoint in test. // FIXME (DAOS-3471): The tests in this layer really shouldn't be @@ -1495,7 +1543,7 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { sysProv := system.NewMockSysProvider(log, smsc) mounter := mount.NewProvider(log, sysProv) scmProv := scm.NewProvider(log, nil, sysProv, mounter) - bdevProv := bdev.NewMockProvider(log, tc.bmbc) + bdevProv := bdev.NewMockProvider(log, nil) if tc.getMemInfo == nil { tc.getMemInfo = func() (*common.MemInfo, error) { return &common.MemInfo{ @@ -1530,10 +1578,12 @@ func TestServer_CtlSvc_StorageFormat(t *testing.T) { trc.Running.Store(tc.instancesStarted) runner := engine.NewTestRunner(trc, ec) - storProv := storage.MockProvider(log, 0, &ec.Storage, sysProv, - scmProv, bdevProv, nil) + // Engine specific bdev provider. + ebp := bdev.NewMockProvider(log, tc.bmbcs[i]) + esp := storage.MockProvider(log, 0, &ec.Storage, sysProv, + scmProv, ebp, nil) - ei := NewEngineInstance(log, storProv, nil, runner) + ei := NewEngineInstance(log, esp, nil, runner) ei.ready.Store(tc.instancesStarted) // if the instance is expected to have a valid superblock, create one diff --git a/src/control/server/harness.go b/src/control/server/harness.go index 70d0a8f296a1..ae90d3ed711d 100644 --- a/src/control/server/harness.go +++ b/src/control/server/harness.go @@ -15,7 +15,6 @@ import ( "github.com/pkg/errors" "google.golang.org/protobuf/proto" - commonpb "github.com/daos-stack/daos/src/control/common/proto" ctlpb "github.com/daos-stack/daos/src/control/common/proto/ctl" srvpb "github.com/daos-stack/daos/src/control/common/proto/srv" "github.com/daos-stack/daos/src/control/drpc" @@ -46,7 +45,6 @@ type Engine interface { // These methods should probably be refactored out into functions that // accept the engine instance as a parameter. StorageFormatSCM(context.Context, bool) *ctlpb.ScmMountResult - StorageFormatNVMe() commonpb.NvmeControllerResults // This is a more reasonable surface that will be easier to maintain and test. CallDrpc(context.Context, drpc.Method, proto.Message) (*drpc.Response, error) diff --git a/src/control/server/instance_storage_rpc.go b/src/control/server/instance_storage_rpc.go index 750ae8f0d8c6..ff0711ac43c2 100644 --- a/src/control/server/instance_storage_rpc.go +++ b/src/control/server/instance_storage_rpc.go @@ -22,8 +22,9 @@ import ( ) var ( - scanSmd = listSmdDevices - getCtrlrHealth = getBioHealth + scanSmd = listSmdDevices + getCtrlrHealth = getBioHealth + errEngineBdevScanEmptyDevList = errors.New("empty device list for engine instance") ) // newMntRet creates and populates SCM mount result. @@ -78,11 +79,28 @@ func (ei *EngineInstance) scmFormat(force bool) (*ctlpb.ScmMountResult, error) { return ei.newMntRet(cfg.Scm.MountPoint, nil), nil } -func (ei *EngineInstance) bdevFormat() (results proto.NvmeControllerResults) { +func formatEngineBdevs(engine Engine, ctrlrs storage.NvmeControllers) (results proto.NvmeControllerResults) { + ei := engine.(*EngineInstance) + + // If no superblock exists, format NVMe and populate response with results. + needsSuperblock, err := ei.NeedsSuperblock() + if err != nil { + ei.log.Errorf("engine storage for %s instance %d: NeedsSuperblock(): %s", + build.DataPlaneName, ei.Index(), err) + + return proto.NvmeControllerResults{ + ei.newCret("", err), + } + } + + if !needsSuperblock { + return + } + defer ei.logDuration(track(fmt.Sprintf( "Format of NVMe storage for %s instance %d", build.DataPlaneName, ei.Index()))) - for _, tr := range ei.storage.FormatBdevTiers() { + for _, tr := range ei.storage.FormatBdevTiers(ctrlrs) { // storage.NvmeControllers) { if tr.Error != nil { results = append(results, ei.newCret(fmt.Sprintf("tier %d", tr.Tier), tr.Error)) @@ -145,26 +163,6 @@ func (ei *EngineInstance) StorageFormatSCM(ctx context.Context, force bool) (mRe return } -// StorageFormatNVMe performs format on NVMe if superblock needs writing. -func (ei *EngineInstance) StorageFormatNVMe() (cResults proto.NvmeControllerResults) { - // If no superblock exists, format NVMe and populate response with results. - needsSuperblock, err := ei.NeedsSuperblock() - if err != nil { - ei.log.Errorf("engine storage for %s instance %d: NeedsSuperblock(): %s", - build.DataPlaneName, ei.Index(), err) - - return proto.NvmeControllerResults{ - ei.newCret("", err), - } - } - - if needsSuperblock { - cResults = ei.bdevFormat() - } - - return -} - func populateCtrlrHealth(ctx context.Context, engine Engine, req *ctlpb.BioHealthReq, ctrlr *ctlpb.NvmeController) (bool, error) { state := ctrlr.DevState if state != ctlpb.NvmeDevState_NORMAL && state != ctlpb.NvmeDevState_EVICTED { @@ -306,8 +304,7 @@ func bdevScanEngine(ctx context.Context, engine Engine, req *ctlpb.ScanNvmeReq) nrCfgBdevs := bdevCfgs.Bdevs().Len() if nrCfgBdevs == 0 { - return nil, errors.Errorf("empty device list for engine instance %d", - engine.Index()) + return nil, errEngineBdevScanEmptyDevList } var isStarted bool diff --git a/src/control/server/storage/bdev.go b/src/control/server/storage/bdev.go index 648290913657..b79c672eaac2 100644 --- a/src/control/server/storage/bdev.go +++ b/src/control/server/storage/bdev.go @@ -528,12 +528,12 @@ type ( // BdevFormatRequest defines the parameters for a Format operation. BdevFormatRequest struct { pbin.ForwardableRequest - Properties BdevTierProperties - OwnerUID int - OwnerGID int - VMDEnabled bool - Hostname string - BdevCache *BdevScanResponse + Properties BdevTierProperties + OwnerUID int + OwnerGID int + Hostname string + VMDEnabled bool + ScannedBdevs NvmeControllers // VMD needs address mapping for backing devices. } // BdevWriteConfigRequest defines the parameters for a WriteConfig operation. @@ -543,14 +543,14 @@ type ( OwnerUID int OwnerGID int TierProps []BdevTierProperties - VMDEnabled bool HotplugEnabled bool HotplugBusidBegin uint8 HotplugBusidEnd uint8 Hostname string - BdevCache *BdevScanResponse AccelProps AccelProps SpdkRpcSrvProps SpdkRpcServer + VMDEnabled bool + ScannedBdevs NvmeControllers // VMD needs address mapping for backing devices. } // BdevWriteConfigResponse contains the result of a WriteConfig operation. diff --git a/src/control/server/storage/bdev/backend.go b/src/control/server/storage/bdev/backend.go index 464bc680cb97..65fcc4d5408c 100644 --- a/src/control/server/storage/bdev/backend.go +++ b/src/control/server/storage/bdev/backend.go @@ -466,7 +466,7 @@ func (sb *spdkBackend) formatNvme(req *storage.BdevFormatRequest) (*storage.Bdev if req.VMDEnabled { sb.log.Debug("vmd support enabled during nvme format") - dl, err := substituteVMDAddresses(sb.log, needDevs, req.BdevCache) + dl, err := substituteVMDAddresses(sb.log, needDevs, req.ScannedBdevs) if err != nil { return nil, err } @@ -501,7 +501,6 @@ func (sb *spdkBackend) formatNvme(req *storage.BdevFormatRequest) (*storage.Bdev func (sb *spdkBackend) Format(req storage.BdevFormatRequest) (resp *storage.BdevFormatResponse, err error) { sb.log.Debugf("spdk backend format (bindings call): %+v", req) - // TODO (DAOS-3844): Kick off device formats in parallel? switch req.Properties.Class { case storage.ClassFile: return sb.formatAioFile(&req) @@ -529,7 +528,7 @@ func (sb *spdkBackend) writeNvmeConfig(req storage.BdevWriteConfigRequest, confW bdevs := &props.DeviceList.PCIAddressSet - dl, err := substituteVMDAddresses(sb.log, bdevs, req.BdevCache) + dl, err := substituteVMDAddresses(sb.log, bdevs, req.ScannedBdevs) if err != nil { return errors.Wrapf(err, "storage tier %d", props.Tier) } diff --git a/src/control/server/storage/bdev/backend_test.go b/src/control/server/storage/bdev/backend_test.go index 76780e6ea87c..7e9a5fe3201b 100644 --- a/src/control/server/storage/bdev/backend_test.go +++ b/src/control/server/storage/bdev/backend_test.go @@ -512,10 +512,8 @@ func TestBackend_Format(t *testing.T) { Class: storage.ClassNvme, DeviceList: storage.MustNewBdevDeviceList(vmdAddr), }, - VMDEnabled: true, - BdevCache: &storage.BdevScanResponse{ - Controllers: mockCtrlrsInclVMD(), - }, + VMDEnabled: true, + ScannedBdevs: mockCtrlrsInclVMD(), }, expResp: &storage.BdevFormatResponse{ DeviceResponses: map[string]*storage.BdevDeviceFormatResponse{ @@ -649,9 +647,7 @@ func TestBackend_writeNvmeConfig(t *testing.T) { DeviceList: storage.MustNewBdevDeviceList(vmdAddr), }, }, - BdevCache: &storage.BdevScanResponse{ - Controllers: mockCtrlrsInclVMD(), - }, + ScannedBdevs: mockCtrlrsInclVMD(), }, expCall: &storage.BdevWriteConfigRequest{ VMDEnabled: true, @@ -661,9 +657,7 @@ func TestBackend_writeNvmeConfig(t *testing.T) { DeviceList: storage.MustNewBdevDeviceList(vmdBackingAddr1, vmdBackingAddr2), }, }, - BdevCache: &storage.BdevScanResponse{ - Controllers: mockCtrlrsInclVMD(), - }, + ScannedBdevs: mockCtrlrsInclVMD(), }, }, } { diff --git a/src/control/server/storage/bdev/backend_vmd.go b/src/control/server/storage/bdev/backend_vmd.go index 20b5d1b07207..027bf725bb2a 100644 --- a/src/control/server/storage/bdev/backend_vmd.go +++ b/src/control/server/storage/bdev/backend_vmd.go @@ -118,19 +118,19 @@ func substVMDAddrs(inPCIAddrs *hardware.PCIAddressSet, foundCtrlrs storage.NvmeC // substituteVMDAddresses wraps around substVMDAddrs to substitute VMD addresses with the relevant // backing device addresses. // Function takes a BdevScanResponse reference to derive address map and a logger. -func substituteVMDAddresses(log logging.Logger, inPCIAddrs *hardware.PCIAddressSet, bdevCache *storage.BdevScanResponse) (*hardware.PCIAddressSet, error) { +func substituteVMDAddresses(log logging.Logger, inPCIAddrs *hardware.PCIAddressSet, ctrlrs storage.NvmeControllers) (*hardware.PCIAddressSet, error) { if inPCIAddrs == nil { return nil, errors.New("nil input PCIAddressSet") } - if bdevCache == nil || len(bdevCache.Controllers) == 0 { - log.Debugf("no bdev cache to find vmd backing devices (devs: %v)", inPCIAddrs) + if len(ctrlrs) == 0 { + log.Debugf("no bdev info to find vmd backing devices (devs: %v)", inPCIAddrs) return inPCIAddrs, nil } msg := fmt.Sprintf("vmd detected, processing addresses (input %v, existing %v)", - inPCIAddrs, bdevCache.Controllers) + inPCIAddrs, ctrlrs) - dl, err := substVMDAddrs(inPCIAddrs, bdevCache.Controllers) + dl, err := substVMDAddrs(inPCIAddrs, ctrlrs) if err != nil { return nil, errors.Wrapf(err, msg) } diff --git a/src/control/server/storage/bdev/backend_vmd_test.go b/src/control/server/storage/bdev/backend_vmd_test.go index 7038da2a4e5c..ab40829af881 100644 --- a/src/control/server/storage/bdev/backend_vmd_test.go +++ b/src/control/server/storage/bdev/backend_vmd_test.go @@ -28,46 +28,38 @@ const ( func TestBackend_substituteVMDAddresses(t *testing.T) { for name, tc := range map[string]struct { - inAddrs *hardware.PCIAddressSet - bdevCache *storage.BdevScanResponse - expOutAddrs *hardware.PCIAddressSet - expErr error + inAddrs *hardware.PCIAddressSet + scannedBdevs storage.NvmeControllers + expOutAddrs *hardware.PCIAddressSet + expErr error }{ "one vmd requested; no backing devices": { inAddrs: addrListFromStrings(vmdAddr), - bdevCache: &storage.BdevScanResponse{ - Controllers: ctrlrsFromPCIAddrs("850505:07:00.0", "850505:09:00.0", - "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", - "850505:11:00.0", "850505:14:00.0"), - }, + scannedBdevs: ctrlrsFromPCIAddrs("850505:07:00.0", "850505:09:00.0", + "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", + "850505:11:00.0", "850505:14:00.0"), expOutAddrs: addrListFromStrings(vmdAddr), }, "one vmd requested; two backing devices": { - inAddrs: addrListFromStrings(vmdAddr), - bdevCache: &storage.BdevScanResponse{ - Controllers: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2), - }, - expOutAddrs: addrListFromStrings(vmdBackingAddr1, vmdBackingAddr2), + inAddrs: addrListFromStrings(vmdAddr), + scannedBdevs: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2), + expOutAddrs: addrListFromStrings(vmdBackingAddr1, vmdBackingAddr2), }, "two vmds requested; one has backing devices": { inAddrs: addrListFromStrings(vmdAddr, "0000:85:05.5"), - bdevCache: &storage.BdevScanResponse{ - Controllers: ctrlrsFromPCIAddrs("850505:07:00.0", "850505:09:00.0", - "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", - "850505:11:00.0", "850505:14:00.0"), - }, + scannedBdevs: ctrlrsFromPCIAddrs("850505:07:00.0", "850505:09:00.0", + "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", + "850505:11:00.0", "850505:14:00.0"), expOutAddrs: addrListFromStrings(vmdAddr, "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", "850505:14:00.0"), }, "two vmds requested; both have backing devices": { inAddrs: addrListFromStrings(vmdAddr, "0000:85:05.5"), - bdevCache: &storage.BdevScanResponse{ - Controllers: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2, - "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", - "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", - "850505:14:00.0"), - }, + scannedBdevs: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2, + "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", + "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", + "850505:14:00.0"), expOutAddrs: addrListFromStrings(vmdBackingAddr1, vmdBackingAddr2, "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", @@ -75,12 +67,10 @@ func TestBackend_substituteVMDAddresses(t *testing.T) { }, "input vmd backing devices": { inAddrs: addrListFromStrings(vmdBackingAddr2, vmdBackingAddr1), - bdevCache: &storage.BdevScanResponse{ - Controllers: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2, - "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", - "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", - "850505:14:00.0"), - }, + scannedBdevs: ctrlrsFromPCIAddrs(vmdBackingAddr1, vmdBackingAddr2, + "850505:07:00.0", "850505:09:00.0", "850505:0b:00.0", + "850505:0d:00.0", "850505:0f:00.0", "850505:11:00.0", + "850505:14:00.0"), expOutAddrs: addrListFromStrings(vmdBackingAddr1, vmdBackingAddr2), }, "input vmd backing devices; no cache": { @@ -92,7 +82,7 @@ func TestBackend_substituteVMDAddresses(t *testing.T) { log, buf := logging.NewTestLogger(name) defer test.ShowBufferOnFailure(t, buf) - gotAddrs, gotErr := substituteVMDAddresses(log, tc.inAddrs, tc.bdevCache) + gotAddrs, gotErr := substituteVMDAddresses(log, tc.inAddrs, tc.scannedBdevs) test.CmpErr(t, tc.expErr, gotErr) if gotErr != nil { return diff --git a/src/control/server/storage/provider.go b/src/control/server/storage/provider.go index 90a243ecd55c..6f87ad9de50f 100644 --- a/src/control/server/storage/provider.go +++ b/src/control/server/storage/provider.go @@ -480,7 +480,7 @@ type BdevTierFormatResult struct { // FormatBdevTiers formats all the Bdev tiers in the engine storage // configuration. -func (p *Provider) FormatBdevTiers() (results []BdevTierFormatResult) { +func (p *Provider) FormatBdevTiers(ctrlrs NvmeControllers) (results []BdevTierFormatResult) { bdevCfgs := p.engineStorage.Tiers.BdevConfigs() results = make([]BdevTierFormatResult, len(bdevCfgs)) @@ -493,20 +493,21 @@ func (p *Provider) FormatBdevTiers() (results []BdevTierFormatResult) { p.log.Infof("Instance %d: starting format of %s block devices %v", p.engineIndex, cfg.Class, cfg.Bdev.DeviceList) - results[i].Tier = cfg.Tier - results[i].DeviceRoles = cfg.Bdev.DeviceRoles req, err := BdevFormatRequestFromConfig(p.log, cfg) if err != nil { results[i].Error = err p.log.Errorf("Instance %d: format failed (%s)", err) continue } + req.ScannedBdevs = ctrlrs p.RLock() req.VMDEnabled = p.vmdEnabled results[i].Result, results[i].Error = p.bdev.Format(req) p.RUnlock() + results[i].Tier = cfg.Tier + results[i].DeviceRoles = cfg.Bdev.DeviceRoles if err := results[i].Error; err != nil { p.log.Errorf("Instance %d: format failed (%s)", err) continue @@ -596,7 +597,7 @@ func BdevWriteConfigRequestFromConfig(ctx context.Context, log logging.Logger, c // WriteNvmeConfig creates an NVMe config file which describes what devices // should be used by a DAOS engine process. -func (p *Provider) WriteNvmeConfig(ctx context.Context, log logging.Logger) error { +func (p *Provider) WriteNvmeConfig(ctx context.Context, log logging.Logger, ctrlrs NvmeControllers) error { p.RLock() vmdEnabled := p.vmdEnabled engineIndex := p.engineIndex @@ -608,17 +609,11 @@ func (p *Provider) WriteNvmeConfig(ctx context.Context, log logging.Logger) erro if err != nil { return errors.Wrap(err, "creating write config request") } - if req == nil { - return errors.New("BdevWriteConfigRequestFromConfig returned nil request") - } + req.ScannedBdevs = ctrlrs log.Infof("Writing NVMe config file for engine instance %d to %q", engineIndex, req.ConfigOutputPath) - p.RLock() - defer p.RUnlock() - req.BdevCache = &p.bdevCache - _, err = p.bdev.WriteConfig(*req) return err