From fcfa0c2903fb76c8dc8ea72bf2d645e238bbd1ab Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 27 Mar 2026 15:23:13 +0000 Subject: [PATCH] Skip malformed guest fsinfo entries (#1319) --- pkg/proxmox/client.go | 112 ++++++++++++++++----------- pkg/proxmox/client_api_more3_test.go | 46 +++++++++++ 2 files changed, 111 insertions(+), 47 deletions(-) diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 45e5ccc23..029f13464 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -1587,60 +1587,30 @@ func (c *Client) GetVMFSInfo(ctx context.Context, node string, vmid int) ([]VMFi Str("response", string(bodyBytes)). Msg("Raw response from guest agent get-fsinfo") - // Try to unmarshal as an array first (expected format) + // Decode array payloads entry-by-entry so one malformed filesystem record + // does not wipe valid guest-agent disk data for the whole VM. var arrayResult struct { Data struct { - Result []VMFileSystem `json:"result"` + Result []json.RawMessage `json:"result"` } `json:"data"` } if err := json.Unmarshal(bodyBytes, &arrayResult); err == nil && arrayResult.Data.Result != nil { - // Post-process to extract disk device names - for i := range arrayResult.Data.Result { - fs := &arrayResult.Data.Result[i] - // Extract disk device name from the DiskRaw field - if len(fs.DiskRaw) > 0 { - // The disk field usually contains device info as a map - if diskMap, ok := fs.DiskRaw[0].(map[string]interface{}); ok { - // Try to get the device name from various possible fields - if dev, ok := diskMap["dev"].(string); ok { - fs.Disk = dev - } else if serial, ok := diskMap["serial"].(string); ok { - fs.Disk = serial - } else if bus, ok := diskMap["bus-type"].(string); ok { - if target, ok := diskMap["target"].(float64); ok { - fs.Disk = fmt.Sprintf("%s-%d", bus, int(target)) - } - } - } - } - // If we still don't have a disk identifier, use the normalized mountpoint as a fallback - if fs.Disk == "" && fs.Mountpoint != "" { - // For root filesystem, use a special identifier - if fs.Mountpoint == "/" { - fs.Disk = "root-filesystem" - } else { - // For Windows, normalize drive letters to prevent duplicate counting - // Windows guest agent can return multiple directory entries (C:, C:\, C:\Users, C:\Windows) - // all on the same physical drive. Without disk[] metadata, we must deduplicate by drive letter. - isWindowsDrive := len(fs.Mountpoint) >= 2 && fs.Mountpoint[1] == ':' - if isWindowsDrive { - // Use drive letter as identifier (e.g., "C:" for C:\, C:\Users, etc.) - driveLetter := strings.ToUpper(fs.Mountpoint[:2]) - fs.Disk = driveLetter - log.Debug(). - Str("node", node). - Int("vmid", vmid). - Str("mountpoint", fs.Mountpoint). - Str("synthesized_disk", driveLetter). - Msg("Synthesized Windows drive identifier from mountpoint") - } else { - // Use mountpoint as unique identifier for non-Windows paths - fs.Disk = fs.Mountpoint - } - } + filesystems := make([]VMFileSystem, 0, len(arrayResult.Data.Result)) + for idx, rawFS := range arrayResult.Data.Result { + var fs VMFileSystem + if err := json.Unmarshal(rawFS, &fs); err != nil { + log.Warn(). + Err(err). + Str("node", node). + Int("vmid", vmid). + Int("filesystem_index", idx). + Msg("Skipping malformed guest agent filesystem entry") + continue } + filesystems = append(filesystems, fs) } - return arrayResult.Data.Result, nil + postProcessVMFilesystems(node, vmid, filesystems) + return filesystems, nil } // If that fails, try as an object (might be an error response or different format) @@ -1672,6 +1642,54 @@ func (c *Client) GetVMFSInfo(ctx context.Context, node string, vmid int) ([]VMFi return nil, fmt.Errorf("unexpected response format from guest agent get-fsinfo") } +func postProcessVMFilesystems(node string, vmid int, filesystems []VMFileSystem) { + for i := range filesystems { + fs := &filesystems[i] + // Extract disk device name from the DiskRaw field + if len(fs.DiskRaw) > 0 { + // The disk field usually contains device info as a map + if diskMap, ok := fs.DiskRaw[0].(map[string]interface{}); ok { + // Try to get the device name from various possible fields + if dev, ok := diskMap["dev"].(string); ok { + fs.Disk = dev + } else if serial, ok := diskMap["serial"].(string); ok { + fs.Disk = serial + } else if bus, ok := diskMap["bus-type"].(string); ok { + if target, ok := diskMap["target"].(float64); ok { + fs.Disk = fmt.Sprintf("%s-%d", bus, int(target)) + } + } + } + } + // If we still don't have a disk identifier, use the normalized mountpoint as a fallback + if fs.Disk == "" && fs.Mountpoint != "" { + // For root filesystem, use a special identifier + if fs.Mountpoint == "/" { + fs.Disk = "root-filesystem" + } else { + // For Windows, normalize drive letters to prevent duplicate counting + // Windows guest agent can return multiple directory entries (C:, C:\, C:\Users, C:\Windows) + // all on the same physical drive. Without disk[] metadata, we must deduplicate by drive letter. + isWindowsDrive := len(fs.Mountpoint) >= 2 && fs.Mountpoint[1] == ':' + if isWindowsDrive { + // Use drive letter as identifier (e.g., "C:" for C:\, C:\Users, etc.) + driveLetter := strings.ToUpper(fs.Mountpoint[:2]) + fs.Disk = driveLetter + log.Debug(). + Str("node", node). + Int("vmid", vmid). + Str("mountpoint", fs.Mountpoint). + Str("synthesized_disk", driveLetter). + Msg("Synthesized Windows drive identifier from mountpoint") + } else { + // Use mountpoint as unique identifier for non-Windows paths + fs.Disk = fs.Mountpoint + } + } + } + } +} + // GetVMNetworkInterfaces returns network interfaces reported by the guest agent func (c *Client) GetVMNetworkInterfaces(ctx context.Context, node string, vmid int) ([]VMNetworkInterface, error) { resp, err := c.get(ctx, fmt.Sprintf("/nodes/%s/qemu/%d/agent/network-get-interfaces", node, vmid)) diff --git a/pkg/proxmox/client_api_more3_test.go b/pkg/proxmox/client_api_more3_test.go index cda726808..d4a272069 100644 --- a/pkg/proxmox/client_api_more3_test.go +++ b/pkg/proxmox/client_api_more3_test.go @@ -193,6 +193,52 @@ func TestClientVMFSInfoObjectResult(t *testing.T) { } } +func TestClientVMFSInfoSkipsMalformedEntries(t *testing.T) { + client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api2/json/nodes/node1/qemu/100/agent/get-fsinfo": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "data": { + "result": [ + { + "name": "windows", + "type": "ntfs", + "mountpoint": "C:\\Windows", + "total-bytes": 200, + "used-bytes": 20 + }, + { + "name": "broken", + "type": "ntfs", + "mountpoint": "D:\\Data", + "total-bytes": true, + "used-bytes": 10 + } + ] + } + }`)) + default: + http.NotFound(w, r) + } + }) + + ctx := context.Background() + filesystems, err := client.GetVMFSInfo(ctx, "node1", 100) + if err != nil { + t.Fatalf("GetVMFSInfo error: %v", err) + } + if len(filesystems) != 1 { + t.Fatalf("expected 1 valid filesystem after skipping malformed entry, got %d", len(filesystems)) + } + if filesystems[0].Mountpoint != "C:\\Windows" { + t.Fatalf("expected valid filesystem to remain, got mountpoint %q", filesystems[0].Mountpoint) + } + if filesystems[0].Disk != "C:" { + t.Fatalf("expected windows drive disk, got %q", filesystems[0].Disk) + } +} + func TestClientContainerInterfacesError(t *testing.T) { client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path {