GL-5: DR restore passes the FULL archive layout (live finding #2)
The live scratch DR exposed the second half of PVE's all-or-nothing explicit-params restore: mountpoints NOT named in the params are silently DROPPED - the DR guest came up without its mp0/mp1 data volumes (boot passed; the customer's world did not ride along). drRestoreOverrides now derives the COMPLETE param set from the archive's extracted config: explicit rootfs, every storage-backed mpN passed through (size + in-guest path + backup flag preserved so vzrestore extracts its content), the two structural binds replaced by 4d-swapped throwaways; unknown bind mpN or unparseable size refuses loudly. Snapshot sections never shadow the current config. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
@@ -82,20 +82,66 @@ func structuralBootHostDir(stateDir string, vmid int) string {
|
||||
return strings.TrimRight(stateDir, "/") + "/guests/" + strconv.Itoa(vmid) + "/bootstrap"
|
||||
}
|
||||
|
||||
// archiveRootfsSizeGB parses the current-config "rootfs:" line out of an extracted archive config
|
||||
// (raw pct-conf text) and returns its size in whole GB — 0 if absent/unparseable. Snapshot sections
|
||||
// ("[name]") follow the current config; parsing stops at the first one so a snapshot's rootfs can
|
||||
// never shadow the live value.
|
||||
func archiveRootfsSizeGB(raw string) int {
|
||||
// archiveCurrentConfig parses the CURRENT-config key/value lines out of an extracted archive
|
||||
// config (raw pct-conf text). Snapshot sections ("[name]") follow the current config; parsing
|
||||
// stops at the first one so a snapshot's values can never shadow the live ones.
|
||||
func archiveCurrentConfig(raw string) map[string]string {
|
||||
out := map[string]string{}
|
||||
for _, line := range strings.Split(raw, "\n") {
|
||||
if strings.HasPrefix(line, "[") {
|
||||
break
|
||||
}
|
||||
if v, ok := strings.CutPrefix(line, "rootfs:"); ok {
|
||||
return rootfsSizeGB(strings.TrimSpace(v))
|
||||
k, v, ok := strings.Cut(line, ":")
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
out[k] = strings.TrimSpace(v)
|
||||
}
|
||||
return 0
|
||||
return out
|
||||
}
|
||||
|
||||
// drRestoreOverrides builds the COMPLETE restore param set a DR bring-up must pass, from the
|
||||
// archive's extracted config (GL-5; both PVE constraints discovered live — see the call site):
|
||||
// explicit rootfs (sized from the archive), every storage-backed mpN passed through (recreated at
|
||||
// its archived size + in-guest path + backup flag so vzrestore extracts its content), and the two
|
||||
// structural bind slots replaced with throwaway volumes for 4d to swap. Unknown bind mpN or an
|
||||
// unparseable size → error (never silently restore a guest missing a mount).
|
||||
func drRestoreOverrides(rawArchiveCfg, restoreStorage string) (map[string]string, error) {
|
||||
cfg := archiveCurrentConfig(rawArchiveCfg)
|
||||
sz := rootfsSizeGB(cfg["rootfs"])
|
||||
if sz <= 0 {
|
||||
return nil, fmt.Errorf("archive config carries no parseable rootfs size — refusing a mount-override restore without an explicit rootfs")
|
||||
}
|
||||
out := map[string]string{"rootfs": fmt.Sprintf("%s:%d", restoreStorage, sz)}
|
||||
for key, val := range cfg {
|
||||
if len(key) <= 2 || key[:2] != "mp" || key[2] < '0' || key[2] > '9' {
|
||||
continue
|
||||
}
|
||||
volPart, rest, _ := strings.Cut(val, ",")
|
||||
if strings.HasPrefix(volPart, "/") {
|
||||
// a host bind — must be one of the two structural slots (replaced below)
|
||||
if key != structuralParentSlot && key != structuralBootSlot {
|
||||
return nil, fmt.Errorf("archive carries an unknown bind mountpoint %s=%q — refusing (only the structural %s/%s binds are known)", key, val, structuralParentSlot, structuralBootSlot)
|
||||
}
|
||||
continue
|
||||
}
|
||||
msz := rootfsSizeGB(val)
|
||||
if msz <= 0 {
|
||||
return nil, fmt.Errorf("archive mountpoint %s=%q carries no parseable size — cannot pass it through the explicit-params restore", key, val)
|
||||
}
|
||||
mp := mountPathOf(rest)
|
||||
if mp == "" {
|
||||
return nil, fmt.Errorf("archive mountpoint %s=%q carries no mp= path", key, val)
|
||||
}
|
||||
v := fmt.Sprintf("%s:%d,mp=%s", restoreStorage, msz, mp)
|
||||
if strings.Contains(","+rest+",", ",backup=1,") {
|
||||
v += ",backup=1"
|
||||
}
|
||||
out[key] = v
|
||||
}
|
||||
out[structuralParentSlot] = throwawayVolumeOverride(restoreStorage, structuralParentDir)
|
||||
out[structuralBootSlot] = throwawayVolumeOverride(restoreStorage, structuralBootGuestPath)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// configLockMaxAttempts bounds the F4 config-lock retry. configLockBackoff is a package var so
|
||||
@@ -281,26 +327,26 @@ func (e *Engine) runBringUp(ctx context.Context, spec BringUpSpec, res *BringUpR
|
||||
// back-half adds them post-bring-up) — that asymmetry is the whole GL-5 bug.
|
||||
var overrides map[string]string
|
||||
if spec.Mode == ModeDRGuestLoss {
|
||||
// PVE refuses a restore that carries mountpoint params unless `rootfs` is ALSO explicit
|
||||
// ("mount points configured, but 'rootfs' not set" — the same all-or-nothing constraint
|
||||
// restoretest.go:211 documents for the live-config path; hit live in the GL-5 validation).
|
||||
// The lost guest has no live config, so the rootfs SIZE comes from the archive's own
|
||||
// embedded config — used for the SIZE ONLY; the bind LAYOUT stays the platform constants.
|
||||
// PVE's explicit-params restore is ALL-OR-NOTHING (both halves hit live in the GL-5
|
||||
// validation): (a) any mpN param without an explicit `rootfs` → 500 "mount points
|
||||
// configured, but 'rootfs' not set" (the constraint restoretest.go:211 documents);
|
||||
// (b) mountpoints NOT named in the params are silently DROPPED from the restore — a DR
|
||||
// guest restored with only the bind overrides came up WITHOUT its mp0/mp1 data volumes.
|
||||
// So the FULL layout must be specified, from the archive's own embedded config: rootfs
|
||||
// sized from it, every storage-backed mpN passed through (size+path+backup preserved →
|
||||
// vzrestore extracts its content), and the two structural binds replaced by throwaways
|
||||
// that 4d swaps for the real binds. A bind mpN outside the two structural slots means an
|
||||
// unknown topology — refuse loudly rather than restore a guest missing a mount.
|
||||
raw, err := e.api.ExtractArchiveConfig(ctx, spec.Archive)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: bring-up dr: extract archive config (for the explicit rootfs size): %w", err)
|
||||
res.Err = fmt.Errorf("reconcile: bring-up dr: extract archive config (the restore params derive from it): %w", err)
|
||||
return
|
||||
}
|
||||
sz := archiveRootfsSizeGB(raw)
|
||||
if sz <= 0 {
|
||||
res.Err = fmt.Errorf("reconcile: bring-up dr: archive config carries no parseable rootfs size — refusing a mount-override restore without an explicit rootfs")
|
||||
overrides, err = drRestoreOverrides(raw, spec.RestoreStorage)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: bring-up dr: %w", err)
|
||||
return
|
||||
}
|
||||
overrides = map[string]string{
|
||||
"rootfs": fmt.Sprintf("%s:%d", spec.RestoreStorage, sz),
|
||||
structuralParentSlot: throwawayVolumeOverride(spec.RestoreStorage, structuralParentDir),
|
||||
structuralBootSlot: throwawayVolumeOverride(spec.RestoreStorage, structuralBootGuestPath),
|
||||
}
|
||||
}
|
||||
upid, err := e.api.RestoreLXC(ctx, proxmox.RestoreLXCOptions{
|
||||
VMID: spec.VMID, Archive: spec.Archive, Storage: spec.RestoreStorage, Pool: spec.Pool,
|
||||
|
||||
@@ -978,19 +978,54 @@ func TestRunBringUp_DRUnusedDeleteFailureWarns(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// GL-5: the archive-config rootfs parse — current-config line wins, snapshot sections never shadow
|
||||
// it, absent/unparseable → 0 (which makes the DR bring-up refuse rather than guess a size).
|
||||
func TestArchiveRootfsSizeGB(t *testing.T) {
|
||||
full := "hostname: demo\nrootfs: local-lvm:vm-9201-disk-0,size=32G\nmp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,size=200G\n"
|
||||
if got := archiveRootfsSizeGB(full); got != 32 {
|
||||
t.Errorf("full config: got %d, want 32", got)
|
||||
// GL-5: drRestoreOverrides builds the COMPLETE explicit-params restore set from the archive's
|
||||
// extracted config — the live validation proved PVE drops any mountpoint NOT named in the params,
|
||||
// so mp0/mp1 pass-through is what keeps a DR guest's data volumes. Snapshot sections never shadow;
|
||||
// unknown binds and unparseable sizes refuse.
|
||||
func TestDRRestoreOverrides(t *testing.T) {
|
||||
raw := `hostname: demo
|
||||
rootfs: local-lvm:vm-9201-disk-0,size=32G
|
||||
mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,backup=1,size=200G
|
||||
mp1: local-lvm:vm-9201-disk-2,mp=/mnt/sys_drive,backup=1,size=50G
|
||||
mp8: /mnt/felhom-drives,mp=/mnt/felhom-drives
|
||||
mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1
|
||||
[snap1]
|
||||
rootfs: local-lvm:vm-9201-disk-9,size=99G
|
||||
`
|
||||
ov, err := drRestoreOverrides(raw, "local-lvm")
|
||||
if err != nil {
|
||||
t.Fatalf("drRestoreOverrides: %v", err)
|
||||
}
|
||||
snap := "hostname: demo\n\n[before-upgrade]\nrootfs: local-lvm:vm-9201-disk-9,size=99G\n"
|
||||
if got := archiveRootfsSizeGB(snap); got != 0 {
|
||||
t.Errorf("a snapshot section's rootfs must NOT shadow an absent current one: got %d, want 0", got)
|
||||
want := map[string]string{
|
||||
"rootfs": "local-lvm:32",
|
||||
"mp0": "local-lvm:200,mp=/var/lib/docker,backup=1",
|
||||
"mp1": "local-lvm:50,mp=/mnt/sys_drive,backup=1",
|
||||
"mp8": "local-lvm:1,mp=/mnt/felhom-drives,backup=0",
|
||||
"mp9": "local-lvm:1,mp=/etc/felhom-bootstrap,backup=0",
|
||||
}
|
||||
if got := archiveRootfsSizeGB("hostname: demo\n"); got != 0 {
|
||||
t.Errorf("no rootfs line: got %d, want 0", got)
|
||||
if len(ov) != len(want) {
|
||||
t.Fatalf("overrides = %+v, want %+v", ov, want)
|
||||
}
|
||||
for k, v := range want {
|
||||
if ov[k] != v {
|
||||
t.Errorf("override[%s] = %q, want %q", k, ov[k], v)
|
||||
}
|
||||
}
|
||||
|
||||
// an UNKNOWN bind mpN = unknown topology → refuse (never restore a guest missing a mount)
|
||||
if _, err := drRestoreOverrides("rootfs: l:d,size=8G\nmp3: /srv/other,mp=/data\n", "local-lvm"); err == nil ||
|
||||
!strings.Contains(err.Error(), "unknown bind mountpoint") {
|
||||
t.Errorf("unknown bind must refuse, got %v", err)
|
||||
}
|
||||
// no parseable rootfs → refuse
|
||||
if _, err := drRestoreOverrides("hostname: x\n", "local-lvm"); err == nil ||
|
||||
!strings.Contains(err.Error(), "rootfs size") {
|
||||
t.Errorf("missing rootfs must refuse, got %v", err)
|
||||
}
|
||||
// a storage mpN without a size → refuse (cannot pass it through)
|
||||
if _, err := drRestoreOverrides("rootfs: l:d,size=8G\nmp0: l:d1,mp=/x\n", "local-lvm"); err == nil ||
|
||||
!strings.Contains(err.Error(), "no parseable size") {
|
||||
t.Errorf("sizeless mpN must refuse, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user