* [pve-devel] [PATCH common] add fsfreeze helper:
2020-11-05 16:06 [pve-devel] [PATCH common/storage/container] add fsfreeze/thaw for rbd snapshots Stoiko Ivanov
@ 2020-11-05 16:06 ` Stoiko Ivanov
2020-11-05 16:06 ` [pve-devel] [PATCH storage 1/1] add check for fsfreeze before snapshot Stoiko Ivanov
2020-11-05 16:06 ` [pve-devel] [PATCH container 1/1] snapshot creation: fsfreeze mountpoints, if needed Stoiko Ivanov
2 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2020-11-05 16:06 UTC (permalink / raw)
To: pve-devel
fsfreeze_mountpoint issues the same ioctl's as fsfreeze(8) on the provided
directory (the $thaw parameter deciding between '--freeze' and '--unfreeze')
This is used for container backups on RBD, where snapshots on containers, which
are heavy on IO, are not mountable readonly, because the ext4 is not consistent
Needed to fix #2991 and #2528.
The ioctl numbers were found via strace -X verbose (and compared with the
kernel source).
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PVE/Tools.pm | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 4b445ea..c61fc49 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -100,6 +100,9 @@ use constant {O_PATH => 0x00200000,
use constant {AT_EMPTY_PATH => 0x1000,
AT_FDCWD => -100};
+use constant {FIFREEZE => 0xc0045877,
+ FITHAW => 0xc0045878};
+
sub run_with_timeout {
my ($timeout, $code, @param) = @_;
@@ -1442,6 +1445,21 @@ sub sync_mountpoint {
die "syncfs '$path' failed - $syncfs_err\n" if defined $syncfs_err;
}
+sub fsfreeze_mountpoint {
+ my ($path, $thaw) = @_;
+
+ my $op = $thaw ? 'thaw' : 'freeze';
+ my $ioctl = $thaw ? FITHAW : FIFREEZE;
+
+ sysopen my $fd, $path, O_RDONLY|O_CLOEXEC or die "failed to open $path: $!\n";
+ my $ioctl_err;
+ if (!ioctl($fd, $ioctl, 0)) {
+ $ioctl_err = "$!";
+ }
+ close($fd);
+ die "fs$op '$path' failed - $ioctl_err\n" if defined $ioctl_err;
+}
+
# support sending multi-part mail messages with a text and or a HTML part
# mailto may be a single email string or an array of receivers
sub sendmail {
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH storage 1/1] add check for fsfreeze before snapshot
2020-11-05 16:06 [pve-devel] [PATCH common/storage/container] add fsfreeze/thaw for rbd snapshots Stoiko Ivanov
2020-11-05 16:06 ` [pve-devel] [PATCH common] add fsfreeze helper: Stoiko Ivanov
@ 2020-11-05 16:06 ` Stoiko Ivanov
2020-11-05 18:00 ` Thomas Lamprecht
2020-11-05 16:06 ` [pve-devel] [PATCH container 1/1] snapshot creation: fsfreeze mountpoints, if needed Stoiko Ivanov
2 siblings, 1 reply; 5+ messages in thread
From: Stoiko Ivanov @ 2020-11-05 16:06 UTC (permalink / raw)
To: pve-devel
In order to take a snapshot of a container volume, which can be mounted
read-only with RBD, the volume needs to be frozen (fsfreeze (8)) before taking
the snapshot. Only RBD is affected by this (for LVMThin this is handled
by the devicemapper).
This commit adds helpers to determine if the FIFREEZE ioctl needs to be called
for the volume.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
PVE/Storage.pm | 12 ++++++++++++
PVE/Storage/Plugin.pm | 4 ++++
PVE/Storage/RBDPlugin.pm | 5 +++++
3 files changed, 21 insertions(+)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index cd7b5ff..27223b6 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -287,6 +287,18 @@ sub volume_snapshot_delete {
}
}
+# check if a filesystem on top of a volume needs to flush it's journal for
+# constency (see fsfreeze(8)) before a snapshot is taken - needed for
+# container mountpoints
+sub volume_snapshot_needs_fsfreeze {
+ my ($cfg, $volid) = @_;
+
+ my ($storeid, $volname) = parse_volume_id($volid);
+ my $scfg = storage_config($cfg, $storeid);
+ my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+ return $plugin->volume_snapshot_needs_fsfreeze();
+}
+
# check if a volume or snapshot supports a given feature
# $feature - one of:
# clone - linked clone is possible
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 63f68da..a046640 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -888,6 +888,10 @@ sub volume_snapshot_delete {
return undef;
}
+sub volume_snapshot_needs_fsfreeze {
+
+ return 0;
+}
sub storage_can_replicate {
my ($class, $scfg, $storeid, $format) = @_;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 38f2b46..94df89d 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -694,6 +694,11 @@ sub volume_snapshot_delete {
return undef;
}
+sub volume_snapshot_needs_fsfreeze {
+
+ return 1;
+}
+
sub volume_has_feature {
my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running) = @_;
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH container 1/1] snapshot creation: fsfreeze mountpoints, if needed
2020-11-05 16:06 [pve-devel] [PATCH common/storage/container] add fsfreeze/thaw for rbd snapshots Stoiko Ivanov
2020-11-05 16:06 ` [pve-devel] [PATCH common] add fsfreeze helper: Stoiko Ivanov
2020-11-05 16:06 ` [pve-devel] [PATCH storage 1/1] add check for fsfreeze before snapshot Stoiko Ivanov
@ 2020-11-05 16:06 ` Stoiko Ivanov
2 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2020-11-05 16:06 UTC (permalink / raw)
To: pve-devel
fixes #2991, #2528.
creating a snapshot with rbd, after the syncfs finished successfully does not
guarantee that the snapshot has the state of the filesystem after syncfs.
suggestion taken from #2528 (running fsfreeze -f/-u before snapshotting on
the mountpoints)
added helper PVE::Storage::volume_snapshot_needs_fsfreeze, to indicate
which volumes need to be frozen/thawed. (and mocked it in the tests here).
Added to sync_container_namespace, since fsfreeze needs to run inside the
container's mount namespace.
tests in #2991 seem to indicate that this helps to successfully create backups.
needs a versioned dependency bump on pve-common and pve-storage
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PVE/LXC.pm | 21 +++++++++++++++++----
src/PVE/LXC/Config.pm | 15 ++++++++++++++-
src/test/snapshot-test.pm | 12 +++++++++++-
3 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 9c150d9..6f87c96 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1091,7 +1091,7 @@ my $get_container_namespace = sub {
};
my $do_syncfs = sub {
- my ($vmid, $pid, $socket) = @_;
+ my ($vmid, $pid, $socket, $freeze_mountpoints, $thaw) = @_;
&$enter_namespace($vmid, $pid, 'mnt', PVE::Tools::CLONE_NEWNS);
@@ -1121,13 +1121,26 @@ my $do_syncfs = sub {
foreach my $mp (@$mounts) {
my ($what, $dir, $fs) = @$mp;
next if $nosyncfs{$fs};
- eval { PVE::Tools::sync_mountpoint($dir); };
+ eval {
+
+ PVE::Tools::sync_mountpoint($dir) if !$thaw;
+ };
+
+ warn $@ if $@;
+ }
+
+ # freeze/unfreeze the mountpoint which need it
+ foreach my $dir (@$freeze_mountpoints) {
+
+ eval {
+ PVE::Tools::fsfreeze_mountpoint($dir, $thaw);
+ };
warn $@ if $@;
}
};
sub sync_container_namespace {
- my ($vmid) = @_;
+ my ($vmid, $freeze_mountpoints, $thaw) = @_;
my $pid = find_lxc_pid($vmid);
# SOCK_DGRAM is nicer for barriers but cannot be slurped
@@ -1140,7 +1153,7 @@ sub sync_container_namespace {
if (!$child) {
eval {
close $pfd;
- &$do_syncfs($vmid, $pid, $cfd);
+ &$do_syncfs($vmid, $pid, $cfd, $freeze_mountpoints, $thaw);
};
if (my $err = $@) {
warn $err;
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 49f599b..f7b763b 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -112,12 +112,25 @@ sub __snapshot_check_freeze_needed {
sub __snapshot_freeze {
my ($class, $vmid, $unfreeze) = @_;
+ my $conf = $class->load_config($vmid);
+ my $storagecfg = PVE::Storage::config();
+
+ my $freeze_mps = [];
+ $class->foreach_volume($conf, sub {
+ my ($ms, $mountpoint) = @_;
+
+ if (PVE::Storage::volume_snapshot_needs_fsfreeze($storagecfg, $mountpoint->{volume})) {
+ push @$freeze_mps, $mountpoint->{mp};
+ }
+ });
+
if ($unfreeze) {
eval { PVE::LXC::thaw($vmid); };
warn $@ if $@;
+ PVE::LXC::sync_container_namespace($vmid, $freeze_mps, 1);
} else {
PVE::LXC::freeze($vmid);
- PVE::LXC::sync_container_namespace($vmid);
+ PVE::LXC::sync_container_namespace($vmid, $freeze_mps, 0);
}
}
diff --git a/src/test/snapshot-test.pm b/src/test/snapshot-test.pm
index bfb8551..91a2af9 100644
--- a/src/test/snapshot-test.pm
+++ b/src/test/snapshot-test.pm
@@ -113,6 +113,15 @@ sub mocked_volume_rollback_is_possible {
die "volume_rollback_is_possible failed\n";
}
+sub mocked_volume_snapshot_needs_fsfreeze {
+ my ($storecfg, $volid) = @_;
+ die "Storage config not mocked! aborting\n"
+ if defined($storecfg);
+ die "volid undefined\n"
+ if !defined($volid);
+ return 0;
+}
+
sub mocked_vm_stop {
if ($kill_possible) {
$running = 0;
@@ -386,7 +395,8 @@ $storage_module->mock('volume_snapshot', \&mocked_volume_snapshot);
$storage_module->mock('volume_snapshot_delete', \&mocked_volume_snapshot_delete);
$storage_module->mock('volume_snapshot_rollback', \&mocked_volume_snapshot_rollback);
$storage_module->mock('volume_rollback_is_possible', \&mocked_volume_rollback_is_possible);
-printf("\tconfig(), volume_snapshot(), volume_snapshot_delete(), volume_snapshot_rollback() and volume_rollback_is_possible() mocked\n");
+$storage_module->mock('volume_snapshot_needs_fsfreeze', \&mocked_volume_snapshot_needs_fsfreeze);
+printf("\tconfig(), volume_snapshot(), volume_snapshot_delete(), volume_snapshot_rollback(), volume_rollback_is_possible() and volume_snapshot_needs_fsfreeze() mocked\n");
printf("\n");
printf("Setting up Mocking for PVE::Tools\n");
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread