From 2da4c38773fe0f4974f9a193f2ec1ebd26084db8 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 15 Jun 2026 20:24:42 +0200 Subject: [PATCH] agent v0.36.5: AttachDrive normalizes to exactly one bind (converges stacked binds) countHostMounts + normalize: no-op only when exactly one bind is guest-visible; else strip all binds and lay one fresh. Converges a stacked double-bind to one (the old umount-one+mount-one never did). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 ++++ cmd/felhom-agent/main.go | 2 +- internal/localapi/intermediary.go | 58 +++++++++++++++++++++---------- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527de4c..ab3131a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.36.5 — AttachDrive normalizes to exactly one bind (2026-06-15) + +AttachDrive now COUNTS the binds at a stable path (countHostMounts) and normalizes to exactly one: it is +a no-op only when there is exactly ONE bind the guest sees; otherwise it strips ALL existing binds +(bounded loop) and lays down one fresh bind. This converges a stacked double-bind to one — the old +umount-one+mount-one force-rebind never did. Caught when a double-bind survived a guest reboot. + ## v0.36.4 — serialize AttachDrive/DetachDrive (no double-bind race) (2026-06-15) A mutex on GuestBinder serializes AttachDrive/DetachDrive so a controller-triggered reconnect and the diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index ad304c5..234ff6d 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -44,7 +44,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.36.4" +var version = "0.36.5" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots diff --git a/internal/localapi/intermediary.go b/internal/localapi/intermediary.go index a5af2b8..3a9fd56 100644 --- a/internal/localapi/intermediary.go +++ b/internal/localapi/intermediary.go @@ -169,29 +169,49 @@ func (b *GuestBinder) AttachDrive(ctx context.Context, vmid int, where string) ( if err := b.run(ctx, "mkdir", "-p", stable); err != nil { return "", fmt.Errorf("guest-attach: stable dir %s: %w", stable, err) } - hostHas := isHostMountpoint(stable) - guestSees := b.GuestSeesMount(ctx, vmid, stable) - switch { - case hostHas && guestSees: - return stable, nil // fully live — no-op - case hostHas && !guestSees: - // Guest rebooted (or bound the parent before this drive's bind existed): re-fire propagation. - if err := b.run(ctx, "umount", stable); err != nil { - b.logger.Warn("guest-attach: re-bind umount failed (continuing to re-mount)", "stable", stable, "err", err) - } - if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { - return "", fmt.Errorf("guest-attach: re-bind %s -> %s: %w", src, stable, err) - } - b.logger.Info("guest-attach: re-bound drive to re-propagate into guest (post-reboot)", "vmid", vmid, "where", where, "stable", stable) - default: // !hostHas - if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { - return "", fmt.Errorf("guest-attach: bind %s -> %s: %w", src, stable, err) - } - b.logger.Info("guest-attach: drive bound under shared parent (live, no reboot)", "vmid", vmid, "where", where, "stable", stable) + // NORMALIZE to EXACTLY ONE bind. The target is usable only when there is exactly one bind AND the + // guest sees it; in that case this is a no-op. Otherwise (zero binds, a post-reboot bind the guest + // can't see, OR stacked duplicate binds from an earlier race/operator action) we strip ALL existing + // binds (bounded loop) and lay down exactly one fresh bind — which also re-fires propagation into the + // current guest namespace. Counting (countHostMounts) rather than a boolean isHostMountpoint is what + // makes this converge a double-bind to one (the old umount-one+mount-one never did). + n := countHostMounts(stable) + if n == 1 && b.GuestSeesMount(ctx, vmid, stable) { + return stable, nil // exactly one bind + guest-visible → fully live, no-op } + for i := 0; i < 16 && countHostMounts(stable) > 0; i++ { + if err := b.run(ctx, "umount", stable); err != nil { + b.logger.Warn("guest-attach: normalize umount failed (continuing)", "stable", stable, "err", err) + break + } + } + if err := b.run(ctx, "mount", "--bind", src, stable); err != nil { + return "", fmt.Errorf("guest-attach: bind %s -> %s: %w", src, stable, err) + } + b.logger.Info("guest-attach: drive bound under shared parent (normalized to one bind, live)", "vmid", vmid, "where", where, "stable", stable, "prior_binds", n) return stable, nil } +// countHostMounts returns how many times `path` appears as a mount target in /proc/self/mountinfo (i.e. +// how many stacked binds are at it). 0 = not mounted; >1 = stacked duplicates. Used to normalize to one. +func countHostMounts(path string) int { + f, err := os.Open("/proc/self/mountinfo") + if err != nil { + return 0 + } + defer f.Close() + n := 0 + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for sc.Scan() { + fields := strings.Fields(sc.Text()) + if len(fields) >= 5 && fields[4] == path { + n++ + } + } + return n +} + // GuestSeesMount reports whether vmid's guest currently has `path` as a mount target in ITS mount // namespace (read from /proc//mountinfo). This is the GUEST-side truth the host-side // isHostMountpoint can't see — the signal that distinguishes "bound on the host" from "live in the