[pve-devel] [RFC container] fix #3606: drop --inplace from suspend backups

[pve-devel] [RFC container] fix #3606: drop --inplace from suspend backups

[Fabian Grünbichler]

for bullseye-based systems, the 'fs.protected_regular'[0] sysctl is set
to '2' by default[1] (as opposed to the old value of '0'). this breaks
rsync's `--inplace` mode for such protected files, since opening them
with O_CREAT is not even possible for the root user anymore.

one example in the wild are debian (-based) containers using PHP, where
the session dir '/var/lib/php/sessions' is sticky, world-writable, owned
by root and contains sessions files usually owned by www-data. if any of
these session files are modified between the first and second rsync run,
the second run and thus the backup will fail.

the downside of this change is that containers with large files that are
updated between the first and second run will now see more (temp) space
usage - but suspend mode is not space efficient anyway and such setups
should consider switching to snapshot mode anyway.

additionaly, this commit drops the now no longer needed $first parameter
previously used to decide between different parameters for first and
second rsync run.

0: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30aba6656f61ed44cba445a3c0d38b296fa9e8f5
1: https://salsa.debian.org/debian/procps/-/commit/299f4a1a10810e2995e666374b880b543af8e8e4

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
RFC in case anybody has a better solution other than setting the sysctl
to 0 again ;)

 src/PVE/VZDump/LXC.pm | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index b7f7463..19b5c16 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -20,7 +20,7 @@ use base qw (PVE::VZDump::Plugin);
 my $default_mount_point = "/mnt/vzsnap0";
 
 my $rsync_vm = sub {
-    my ($self, $task, $to, $text, $first) = @_;
+    my ($self, $task, $to, $text) = @_;
 
     my $disks = $task->{disks};
     my $from = $disks->[0]->{dir};
@@ -32,8 +32,7 @@ my $rsync_vm = sub {
 
     my $rsync = ['rsync', '--stats', '-h', @xattr, '--numeric-ids',
                  '-aH', '--delete', '--no-whole-file',
-                 ($first ? '--sparse' : '--inplace'),
-                 '--one-file-system', '--relative'];
+                 '--sparse', '--one-file-system', '--relative'];
     push @$rsync, "--bwlimit=$opts->{bwlimit}" if $opts->{bwlimit};
     push @$rsync, map { "--exclude=$_" } @{$self->{vzdump}->{findexcl}};
     push @$rsync, map { "--exclude=$_" } @{$task->{exclude_dirs}};
@@ -260,13 +259,13 @@ sub copy_data_phase1 {
 	}
     }
 
