From edb2edc12c9745735ec057525ff81c9a98d0c792 Mon Sep 17 00:00:00 2001 From: henrygd Date: Wed, 21 Jan 2026 18:25:03 -0500 Subject: [PATCH] use name-only matching for unique SMART devices (#1655) Fall back to name-only matching (previous behavior) when a device name appears only once, preserving RAID composite key support added in #1655. --- agent/smart.go | 121 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 37 deletions(-) diff --git a/agent/smart.go b/agent/smart.go index a59f9d43..91bf7df2 100644 --- a/agent/smart.go +++ b/agent/smart.go @@ -54,6 +54,12 @@ type DeviceInfo struct { parserType string } +// deviceKey is a composite key for a device, used to identify a device uniquely. +type deviceKey struct { + name string + deviceType string +} + var errNoValidSmartData = fmt.Errorf("no valid SMART data found") // Error for missing data // Refresh updates SMART data for all known devices @@ -330,14 +336,11 @@ func normalizeParserType(value string) string { } } -// deviceKey creates a composite key from device name and type. +// makeDeviceKey creates a composite key from device name and type. // This allows multiple drives under the same device path (e.g., RAID controllers) // to be tracked separately. -func deviceKey(name, deviceType string) string { - if deviceType == "" { - return name - } - return name + "|" + deviceType +func makeDeviceKey(name, deviceType string) deviceKey { + return deviceKey{name: name, deviceType: deviceType} } // parseSmartOutput attempts each SMART parser, optionally detecting the type when @@ -583,6 +586,28 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo return existing } + // buildUniqueNameIndex returns devices that appear exactly once by name. + // It is used to safely apply name-only fallbacks without RAID ambiguity. + buildUniqueNameIndex := func(devices []*DeviceInfo) map[string]*DeviceInfo { + counts := make(map[string]int, len(devices)) + for _, dev := range devices { + if dev == nil || dev.Name == "" { + continue + } + counts[dev.Name]++ + } + unique := make(map[string]*DeviceInfo, len(counts)) + for _, dev := range devices { + if dev == nil || dev.Name == "" { + continue + } + if counts[dev.Name] == 1 { + unique[dev.Name] = dev + } + } + return unique + } + // preserveVerifiedType copies the verified type/parser metadata from an existing // device record so that subsequent scans/config updates never downgrade a // previously verified device. @@ -595,73 +620,89 @@ func mergeDeviceLists(existing, scanned, configured []*DeviceInfo) []*DeviceInfo target.parserType = prev.parserType } - existingIndex := make(map[string]*DeviceInfo, len(existing)) + // applyConfiguredMetadata updates a matched device with any configured + // overrides, preserving verified type data when present. + applyConfiguredMetadata := func(existingDev, configuredDev *DeviceInfo) { + // Only update the type if it has not been verified yet; otherwise we + // keep the existing verified metadata intact. + if configuredDev.Type != "" && !existingDev.typeVerified { + newType := strings.TrimSpace(configuredDev.Type) + existingDev.Type = newType + existingDev.typeVerified = false + existingDev.parserType = normalizeParserType(newType) + } + if configuredDev.InfoName != "" { + existingDev.InfoName = configuredDev.InfoName + } + if configuredDev.Protocol != "" { + existingDev.Protocol = configuredDev.Protocol + } + } + + existingIndex := make(map[deviceKey]*DeviceInfo, len(existing)) for _, dev := range existing { if dev == nil || dev.Name == "" { continue } - existingIndex[deviceKey(dev.Name, dev.Type)] = dev + existingIndex[makeDeviceKey(dev.Name, dev.Type)] = dev } + existingByName := buildUniqueNameIndex(existing) finalDevices := make([]*DeviceInfo, 0, len(scanned)+len(configured)) - deviceIndex := make(map[string]*DeviceInfo, len(scanned)+len(configured)) + deviceIndex := make(map[deviceKey]*DeviceInfo, len(scanned)+len(configured)) // Start with the newly scanned devices so we always surface fresh metadata, // but ensure we retain any previously verified parser assignment. - for _, dev := range scanned { - if dev == nil || dev.Name == "" { + for _, scannedDevice := range scanned { + if scannedDevice == nil || scannedDevice.Name == "" { continue } // Work on a copy so we can safely adjust metadata without mutating the // input slices that may be reused elsewhere. - copyDev := *dev - key := deviceKey(copyDev.Name, copyDev.Type) + copyDev := *scannedDevice + key := makeDeviceKey(copyDev.Name, copyDev.Type) if prev := existingIndex[key]; prev != nil { preserveVerifiedType(©Dev, prev) + } else if prev := existingByName[copyDev.Name]; prev != nil { + preserveVerifiedType(©Dev, prev) } finalDevices = append(finalDevices, ©Dev) - copyKey := deviceKey(copyDev.Name, copyDev.Type) + copyKey := makeDeviceKey(copyDev.Name, copyDev.Type) deviceIndex[copyKey] = finalDevices[len(finalDevices)-1] } + deviceIndexByName := buildUniqueNameIndex(finalDevices) // Merge configured devices on top so users can override scan results (except // for verified type information). - for _, dev := range configured { - if dev == nil || dev.Name == "" { + for _, configuredDevice := range configured { + if configuredDevice == nil || configuredDevice.Name == "" { continue } - key := deviceKey(dev.Name, dev.Type) + key := makeDeviceKey(configuredDevice.Name, configuredDevice.Type) if existingDev, ok := deviceIndex[key]; ok { - // Only update the type if it has not been verified yet; otherwise we - // keep the existing verified metadata intact. - if dev.Type != "" && !existingDev.typeVerified { - newType := strings.TrimSpace(dev.Type) - existingDev.Type = newType - existingDev.typeVerified = false - existingDev.parserType = normalizeParserType(newType) - } - if dev.InfoName != "" { - existingDev.InfoName = dev.InfoName - } - if dev.Protocol != "" { - existingDev.Protocol = dev.Protocol - } + applyConfiguredMetadata(existingDev, configuredDevice) + continue + } + if existingDev := deviceIndexByName[configuredDevice.Name]; existingDev != nil { + applyConfiguredMetadata(existingDev, configuredDevice) continue } - copyDev := *dev - key = deviceKey(copyDev.Name, copyDev.Type) + copyDev := *configuredDevice + key = makeDeviceKey(copyDev.Name, copyDev.Type) if prev := existingIndex[key]; prev != nil { preserveVerifiedType(©Dev, prev) + } else if prev := existingByName[copyDev.Name]; prev != nil { + preserveVerifiedType(©Dev, prev) } else if copyDev.Type != "" { copyDev.parserType = normalizeParserType(copyDev.Type) } finalDevices = append(finalDevices, ©Dev) - copyKey := deviceKey(copyDev.Name, copyDev.Type) + copyKey := makeDeviceKey(copyDev.Name, copyDev.Type) deviceIndex[copyKey] = finalDevices[len(finalDevices)-1] } @@ -680,12 +721,14 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) { return } - validKeys := make(map[string]struct{}, len(devices)) + validKeys := make(map[deviceKey]struct{}, len(devices)) + nameCounts := make(map[string]int, len(devices)) for _, device := range devices { if device == nil || device.Name == "" { continue } - validKeys[deviceKey(device.Name, device.Type)] = struct{}{} + validKeys[makeDeviceKey(device.Name, device.Type)] = struct{}{} + nameCounts[device.Name]++ } for key, data := range sm.SmartDataMap { @@ -694,7 +737,11 @@ func (sm *SmartManager) updateSmartDevices(devices []*DeviceInfo) { continue } - if _, ok := validKeys[deviceKey(data.DiskName, data.DiskType)]; ok { + if data.DiskType == "" { + if nameCounts[data.DiskName] == 1 { + continue + } + } else if _, ok := validKeys[makeDeviceKey(data.DiskName, data.DiskType)]; ok { continue }