Windows hardware detection - #22942
Conversation
There was a problem hiding this comment.
2 issues found across 16 files
Confidence score: 2/5
- In
src/libnetdata/os/windows-wmi/windows-wmi-GetSystemInfo.c, the helper can call cleanup on an uninitializedVARIANTwhen a WMIGetfails, which can crash the agent or corrupt memory on error paths; initializevtPropbeforeGet(or only clear it after successful initialization) before merging. - In
src/daemon/win_system-info.c,detect_virt()may treat an SMBIOS “unknown” sentinel as a final result and skip registry fallback, so some virtualized hosts can be misclassified; adjust the control flow so “unknown” still triggers registry-based detection before merge.
Architecture diagram
sequenceDiagram
participant Main as netdata_main
participant WinSys as win_system-info
participant WMI as WMI Provider
participant SMBIOS as DMI / SMBIOS
participant Reg as Windows Registry
participant SysInfo as System Info Store
participant Env as Environment Variables
participant UnitTest as Unit Test
Note over Main,Env: Windows Hardware Detection Probe Chain
Main->>WinSys: netdata_windows_get_system_info()
activate WinSys
WinSys->>WinSys: netdata_windows_detect_virtualization()
WinSys->>WinSys: netdata_windows_detect_virt()
alt WMI query succeeds
WinSys->>WMI: GetWin32ComputerSystemInfo()
WMI-->>WinSys: Model, Manufacturer
WinSys->>WinSys: netdata_windows_normalize_virt_string()
Note over WinSys: Returns canonical name (e.g., "vmware", "microsoft", "kvm"...)
else WMI fails
WinSys->>SMBIOS: dmi_is_virtual_machine() via os_dmi_info_get()
SMBIOS-->>WinSys: true/false + product name
WinSys->>WinSys: netdata_windows_normalize_virt_string()
else SMBIOS fails
WinSys->>Reg: netdata_registry_key_exists(HKLM\SOFTWARE\VMware..., Oracle..., Parallels...)
Reg-->>WinSys: true/false
else all probes fail
Note over WinSys: Fallback to "none" (bare metal)
end
WinSys->>SysInfo: rrdhost_system_info_set(NETDATA_SYSTEM_VIRTUALIZATION)
WinSys->>Env: nd_setenv("NETDATA_SYSTEM_VIRTUALIZATION", ...)
WinSys->>SysInfo: rrdhost_system_info_set(NETDATA_SYSTEM_VIRT_DETECTION, "windows-api")
WinSys->>Env: nd_setenv("NETDATA_SYSTEM_VIRT_DETECTION", "windows-api")
WinSys->>WinSys: netdata_windows_detect_container_state()
alt Kubernetes env vars present
WinSys->>WinSys: getenv(KUBERNETES_SERVICE_HOST) and getenv(KUBERNETES_SERVICE_PORT)
Note over WinSys: Detected as Kubernetes container
else WMI caption contains "container"
WinSys->>WMI: GetWin32OperatingSystemInfo()
WMI-->>WinSys: Caption
WinSys->>WinSys: netdata_windows_str_contains_ci(Caption, "container")
Note over WinSys: Detected as Windows Server container
else neither
Note over WinSys: No container environment detected
end
WinSys->>SysInfo: rrdhost_system_info_set(NETDATA_SYSTEM_CONTAINER, ...)
WinSys->>Env: nd_setenv("NETDATA_SYSTEM_CONTAINER", ...)
WinSys->>SysInfo: rrdhost_system_info_set(NETDATA_SYSTEM_CONTAINER_DETECTION, ...)
WinSys->>Env: nd_setenv("NETDATA_SYSTEM_CONTAINER_DETECTION", ...)
deactivate WinSys
Note over Main,Env: Unit Test (Windows only)
Main->>UnitTest: unit_test_windows_virt_normalize()
activate UnitTest
UnitTest->>WinSys: netdata_windows_normalize_virt_string() with test cases
WinSys-->>UnitTest: canonical string
UnitTest->>UnitTest: strcmp() against expected
alt all matched
UnitTest-->>Main: return 0 (OK)
else failures
UnitTest-->>Main: return 1 (ERROR)
end
deactivate UnitTest
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Thanks for fixing the original Windows VM detection bug. The direction looks right, and the earlier hardcoded Before merge, I think there are two issues worth tightening:
A good minimal fix would be: bound the WMI |
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
May need to consider WBEM_FLAG_CONNECT_USE_MAX_WAIT on ConnectServer() here: the new timeout only bounds Next(), but InitializeWMI()'s connect to winmgmt is unbounded, and since these probes now run on the daemon's synchronous startup path, a hung WMI would block agent startup indefinitely (the service already reports RUNNING, so it would look alive while serving nothing). The flag caps the connect at ~2 min — not the configured timeout, but it turns an indefinite hang into a bounded delay. Note both startup probes would retry the connect, so worst case is ~2×2 min. |
|
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Backport to 2.10.x. The feature files apply cleanly. During conflict resolution the unit-test wiring was trimmed to what exists on this branch: the OS_WINDOWS virt/container tests are kept, but the neighbouring ringbuffer/log_stack/clocks/ws_client/mqtt_ng unittest registrations (from netdata#22878 and later PRs, not present in 2.10.x) were excluded, along with their unit_test.c includes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Backport to 2.10.x. The feature files apply cleanly. During conflict resolution the unit-test wiring was trimmed to what exists on this branch: the OS_WINDOWS virt/container tests are kept, but the neighbouring ringbuffer/log_stack/clocks/ws_client/mqtt_ng unittest registrations (from #22878 and later PRs, not present in 2.10.x) were excluded, along with their unit_test.c includes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Fixed an issue reported by the customer in which VMs were detected as bare metal.
Test Plan
Additional Information
For users: How does this change affect me?
Summary by cubic
Fixes Windows virtualization and container detection so VMs aren’t mislabeled as bare metal and Kubernetes/Windows Server containers are correctly identified. Adds a WMI → SMBIOS → registry probe chain with normalized strings, bounded WMI timeouts, aligned host/container OS fields, exported detection env vars, and updated docs/API.
Bug Fixes
KUBERNETES_SERVICE_{HOST,PORT}; Windows Server containers viaHKLM\SYSTEM\CurrentControlSet\Control\ContainerType. Populate and exportNETDATA_SYSTEM_CONTAINERandNETDATA_SYSTEM_CONTAINER_DETECTION.NETDATA_OFFICIAL_IMAGEand exportNETDATA_CONTAINER_IS_OFFICIAL_IMAGE.RPC_E_TOO_LATE, per‑query timeout viaNETDATA_WMI_STARTUP_TIMEOUT_MS, and capConnectServerwithWBEM_FLAG_CONNECT_USE_MAX_WAIT(~2 minutes). Windows build fixes included.New Features
GetWin32ComputerSystemInfo()(Win32_ComputerSystem) with per‑query timeout, a registry key‑existence probe, and exposedmi_is_virtual_machine()for SMBIOS fallback. Add virtualization string normalizer, probe resolution logic, and wire Windows unit tests into the test runner.wmi startup timeoutinnetdata.conf(default 5000 ms, clamped 100–60000) to setNETDATA_WMI_STARTUP_TIMEOUT_MS. Docs clarify per‑query timeout vs. the separate ~2‑minute connect cap.NETDATA_SYSTEM_VIRTUALIZATION,NETDATA_SYSTEM_VIRT_DETECTION,NETDATA_SYSTEM_CONTAINER,NETDATA_SYSTEM_CONTAINER_DETECTION,NETDATA_SYSTEM_CPU_DETECTION,NETDATA_SYSTEM_RAM_DETECTION,NETDATA_SYSTEM_DISK_DETECTION,NETDATA_CONTAINER_IS_OFFICIAL_IMAGE.Written for commit 9b68e46. Summary will update on new commits.