Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions pkg/storage/lvm/lvm_thin.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (t *Thin) DeleteSnapshot(ctx context.Context, snap storage.Snapshot) error
//
// Upstream LINSTOR equivalent:
//
// lvcreate -s --kernel --activate y --name <tgt> <vg>/<src-snap>
// lvcreate -s --setactivationskip n --activate y --name <tgt> <vg>/<src-snap>
//
// Idempotent: target LV present → resumed reconcile, no-op.
func (t *Thin) RestoreVolumeFromSnapshot(ctx context.Context, target storage.Volume, src storage.Snapshot) error {
Expand All @@ -272,7 +272,6 @@ func (t *Thin) RestoreVolumeFromSnapshot(ctx context.Context, target storage.Vol

_, err := t.exec.Run(ctx, "lvcreate",
Args("--snapshot",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since this lvcreate command explicitly activates the volume (--activate y), it should use ArgsUdevless instead of Args to apply the udev-less activation workaround (activation { udev_sync=0 udev_rules=0 }). In a containerized environment without a udev daemon, activating a volume without this workaround can cause the command to hang or race during device node creation (as documented in Bug 269 and Bug 305).

Suggested change
Args("--snapshot",
\t\tArgsUdevless(\"--snapshot\",

"--kernel",
"--setactivationskip", "n",
"--activate", "y",
"--name", tgtName,
Expand Down
46 changes: 46 additions & 0 deletions pkg/storage/lvm/lvm_thin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,52 @@ func TestThinCreateSnapshot(t *testing.T) {
}
}

// TestThinRestoreVolumeFromSnapshotIssuesThinSnapshotLvcreate is the
// BUG-043 regression pin. The clone / snapshot-restore seed path on
// LVM-thin must drive a plain thin-snapshot `lvcreate --snapshot
// --setactivationskip n --activate y --name <tgt> <vg>/<src-snap>` —
// the original code emitted a `--kernel` token, which is NOT a valid
// lvcreate option (`lvcreate: unrecognized option '--kernel'`, exit 3)
// on LVM >= 2.03. With that flag the destination backing LV was never
// created, so DRBD never came up and clone/restore onto LVM-thin pools
// never converged to UpToDate. The negative assertion (no `--kernel`)
// is load-bearing; the positive assertion pins the activation
// semantics so the cloned LV is actually usable (skip-activation
// cleared, activated).
func TestThinRestoreVolumeFromSnapshotIssuesThinSnapshotLvcreate(t *testing.T) {
fx := storage.NewFakeExec()
// Target LV not yet present → restore proceeds past the
// idempotency short-circuit.
fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-2_00000",
storage.FakeResponse{Stdout: []byte("")})
// Source snapshot LV exists → clone source is found.
fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-1_snap-1_00000",
storage.FakeResponse{Stdout: []byte(" pvc-1_snap-1_00000")})

p := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "thinpool"}, fx)

err := p.RestoreVolumeFromSnapshot(t.Context(),
storage.Volume{ResourceName: "pvc-2", VolumeNumber: 0},
storage.Snapshot{ResourceName: "pvc-1", SnapshotName: "snap-1"})
if err != nil {
t.Fatalf("RestoreVolumeFromSnapshot: %v", err)
}

want := "lvcreate --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --snapshot --setactivationskip n --activate y --name pvc-2_00000 vg/pvc-1_snap-1_00000"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Update the expected command string to include the udev-less activation configuration (activation { udev_sync=0 udev_rules=0 }) to match the change to ArgsUdevless in RestoreVolumeFromSnapshot.

\twant := \"lvcreate --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } activation { udev_sync=0 udev_rules=0 } --snapshot --setactivationskip n --activate y --name pvc-2_00000 vg/pvc-1_snap-1_00000\"

if !slices.Contains(fx.CommandLines(), want) {
t.Errorf("expected %q in calls; got %v", want, fx.CommandLines())
}

// BUG-043: no lvcreate invocation may carry the invalid `--kernel`
// flag — it makes LVM >= 2.03 reject the command with exit 3.
for _, line := range fx.CommandLines() {
if strings.HasPrefix(line, "lvcreate ") && strings.Contains(line, "--kernel") {
t.Errorf("BUG-043: thin restore lvcreate must NOT pass the invalid "+
"--kernel flag; got %q", line)
}
}
}

// TestThinExecError surfaces the wrapped exec error verbatim.
func TestThinExecError(t *testing.T) {
fx := storage.NewFakeExec()
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cli-matrix/multi-volume-late-vd-create.sh
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ if [[ "$late_up" != "true" ]]; then
# Surface the smoking gun: a Diskless line for a non-DISKLESS spec.
echo "----- linstor r l --resources $RD (with flags) -----" >&2
"${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
| jq -r '.[][]? | "\(.node_name) vol=\(.vlms[]?.vlm_nr) state=\(.vlms[]?.state.disk_state) flags=\(.rsc_flags//[])"' >&2 || true
| jq -r '.[][]? | "\(.node_name) vol=\(.vlms[]?.vlm_nr) state=\(.vlms[]?.state.disk_state) flags=\(.flags//[])"' >&2 || true
exit 1
fi

Expand All @@ -137,7 +137,7 @@ fi
# Bug 332 (Unintentional Diskless) from spec-pinned diskless replicas.
echo ">> [Bug 332] kernel-truth: drbdadm status on a diskful node"
satellite_node=$("${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
| jq -r '.[][]? | select((.rsc_flags//[]) | (map(. == "DISKLESS") | any | not)) | .node_name' \
| jq -r '.[][]? | select((.flags//[]) | (map(. == "DISKLESS") | any | not)) | .node_name' \
2>/dev/null | head -1)

if [[ -z "$satellite_node" ]]; then
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cli-matrix/n-rst-recreates-tiebreaker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ n_tb=0
while (( $(date +%s) < deadline )); do
wire=$(linstor_r_l_json "$RD")
n_tb=$(printf '%s' "$wire" \
| jq -r '.[][] | select((.rsc_flags // []) | index("TIE_BREAKER")) | .name' 2>/dev/null \
| jq -r '.[][] | select((.flags // []) | index("TIE_BREAKER")) | .name' 2>/dev/null \
| wc -l | tr -d ' ' || echo 0)
if [[ "$n_tb" == "1" ]]; then
break
Expand All @@ -195,7 +195,7 @@ while (( $(date +%s) < deadline )); do
done
if [[ "$n_tb" != "1" ]]; then
echo "FAIL (Bug 386): linstor r l shows $n_tb TIE_BREAKER rows for $RD, want 1" >&2
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
exit 1
fi

Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cli-matrix/r-d-collapses-tiebreaker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ wire=$(linstor_r_l_json "$RD")
n_wire=$(printf '%s' "$wire" | jq -r '.[][] | .name' 2>/dev/null | wc -l | tr -d ' ' || echo 0)
if [[ "$n_wire" != "1" ]]; then
echo "FAIL (Bug 338): linstor r l shows $n_wire rows for $RD, want 1" >&2
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
exit 1
fi

wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .rsc_flags // [] | join(",")' 2>/dev/null || echo "")
wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .flags // [] | join(",")' 2>/dev/null || echo "")
if [[ "$wire_flags" == *"TIE_BREAKER"* ]] || [[ "$wire_flags" == *"DISKLESS"* ]]; then
echo "FAIL (Bug 338): surviving row carries unexpected flags=$wire_flags" >&2
exit 1
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cli-matrix/r-td-diskless-reaps-tiebreaker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ wire=$(linstor_r_l_json "$RD")
n_wire=$(printf '%s' "$wire" | jq -r '.[][] | .name' 2>/dev/null | wc -l | tr -d ' ' || echo 0)
if [[ "$n_wire" != "2" ]]; then
echo "FAIL: linstor r l shows $n_wire rows for $RD, want 2" >&2
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
exit 1
fi

wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .rsc_flags // [] | join(",")' 2>/dev/null || echo "")
wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .flags // [] | join(",")' 2>/dev/null || echo "")
if [[ "$wire_flags" == *"TIE_BREAKER"* ]]; then
echo "FAIL: a surviving row still carries TIE_BREAKER (flags=$wire_flags)" >&2
exit 1
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/cli-matrix/rd-d-deleting-surface.sh
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ while (( $(date +%s) < deadline )); do
# Wire-level assertion: the Resource view carries the DELETE flag for
# the blocked replica. golinstor's `r l -o json` is a doubly-nested
# array of per-replica rows; the resource-level flags live under
# `rsc_flags` (see multi-volume-late-vd-create.sh / n-rst-recreates-
# `flags` (see multi-volume-late-vd-create.sh / n-rst-recreates-
# tiebreaker.sh for the same shape).
rl_json=$("${LCTL[@]}" resource list --resources "$RD" -o json 2>/dev/null || echo '[]')
has_delete=$(jq -r --arg n "$N2" '
[ .[]?[]?
| select(.node_name == $n)
| (.rsc_flags // []) | index("DELETE") ] | any' <<<"$rl_json" 2>/dev/null || echo false)
| (.flags // []) | index("DELETE") ] | any' <<<"$rl_json" 2>/dev/null || echo false)
# Fallback: the human-readable State column rendered by the CLI.
rl_txt=$("${LCTL[@]}" resource list --resources "$RD" 2>/dev/null || true)
if [[ "$has_delete" == "true" ]] || grep -qiE 'DELETING' <<<"$rl_txt"; then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ both_up=false
while (( $(date +%s) < deadline )); do
# 2 diskful replicas × 2 volumes = 4 disk_state strings, all UpToDate.
states=$("${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
| jq -r '[.[][]? | select((.rsc_flags//[]) | (map(. == "DISKLESS" or . == "TIE_BREAKER") | any | not)) | .vlms[]? | .state.disk_state // "Unknown"] | join(",")' \
| jq -r '[.[][]? | select((.flags//[]) | (map(. == "DISKLESS" or . == "TIE_BREAKER") | any | not)) | .vlms[]? | .state.disk_state // "Unknown"] | join(",")' \
2>/dev/null || echo "")
count_uptodate=$(awk -F, '{ for (i=1;i<=NF;i++) if ($i=="UpToDate") n++ } END { print n+0 }' <<<"$states")
if (( count_uptodate == 4 )); then
Expand Down