-    $self->$rsync_vm($task, $task->{snapdir}, "first", 1);
+    $self->$rsync_vm($task, $task->{snapdir}, "first");
 }
 
 sub copy_data_phase2 {
     my ($self, $task) = @_;
 
-    $self->$rsync_vm($task, $task->{snapdir}, "final", 0);
+    $self->$rsync_vm($task, $task->{snapdir}, "final");
 }
 
 sub stop_vm {
-- 
2.30.2

Re: [pve-devel] [RFC container] fix #3606: drop --inplace from suspend backups

[Fabian Grünbichler]

ping - this still regularly hits users in the forum, and given rsyncs 
rather terse output it's really hard to disambiguate from other error 
conditions causing error "23".

On September 8, 2021 12:04 pm, Fabian Grünbichler wrote:
> for bullseye-based systems, the 'fs.protected_regular'[0] sysctl is set
> to '2' by default[1] (as opposed to the old value of '0'). this breaks
> rsync's `--inplace` mode for such protected files, since opening them
> with O_CREAT is not even possible for the root user anymore.
> 
> one example in the wild are debian (-based) containers using PHP, where
> the session dir '/var/lib/php/sessions' is sticky, world-writable, owned
> by root and contains sessions files usually owned by www-data. if any of
> these session files are modified between the first and second rsync run,
> the second run and thus the backup will fail.
> 
> the downside of this change is that containers with large files that are
> updated between the first and second run will now see more (temp) space
> usage - but suspend mode is not space efficient anyway and such setups
> should consider switching to snapshot mode anyway.
> 
> additionaly, this commit drops the now no longer needed $first parameter
> previously used to decide between different parameters for first and
> second rsync run.
> 
> 0: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30aba6656f61ed44cba445a3c0d38b296fa9e8f5
> 1: https://salsa.debian.org/debian/procps/-/commit/299f4a1a10810e2995e666374b880b543af8e8e4
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> RFC in case anybody has a better solution other than setting the sysctl
> to 0 again ;)
> 
>  src/PVE/VZDump/LXC.pm | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
> index b7f7463..19b5c16 100644
> --- a/src/PVE/VZDump/LXC.pm
> +++ b/src/PVE/VZDump/LXC.pm
> @@ -20,7 +20,7 @@ use base qw (PVE::VZDump::Plugin);
>  my $default_mount_point = "/mnt/vzsnap0";
>  
>  my $rsync_vm = sub {
> -    my ($self, $task, $to, $text, $first) = @_;
> +    my ($self, $task, $to, $text) = @_;
>  
>      my $disks = $task->{disks};
>      my $from = $disks->[0]->{dir};
> @@ -32,8 +32,7 @@ my $rsync_vm = sub {
>  
>      my $rsync = ['rsync', '--stats', '-h', @xattr, '--numeric-ids',
>                   '-aH', '--delete', '--no-whole-file',
> -                 ($first ? '--sparse' : '--inplace'),
> -                 '--one-file-system', '--relative'];
> +                 '--sparse', '--one-file-system', '--relative'];
>      push @$rsync, "--bwlimit=$opts->{bwlimit}" if $opts->{bwlimit};
>      push @$rsync, map { "--exclude=$_" } @{$self->{vzdump}->{findexcl}};
>      push @$rsync, map { "--exclude=$_" } @{$task->{exclude_dirs}};
> @@ -260,13 +259,13 @@ sub copy_data_phase1 {
>  	}
>      }
>  
> -    $self->$rsync_vm($task, $task->{snapdir}, "first", 1);
> +    $self->$rsync_vm($task, $task->{snapdir}, "first");
>  }
>  
>  sub copy_data_phase2 {
>      my ($self, $task) = @_;
>  
> -    $self->$rsync_vm($task, $task->{snapdir}, "final", 0);
> +    $self->$rsync_vm($task, $task->{snapdir}, "final");
>  }
>  
>  sub stop_vm {
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 

[pve-devel] applied: [RFC container] fix #3606: drop --inplace from suspend backups

[Thomas Lamprecht]

Am 08/09/2021 um 12:04 schrieb Fabian Grünbichler:
> for bullseye-based systems, the 'fs.protected_regular'[0] sysctl is set
> to '2' by default[1] (as opposed to the old value of '0'). this breaks
> rsync's `--inplace` mode for such protected files, since opening them
> with O_CREAT is not even possible for the root user anymore.
> 
> one example in the wild are debian (-based) containers using PHP, where
> the session dir '/var/lib/php/sessions' is sticky, world-writable, owned
> by root and contains sessions files usually owned by www-data. if any of
> these session files are modified between the first and second rsync run,
> the second run and thus the backup will fail.
> 
> the downside of this change is that containers with large files that are
> updated between the first and second run will now see more (temp) space
> usage - but suspend mode is not space efficient anyway and such setups
> should consider switching to snapshot mode anyway.
> 
> additionaly, this commit drops the now no longer needed $first parameter
> previously used to decide between different parameters for first and
> second rsync run.
> 
> 0: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30aba6656f61ed44cba445a3c0d38b296fa9e8f5
> 1: https://salsa.debian.org/debian/procps/-/commit/299f4a1a10810e2995e666374b880b543af8e8e4
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> RFC in case anybody has a better solution other than setting the sysctl
> to 0 again ;)

doesn't seems like anybody could come up with something better...

> 
>  src/PVE/VZDump/LXC.pm | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
>

applied, thanks!