yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lvm plugin: snapshot-as-volume-chain: use locking for snapshot operations

Browse Source 
As reported by a user in the enterprise support in a ticket handled by
Friedrich, concurrent snapshot operations could lead to metadata
corruption of the volume group with unlucky timing. Add the missing
locking for operations modifying the metadata, i.e. allocation, rename
and removal. Since volume_snapshot() and volume_snapshot_rollback()
only do those, use a wrapper for the whole function. Since
volume_snapshot_delete() can do longer-running commit or rebase
operations, only lock the necessary sections there.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Link: https://lore.proxmox.com/20251103162330.112603-5-f.ebner@proxmox.com
This commit is contained in:
Fiona Ebner
2025-11-03 17:23:15 +01:00
committed by Thomas Lamprecht
parent 5988ac0250
commit 8eabcc7011
1 changed files with 66 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
src/PVE/Storage/LVMPlugin.pm
View File

@ -1029,7 +1029,7 @@ sub volume_size_info {
return wantarray ? ($size, 'raw', 0, undef) : $size; return wantarray ? ($size, 'raw', 0, undef) : $size;
} }
sub volume_snapshot { my sub volume_snapshot_locked {
my ($class, $scfg, $storeid, $volname, $snap) = @_; my ($class, $scfg, $storeid, $volname, $snap) = @_;
my ($vmid, $format) = ($class->parse_volname($volname))[2, 6]; my ($vmid, $format) = ($class->parse_volname($volname))[2, 6];
@ -1050,6 +1050,17 @@ sub volume_snapshot {
} }
} }
sub volume_snapshot {
my ($class, $scfg, $storeid, $volname, $snap) = @_;
return $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub { return volume_snapshot_locked($class, $scfg, $storeid, $volname, $snap); },
);
}
# Asserts that a rollback to $snap on $volname is possible. # Asserts that a rollback to $snap on $volname is possible.
# If certain snapshots are preventing the rollback and $blockers is an array # If certain snapshots are preventing the rollback and $blockers is an array
# reference, the snapshot names can be pushed onto $blockers prior to dying. # reference, the snapshot names can be pushed onto $blockers prior to dying.
@ -1086,7 +1097,7 @@ sub volume_rollback_is_possible {
return 1; return 1;
} }
sub volume_snapshot_rollback { my sub volume_snapshot_rollback_locked {
my ($class, $scfg, $storeid, $volname, $snap) = @_; my ($class, $scfg, $storeid, $volname, $snap) = @_;
my $format = ($class->parse_volname($volname))[6]; my $format = ($class->parse_volname($volname))[6];
@ -1108,6 +1119,19 @@ sub volume_snapshot_rollback {
return undef; return undef;
} }
sub volume_snapshot_rollback {
my ($class, $scfg, $storeid, $volname, $snap) = @_;
return $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub {
return volume_snapshot_rollback_locked($class, $scfg, $storeid, $volname, $snap);
},
);
}
sub volume_snapshot_delete { sub volume_snapshot_delete {
my ($class, $scfg, $storeid, $volname, $snap, $running) = @_; my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
@ -1117,7 +1141,14 @@ sub volume_snapshot_delete {
die "can't delete snapshot for '$format' volume\n" if $format ne 'qcow2'; die "can't delete snapshot for '$format' volume\n" if $format ne 'qcow2';
if ($running) { if ($running) {
my $cleanup_worker = eval { free_snap_image($class, $storeid, $scfg, $volname, $snap); }; my $cleanup_worker = eval {
return $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub { return free_snap_image($class, $storeid, $scfg, $volname, $snap); },
);
};
die "error deleting snapshot $snap $@\n" if $@; die "error deleting snapshot $snap $@\n" if $@;
fork_cleanup_worker($cleanup_worker); fork_cleanup_worker($cleanup_worker);
return; return;
@ -1152,19 +1183,31 @@ sub volume_snapshot_delete {
"The state of $snap is now invalid. Don't try to clone or rollback it. You can only try to delete it again later\n"; "The state of $snap is now invalid. Don't try to clone or rollback it. You can only try to delete it again later\n";
die "error commiting $childsnap to $snap; $@\n"; die "error commiting $childsnap to $snap; $@\n";
} }
print "delete $childvolname\n";
my $cleanup_worker =
eval { free_snap_image($class, $storeid, $scfg, $volname, $childsnap) };
if ($@) {
die "error delete old snapshot volume $childvolname: $@\n";
}
print "rename $snapvolname to $childvolname\n"; print "delete $childvolname\n";
eval { lvrename($scfg, $snapvolname, $childvolname) }; my $cleanup_worker = eval {
if ($@) { return $class->cluster_lock_storage(
warn $@; $storeid,
$err = "error renaming snapshot: $@\n"; $scfg->{shared},
} undef,
sub {
my $cleanup_worker_sub =
eval { free_snap_image($class, $storeid, $scfg, $volname, $childsnap) };
if ($@) {
die "error delete old snapshot volume $childvolname: $@\n";
}
print "rename $snapvolname to $childvolname\n";
eval { lvrename($scfg, $snapvolname, $childvolname) };
if ($@) {
warn $@;
$err = "error renaming snapshot: $@\n";
}
return $cleanup_worker_sub;
},
);
};
fork_cleanup_worker($cleanup_worker); fork_cleanup_worker($cleanup_worker);
} else { } else {
@ -1190,7 +1233,14 @@ sub volume_snapshot_delete {
die "error rebase $childsnap from $parentsnap; $@\n"; die "error rebase $childsnap from $parentsnap; $@\n";
} }
#delete the snapshot #delete the snapshot
my $cleanup_worker = eval { free_snap_image($class, $storeid, $scfg, $volname, $snap); }; my $cleanup_worker = eval {
return $class->cluster_lock_storage(
$storeid,
$scfg->{shared},
undef,
sub { return free_snap_image($class, $storeid, $scfg, $volname, $snap); },
);
};
die "error deleting old snapshot volume $snapvolname: $@\n" if $@; die "error deleting old snapshot volume $snapvolname: $@\n" if $@;
fork_cleanup_worker($cleanup_worker); fork_cleanup_worker($cleanup_worker);
} }