From 88776342e8c506d6489355085555faccac95128b Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 17 Feb 2026 10:56:30 +0100 Subject: [PATCH] BUGFIX: /dev/sdb not accessible inside container --- TASK.md | 370 ++++++++++++++++++++++++-------------------------------- 1 file changed, 158 insertions(+), 212 deletions(-) diff --git a/TASK.md b/TASK.md index df0d702..20360c4 100644 --- a/TASK.md +++ b/TASK.md @@ -1,252 +1,198 @@ -# BUGFIX: Storage Scan — System Disk Detection & FSType in Container +# BUGFIX: /dev/sdb not accessible inside container -**Affects:** v0.11.0, `internal/storage/scan_linux.go` -**Root cause:** Controller runs in a Docker container. Even with `--privileged`, `lsblk` reports mount points from the container's mount namespace (not host), and often can't probe filesystem types due to missing udev/blkid cache. +## Problem -## Bug 1: System disk (sda) shows as available +`FormatAndMount` fails with `stat /dev/sdb: no such file or directory` because block device nodes +don't exist inside the container's `/dev`. -### Current broken logic -```go -if part.MountPoint == "/" || part.MountPoint == "/boot" || part.MountPoint == "/boot/efi" { - isSystem = true -} +Even with `privileged: true`, Docker creates its own tmpfs at `/dev` with minimal device nodes. +The explicit `- /dev:/dev` volume mount in docker-compose.yml is silently overridden by Docker's +internal `/dev` tmpfs setup — `docker inspect` shows no bind mount for `/dev`. + +## Root Cause + +Docker always creates a fresh tmpfs for `/dev` inside containers. The `privileged: true` flag +relaxes cgroup device access (the kernel allows I/O to any device), but doesn't populate `/dev` +with all host device nodes. The bind mount `- /dev:/dev` conflicts with Docker's own `/dev` +management and gets silently dropped. + +## Fix + +Mount host `/dev` at a **different path** inside the container. The device nodes at `/host-dev/sdb` +are real block devices that the kernel will allow I/O to (because `privileged: true`). + +### 1. docker-compose.yml change + +```yaml +volumes: + # ...existing... + # Block devices — mounted at /host-dev (can't override Docker's /dev) + - /dev:/host-dev:rw ``` -Inside the container, sda2 (host's `/`) shows mounted at `/opt/docker/felhom-controller/data` (bind mount), not `/`. So `isSystem` stays false → sda appears in AvailableDisks. -### Fix: Parse host's fstab + blkid to detect system disk +Change `- /dev:/dev` to `- /dev:/host-dev:rw` -The host's fstab is mounted at `/host-fstab` inside the container. Parse it to find which devices/UUIDs are used for `/`, `/boot`, `/boot/efi`, and `swap`. Then resolve UUIDs to device paths via `blkid`, and mark their parent disks as system disks. +### 2. Go code: Add host device path constant + +In `internal/storage/` package (e.g., `format_linux.go` or a new `paths.go`): ```go -// getSystemDiskNames returns the set of parent disk names (e.g., "sda") -// that contain system partitions (/, /boot, /boot/efi, swap). -func getSystemDiskNames() map[string]bool { - systemDisks := map[string]bool{} +const ( + // HostDevPath is where the host's /dev is mounted inside the container. + // Docker overrides /dev with its own tmpfs, so we mount at /host-dev. + HostDevPath = "/host-dev" - // Step 1: Parse /host-fstab for system mount points - fstabPath := "/host-fstab" - if _, err := os.Stat(fstabPath); err != nil { - // Fallback: try /etc/fstab (if not containerized or different mount) - fstabPath = "/etc/fstab" - } - - data, err := os.ReadFile(fstabPath) - if err != nil { - return systemDisks // Can't read fstab, return empty (safe default: nothing excluded) - } - - // System mount points we care about - systemMounts := map[string]bool{"/": true, "/boot": true, "/boot/efi": true} - - var systemUUIDs []string - var systemDevices []string - - for _, line := range strings.Split(string(data), "\n") { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - fields := strings.Fields(line) - if len(fields) < 3 { - continue - } - source := fields[0] - mountPoint := fields[1] - fsType := fields[2] - - isSystemEntry := systemMounts[mountPoint] || fsType == "swap" - if !isSystemEntry { - continue - } - - if strings.HasPrefix(source, "UUID=") { - systemUUIDs = append(systemUUIDs, strings.TrimPrefix(source, "UUID=")) - } else if strings.HasPrefix(source, "/dev/") { - systemDevices = append(systemDevices, source) - } - } - - // Step 2: Resolve UUIDs to device paths via blkid - for _, uuid := range systemUUIDs { - out, err := exec.Command("blkid", "-U", uuid).Output() - if err == nil { - devPath := strings.TrimSpace(string(out)) // e.g., "/dev/sda2" - systemDevices = append(systemDevices, devPath) - } - } - - // Step 3: Extract parent disk names from device paths - for _, devPath := range systemDevices { - diskName := partitionToParentDisk(devPath) - if diskName != "" { - systemDisks[diskName] = true - } - } - - return systemDisks -} + // HostFstabPath is where the host's /etc/fstab is mounted. + HostFstabPath = "/host-fstab" +) -// partitionToParentDisk extracts parent disk name from a partition device path. -// "/dev/sda2" → "sda", "/dev/nvme0n1p2" → "nvme0n1" -func partitionToParentDisk(devPath string) string { - name := filepath.Base(devPath) // "sda2" - - // NVMe: nvme0n1p2 → nvme0n1 - if strings.Contains(name, "nvme") { - if idx := strings.LastIndex(name, "p"); idx > 0 { - candidate := name[:idx] - // Verify it's actually a partition number after 'p' - if _, err := strconv.Atoi(name[idx+1:]); err == nil { - return candidate - } - } - return name +// HostDevicePath converts a standard device path to the container-accessible path. +// "/dev/sdb" → "/host-dev/sdb" +// "/dev/sdb1" → "/host-dev/sdb1" +func HostDevicePath(devPath string) string { + if strings.HasPrefix(devPath, "/dev/") { + return HostDevPath + "/" + strings.TrimPrefix(devPath, "/dev/") } - - // Standard: sda2 → sda, sdb1 → sdb - return strings.TrimRight(name, "0123456789") + return devPath } ``` -Then in `ScanDisks()`, replace the mount-point-based detection: +### 3. Update all device operations to use HostDevicePath() + +In `format_linux.go` (or wherever FormatAndMount is): ```go -func ScanDisks() (*ScanResult, error) { - // ... lsblk parsing as before ... - - // Get system disk names from host fstab - systemDiskNames := getSystemDiskNames() - - for _, dev := range parsed.BlockDevices { - if dev.Type != "disk" { continue } - - // ... build BlockDevice as before ... - - // Check if this is a system disk (from fstab analysis) - isSystem := systemDiskNames[dev.Name] - - // Also check if any partition is currently mounted (fallback safety) - anyMounted := false - for _, child := range dev.Children { - // ... as before ... - if part.MountPoint != "" { - anyMounted = true - } - } - - bd.Mounted = anyMounted || isSystem - - if isSystem || anyMounted { - result.SystemDisks = append(result.SystemDisks, bd) - } else { - result.AvailableDisks = append(result.AvailableDisks, bd) - } - } - return result, nil +// Validation — check device exists +hostDev := HostDevicePath(req.DevicePath) // "/dev/sdb" → "/host-dev/sdb" +if _, err := os.Stat(hostDev); err != nil { + return fmt.Errorf("device not found: %s", req.DevicePath) } + +// Partition — use host device path +cmd := exec.Command("sfdisk", hostDev) + +// Format — use host device path +cmd := exec.Command("mkfs.ext4", "-F", "-L", label, HostDevicePath(partPath)) + +// blkid — use host device path +cmd := exec.Command("blkid", "-o", "value", "-s", "UUID", HostDevicePath(partPath)) + +// mount — use host device path for source, real path for target +cmd := exec.Command("mount", HostDevicePath(partPath), mountPath) ``` -## Bug 2: "nincs fájlrendszer" for all partitions +### 4. Update ScanDisks blkid enrichment -### Current broken logic -`lsblk` inside a container often returns `null` for `fstype` because it relies on udev/blkid cache that's incomplete in the container's environment. - -### Fix: Enrich with blkid - -After lsblk parsing, run `blkid` to get filesystem types for all partitions. `blkid` directly probes the device (works in privileged containers): +In `scan_linux.go`, the `enrichWithBlkid` function and `getSystemDiskNames` function +use `blkid` which scans `/dev` by default. Update to scan `/host-dev`: ```go -// enrichWithBlkid fills in missing FSType, UUID, and Label from blkid. +// enrichWithBlkid — run blkid on /host-dev to get filesystem info func enrichWithBlkid(disks []BlockDevice) { - // Run blkid once for all devices + // blkid by default scans /dev — we need it to scan /host-dev + // Option 1: Run blkid with explicit device paths from /host-dev + // Option 2: Run blkid -o export and it will find devices from /proc/partitions + + // blkid -o export still works because it reads /proc/partitions (kernel-level) + // and then probes the devices. With privileged mode, it can probe via /proc. + // BUT the DEVNAME in output will say /dev/sdb1, not /host-dev/sdb1. + // That's fine — we match by device name anyway. out, err := exec.Command("blkid", "-o", "export").Output() - if err != nil { - return // Best-effort; lsblk data still usable - } - - // Parse blkid output — blocks separated by blank lines: - // DEVNAME=/dev/sda1 - // UUID=XXXX-YYYY - // TYPE=vfat - // ... - blkidMap := parseBlkidExport(out) - + // ... parsing as before, matching by /dev/xxx paths ... +} + +// For getSystemDiskNames, blkid -U returns /dev/xxx paths which is correct +// for fstab parsing (fstab uses /dev/xxx paths too). +// No changes needed there — it's just resolving UUIDs to device names. +``` + +**Note:** `blkid -o export` may not find devices if it can only see Docker's minimal `/dev`. +In that case, enumerate `/host-dev/sd*` explicitly: + +```go +func enrichWithBlkid(disks []BlockDevice) { for i := range disks { for j := range disks[i].Partitions { p := &disks[i].Partitions[j] - if info, ok := blkidMap[p.Path]; ok { - if p.FSType == "" { - p.FSType = info.FSType - } - if p.UUID == "" { - p.UUID = info.UUID - } - if p.Label == "" { - p.Label = info.Label - } + hostPath := HostDevicePath(p.Path) // "/host-dev/sdb1" + + // Probe individually + if fstype, err := exec.Command("blkid", "-o", "value", "-s", "TYPE", hostPath).Output(); err == nil { + p.FSType = strings.TrimSpace(string(fstype)) + } + if uuid, err := exec.Command("blkid", "-o", "value", "-s", "UUID", hostPath).Output(); err == nil { + p.UUID = strings.TrimSpace(string(uuid)) + } + if label, err := exec.Command("blkid", "-o", "value", "-s", "LABEL", hostPath).Output(); err == nil { + p.Label = strings.TrimSpace(string(label)) } } } } - -type blkidInfo struct { - FSType string - UUID string - Label string -} - -func parseBlkidExport(data []byte) map[string]blkidInfo { - result := map[string]blkidInfo{} - - blocks := strings.Split(string(data), "\n\n") - for _, block := range blocks { - var devName string - info := blkidInfo{} - for _, line := range strings.Split(strings.TrimSpace(block), "\n") { - parts := strings.SplitN(line, "=", 2) - if len(parts) != 2 { continue } - key, val := parts[0], parts[1] - switch key { - case "DEVNAME": - devName = val - case "TYPE": - info.FSType = val - case "UUID": - info.UUID = val - case "LABEL": - info.Label = val - } - } - if devName != "" { - result[devName] = info - } - } - return result -} ``` -Call `enrichWithBlkid()` at the end of `ScanDisks()` on both `AvailableDisks` and `SystemDisks`. +### 5. fstab writing — use real /dev paths -## UI impact +When writing to fstab, use UUID-based entries (already the plan), so no /dev path needed: +``` +UUID= /mnt/hdd_1 ext4 defaults,nofail,noatime 0 2 +``` -After these fixes: -- sda will appear in `SystemDisks` (shown grayed out with "Rendszermeghajtó" label, or hidden entirely) -- sdb will be the only entry in `AvailableDisks` -- sda partitions will show: sda1 (vfat, /boot/efi), sda2 (ext4, /), sda3 (swap) -- sdb1 will correctly show "(nincs fájlrendszer)" since it genuinely has none +The UUID is obtained from `blkid` using the `/host-dev/sdb1` path, but the UUID itself +is filesystem-level and doesn't depend on device path. -## Template update +### 6. mount command — needs special handling -If SystemDisks are currently shown alongside AvailableDisks (both selectable), the template should either: -- **Option A:** Hide system disks entirely — simpler, less confusion -- **Option B:** Show them grayed out with a "Rendszermeghajtó — nem választható" badge +`mount /host-dev/sdb1 /mnt/hdd_1` should work because `/host-dev/sdb1` is a real block +device node (same major:minor as the host's `/dev/sdb1`). The kernel doesn't care about +the path — it uses the device numbers. -Recommended: **Option A** — only show AvailableDisks. The user doesn't need to see sda at all. +However, `mount` may also accept UUID directly: +```go +exec.Command("mount", "UUID="+uuid, mountPath) +``` +This is even better — no device path needed at all. But it requires the kernel to find +the device, which should work since the device is visible in `/proc/partitions`. -## Files modified -- `controller/internal/storage/scan_linux.go` — `getSystemDiskNames()`, `partitionToParentDisk()`, `enrichWithBlkid()`, `parseBlkidExport()`, updated `ScanDisks()` +**Recommended:** Use the device path approach (`mount /host-dev/sdb1 /mnt/hdd_1`) as +it's more explicit and debuggable. -## Testing -1. After fix: scan page shows only sdb (931.5 GB, HD710 PRO, sdb1 no filesystem) -2. sda is no longer listed -3. If you temporarily disconnect the USB HDD: scan shows "Nem található inicializálható meghajtó" \ No newline at end of file +### 7. Also update docker-setup.sh + +If `docker-setup.sh` generates the controller compose file, update the `/dev` mount: + +```bash +# Was: +echo " - /dev:/dev" >> "$compose_file" +# Now: +echo " - /dev:/host-dev:rw" >> "$compose_file" +``` + +Check in `docker-setup.sh` whether all necessary packages are deployed during installation (rsync, etc.) + +### 8. Update documentation + +Update CONTEXT.md, CHANGELOG.md, README.md + +## Summary of changes + +| File | Change | +|------|--------| +| `controller/docker-compose.yml` | `/dev:/dev` → `/dev:/host-dev:rw` | +| `controller/internal/storage/format_linux.go` | Use `HostDevicePath()` for all device operations | +| `controller/internal/storage/scan_linux.go` | Use `HostDevicePath()` for `blkid` probing | +| `controller/internal/storage/paths.go` (NEW) | `HostDevPath`, `HostFstabPath`, `HostDevicePath()` | +| `scripts/docker-setup.sh` | Update compose generation | + +## Quick test after fix + +```bash +# Rebuild + redeploy controller +# Then verify: +docker exec felhom-controller ls -la /host-dev/sd* +# Should show: /host-dev/sda, /host-dev/sda1, sda2, sda3, /host-dev/sdb, /host-dev/sdb1 + +# Try format (from UI or manually): +docker exec felhom-controller blkid /host-dev/sdb1 +# Should work (empty output = no filesystem, which is correct for unformatted) +``` \ No newline at end of file