From 67c5905b3691b3dc5315fb37604cf7883cfe28fa Mon Sep 17 00:00:00 2001 From: spatiumstas <79056064+spatiumstas@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:09:32 +0300 Subject: [PATCH] fix(agent): handle partial NVMe SMART data (#2340) --- agent/smart.go | 34 +++++++++++++++++++++++--------- agent/smart_test.go | 21 ++++++++++++++++++++ internal/entities/smart/smart.go | 10 +++++----- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/agent/smart.go b/agent/smart.go index 28a5ad81e..3490e7fb0 100644 --- a/agent/smart.go +++ b/agent/smart.go @@ -311,11 +311,13 @@ func (sm *SmartManager) filterExcludedDevices(devices []*DeviceInfo) []*DeviceIn return filtered } -// detectSmartOutputType inspects sections that are unique to each smartctl -// JSON schema (NVMe, ATA/SATA, SCSI) to determine which parser should be used -// when the reported device type is ambiguous or missing. +// detectSmartOutputType inspects protocol-specific sections and the reported +// device type to choose a parser, including when the NVMe health log is missing. func detectSmartOutputType(output []byte) string { var hints struct { + Device struct { + Type string `json:"type"` + } `json:"device"` AtaSmartAttributes json.RawMessage `json:"ata_smart_attributes"` NVMeSmartHealthInformationLog json.RawMessage `json:"nvme_smart_health_information_log"` ScsiErrorCounterLog json.RawMessage `json:"scsi_error_counter_log"` @@ -326,7 +328,7 @@ func detectSmartOutputType(output []byte) string { } switch { - case hasJSONValue(hints.NVMeSmartHealthInformationLog): + case hasJSONValue(hints.NVMeSmartHealthInformationLog), normalizeParserType(hints.Device.Type) == "nvme": return "nvme" case hasJSONValue(hints.AtaSmartAttributes): return "sat" @@ -397,11 +399,11 @@ func (sm *SmartManager) parseSmartOutput(deviceInfo *DeviceInfo, output []byte) } } - // Only run the type detection when we do not yet know which parser works - // or the previous attempt failed. + // Inspect every response so a failed NVMe query cannot reach other parsers. + structureType := detectSmartOutputType(output) + // Update the stored parser only when it is not yet verified. needsDetection := deviceType == "" || !deviceInfo.typeVerified if needsDetection { - structureType := detectSmartOutputType(output) if deviceType != structureType { deviceType = structureType deviceInfo.parserType = structureType @@ -442,6 +444,11 @@ func (sm *SmartManager) parseSmartOutput(deviceInfo *DeviceInfo, output []byte) // Try the selected parsers in order until we find one that succeeds. for _, parser := range selectedParsers { + // A failed NVMe response may still contain a serial number, which is + // enough for the SATA and SCSI parsers to accept incorrect zero values. + if structureType == "nvme" && parser.Type != "nvme" { + continue + } hasData, _ := parser.Parse(output) if hasData { deviceInfo.parserType = parser.Type @@ -1145,6 +1152,16 @@ func (sm *SmartManager) parseSmartForNvme(output []byte, deviceType string) (boo return false, data.Smartctl.ExitStatus } + // smartctl may return device identity fields before failing to read the NVMe + // health log (for example, on an unsupported controller path or with insufficient + // permissions). Do not accept that partial response as valid SMART data: doing + // so stores incorrect zero values and prevents the namespace-path fallback. + log := data.NVMeSmartHealthInformationLog + if log == nil { + slog.Debug("no NVMe SMART health information", "device", data.Device.Name) + return false, data.Smartctl.ExitStatus + } + sm.Lock() defer sm.Unlock() @@ -1167,7 +1184,7 @@ func (sm *SmartManager) parseSmartForNvme(output []byte, deviceType string) (boo if smartData.Capacity == 0 && (runtime.GOOS == "darwin" || sm.darwinNvmeProvider != nil) { smartData.Capacity = sm.lookupDarwinNvmeCapacity(data.SerialNumber) } - smartData.Temperature = data.NVMeSmartHealthInformationLog.Temperature + smartData.Temperature = log.Temperature smartData.SmartStatus = getSmartStatus(smartData.Temperature, data.SmartStatus.Passed) smartData.DiskName = data.Device.Name smartData.DiskType = data.Device.Type @@ -1177,7 +1194,6 @@ func (sm *SmartManager) parseSmartForNvme(output []byte, deviceType string) (boo // nvme attributes does not follow the same format as ata attributes, // so we manually map each field to SmartAttributes - log := data.NVMeSmartHealthInformationLog smartData.Attributes = []*smart.SmartAttribute{ {Name: "CriticalWarning", RawValue: uint64(log.CriticalWarning)}, {Name: "Temperature", RawValue: uint64(log.Temperature)}, diff --git a/agent/smart_test.go b/agent/smart_test.go index f77d109a8..8b3071c8e 100644 --- a/agent/smart_test.go +++ b/agent/smart_test.go @@ -837,6 +837,27 @@ func TestParseSmartOutputMarksVerified(t *testing.T) { assert.True(t, device.typeVerified) } +func TestParseSmartForNvmeRejectsIdentityOnlyResponse(t *testing.T) { + jsonPayload := []byte(`{ + "smartctl": {"exit_status": 2}, + "device": {"name": "/dev/nvme0", "type": "nvme"}, + "model_name": "Netac NVMe SSD 500GB", + "serial_number": "IDENTITY-ONLY", + "user_capacity": {"bytes": 500107862016} + }`) + + sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} + hasData, exitStatus := sm.parseSmartForNvme(jsonPayload, "") + + assert.False(t, hasData) + assert.Equal(t, 2, exitStatus) + assert.NotContains(t, sm.SmartDataMap, "IDENTITY-ONLY") + + device := &DeviceInfo{Name: "/dev/nvme0", Type: "nvme"} + assert.False(t, sm.parseSmartOutput(device, jsonPayload)) + assert.NotContains(t, sm.SmartDataMap, "IDENTITY-ONLY") +} + func TestParseSmartOutputKeepsCustomType(t *testing.T) { fixturePath := filepath.Join("test-data", "smart", "sda.json") data, err := os.ReadFile(fixturePath) diff --git a/internal/entities/smart/smart.go b/internal/entities/smart/smart.go index 6d832e709..ef452a586 100644 --- a/internal/entities/smart/smart.go +++ b/internal/entities/smart/smart.go @@ -503,11 +503,11 @@ type SmartInfoForNvme struct { UserCapacity UserCapacity `json:"user_capacity"` // LogicalBlockSize int `json:"logical_block_size"` // LocalTime LocalTime `json:"local_time"` - SmartStatus SmartStatusInfoNvme `json:"smart_status"` - NVMeSmartHealthInformationLog NVMeSmartHealthInformationLog `json:"nvme_smart_health_information_log"` - Temperature TemperatureInfoNvme `json:"temperature"` - PowerCycleCount uint16 `json:"power_cycle_count"` - PowerOnTime PowerOnTimeInfoNvme `json:"power_on_time"` + SmartStatus SmartStatusInfoNvme `json:"smart_status"` + NVMeSmartHealthInformationLog *NVMeSmartHealthInformationLog `json:"nvme_smart_health_information_log"` + Temperature TemperatureInfoNvme `json:"temperature"` + PowerCycleCount uint16 `json:"power_cycle_count"` + PowerOnTime PowerOnTimeInfoNvme `json:"power_on_time"` } type TemperatureInfoNvme struct {