From d708def38f5552534791c990cb0ba6db637c661e Mon Sep 17 00:00:00 2001 From: yi111 <153097222+Yi-111-a@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:09:11 +0800 Subject: [PATCH] fix(agent): keep Windows volume names intact when matching I/O devices (#2419) filepath.Base is platform-dependent for a bare volume name: on Windows it strips the "C:" specifier and returns "\", so every drive letter normalizes to the same key. findIoDevice's exact match then returns whichever disk.IOCounters entry the map yielded first, which registers the root filesystem under a random drive. With EXTRA_FILESYSTEMS=D:,P: the root disk disappears from the hub, and without it the root can be registered under another drive's I/O device. Normalize a bare volume specifier before taking a path base, and stop taking a path base of the device in addPartitionRootFs, where it mangled the value before findIoDevice could see it. registerFilesystemStats already avoided this by only taking a base when the agent is not on Windows. Fixes #2417 --- agent/disk.go | 39 +++++++++++++++++++++++++-- agent/disk_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/agent/disk.go b/agent/disk.go index 13bb1b1ce..8574beff0 100644 --- a/agent/disk.go +++ b/agent/disk.go @@ -204,7 +204,10 @@ func isRootFallbackPartition(p disk.PartitionStat, rootMountPoint string) bool { // partition looks like the active root mount but still needs translating to an // I/O device key. func (d *diskDiscovery) addPartitionRootFs(device, mountpoint string) bool { - fs, match := findIoDevice(filepath.Base(device), d.ctx.diskIoCounters) + // device is passed through as-is: findIoDevice normalizes it, and + // filepath.Base would turn a Windows volume name such as "C:" into "\" + // on the way in (#2417). + fs, match := findIoDevice(device, d.ctx.diskIoCounters) if !match { return false } @@ -527,13 +530,45 @@ func filesystemMatchesPartitionSetting(filesystem string, p disk.PartitionStat) // normalizeDeviceName canonicalizes device strings for comparisons. func normalizeDeviceName(value string) string { - name := filepath.Base(strings.TrimSpace(value)) + name := strings.TrimSpace(value) + if volume, ok := windowsVolumeName(name); ok { + return volume + } + name = filepath.Base(name) if name == "." { return "" } return name } +// windowsVolumeName returns the canonical form of a bare Windows volume +// specifier, so that "C:", `C:\` and "C:/" all name the same drive. +// +// filepath.Base cannot do this. On Windows it treats "C:" as a volume name +// with no path element to take the base of and returns "\", so every drive +// letter normalizes to the same key. findIoDevice then returns whichever +// counter the map happened to yield first, which registers the root +// filesystem under a random drive (#2417). +func windowsVolumeName(value string) (string, bool) { + if len(value) < 2 || value[1] != ':' { + return "", false + } + if c := value[0]; !('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') { + return "", false + } + if len(value) == 2 { + return value, true + } + // Only separators may follow the specifier. "C:data" is a drive-relative + // path, not a volume. + for i := 2; i < len(value); i++ { + if value[i] != '\\' && value[i] != '/' { + return "", false + } + } + return value[:2], true +} + // Sets start values for disk I/O stats. func (a *Agent) initializeDiskIoStats(diskIoCounters map[string]disk.IOCountersStat) { a.fsNames = a.fsNames[:0] diff --git a/agent/disk_test.go b/agent/disk_test.go index 70fa2083b..59f423bfd 100644 --- a/agent/disk_test.go +++ b/agent/disk_test.go @@ -1057,3 +1057,70 @@ func TestIoTimeDelta(t *testing.T) { assert.Equal(t, uint64(0), ioTimeDelta(200, math.MaxUint32+1000)) } + +func TestNormalizeDeviceName(t *testing.T) { + // A Windows volume name is not a path element, so every spelling of the + // same drive has to normalize to the same key. filepath.Base cannot do + // this: on Windows it strips the "C:" specifier and returns "\", which + // collapses every drive letter onto one key (#2417). + for _, spelling := range []string{"C:", `C:\`, "C:/", `C:\\`} { + assert.Equal(t, "C:", normalizeDeviceName(spelling), "spelling %q", spelling) + } + // Case is left to the caller, as it already is for Linux device names. + assert.Equal(t, "d:", normalizeDeviceName("d:")) + assert.Equal(t, "c:", normalizeDeviceName(" c: ")) + + // Non-volume inputs keep using filepath.Base. + assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1")) + assert.Equal(t, "sda1", normalizeDeviceName("/dev/sda1/")) + assert.Equal(t, "nvme0n1p2", normalizeDeviceName(" /dev/nvme0n1p2 ")) + assert.Equal(t, "", normalizeDeviceName(".")) + assert.Equal(t, "", normalizeDeviceName(" ")) + + // A drive-relative path is a path, not a volume. + assert.Equal(t, `C:data`, normalizeDeviceName(`C:data`)) +} + +func TestFindIoDeviceWindowsVolumeNames(t *testing.T) { + // Every drive normalizes to a distinct key, so the root drive resolves + // exactly instead of to whichever counter the map yielded first (#2417). + ioCounters := map[string]disk.IOCountersStat{ + "C:": {Name: "C:", ReadBytes: 10, WriteBytes: 10}, + "D:": {Name: "D:", ReadBytes: 20, WriteBytes: 20}, + "P:": {Name: "P:", ReadBytes: 30, WriteBytes: 30}, + } + + for i := 0; i < 32; i++ { + device, ok := findIoDevice("C:", ioCounters) + assert.True(t, ok) + assert.Equal(t, "C:", device) + } + + // The drive may arrive with a trailing separator, as a mount point does. + device, ok := findIoDevice(`C:\`, ioCounters) + assert.True(t, ok) + assert.Equal(t, "C:", device) +} + +func TestAddPartitionRootFsWindowsDrive(t *testing.T) { + agent := &Agent{fsStats: make(map[string]*system.FsStats)} + discovery := diskDiscovery{ + agent: agent, + ctx: fsRegistrationContext{ + isWindows: true, + diskIoCounters: map[string]disk.IOCountersStat{ + "C:": {Name: "C:"}, + "D:": {Name: "D:"}, + "P:": {Name: "P:"}, + }, + }, + } + + ok := discovery.addPartitionRootFs("C:", `C:\`) + + assert.True(t, ok) + assert.Len(t, agent.fsStats, 1) + stats, exists := agent.fsStats["C:"] + assert.True(t, exists) + assert.True(t, stats.Root) +}