Package: rsnapshot
Version: 1.4.2-1
Severity: normal
Tags: patch

Problem: the robustness of rsnapshot program under desktop environment
when cron is not always running and "beta" may be invoked consecutively
without invoking "alpha".

(I am using Debian/stable now but the version in testing/unstable is the same)

How to re-produce problem:

Let me present a simple manual invocation case as below (No
cron/anacron/systemd.timer...):

With /etc/rsnapshot.conf having:
---
retain  alpha   6
retain  beta    7
retain  gamma   4
---

Let's invoke:
---
 $ sudo rsnapshot alpha
---

Invoking this 6 times will create:
/var/cache/rsnapshot/alpha.0/
/var/cache/rsnapshot/alpha.1/
/var/cache/rsnapshot/alpha.2/
/var/cache/rsnapshot/alpha.3/
/var/cache/rsnapshot/alpha.4/
/var/cache/rsnapshot/alpha.5/

So far, this looks good. Then, let's invoke:
---
 $ sudo rsnapshot beta
---

Invoking this will create:
/var/cache/rsnapshot/alpha.0/
/var/cache/rsnapshot/alpha.1/
/var/cache/rsnapshot/alpha.2/
/var/cache/rsnapshot/alpha.3/
/var/cache/rsnapshot/alpha.4/
/var/cache/rsnapshot/beta.0/

So far still good.  Let's suppose to invoke the following again and see
the result:
---
 $ sudo rsnapshot beta
/var/cache/rsnapshot/alpha.5 not present (yet), nothing to copy
---

This message is true but the result is a bit unexpected:
/var/cache/rsnapshot/alpha.0/
/var/cache/rsnapshot/alpha.1/
/var/cache/rsnapshot/alpha.2/
/var/cache/rsnapshot/alpha.3/
/var/cache/rsnapshot/alpha.4/
/var/cache/rsnapshot/beta.1/

Why beta.0 is missing ???

What happens is delete and rotate actions for "beta" is performed then
it fails to move file from "alpha.5" to "beta.0".  Rotating without new
data added to "*.0" is bad.  It should not do this delete and rotate
actions if "alpha.5" isn't there.

I am afraid this problem may be the root cause of many unexpected
results under non-optimal invocation of rsnapshot.
  https://bugs.debian.org/523923
  https://bugs.debian.org/573254

Backup script should be robust ;-)

Solution proposal:

Let's check situation with:
 -d "$config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max")
If not exist, "delete" and "rotate" should be skipped.

See attached patch file (patch -p0) for exact detail.

This may be better to be addressed by the upstream.

Osamu

-- System Information:
Debian Release: 9.4
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.9.0-6-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages rsnapshot depends on:
ii  liblchown-perl  1.01-3+b2
ii  logrotate       3.11.0-0.1
ii  perl            5.24.1-3+deb9u3
ii  rsync           3.1.2-1+deb9u1

Versions of packages rsnapshot recommends:
ii  openssh-client [ssh-client]  1:7.4p1-10+deb9u3

rsnapshot suggests no packages.

-- Configuration Files:
/etc/rsnapshot.conf changed:
config_version  1.2
snapshot_root   /var/cache/rsnapshot/
cmd_cp          /bin/cp
cmd_rm          /bin/rm
cmd_rsync       /usr/bin/rsync
cmd_logger      /usr/bin/logger
cmd_du          /usr/bin/du
cmd_rsnapshot_diff      /usr/bin/rsnapshot-diff
retain  alpha   6
retain  beta    7
retain  gamma   4
verbose         2
loglevel        3
logfile /var/log/rsnapshot.log
lockfile        /var/run/rsnapshot.pid
du_args -csh
link_dest       1
backup  /home/          localhost/
backup  /etc/           localhost/
backup  /usr/local/     localhost/


-- no debconf information
--- rsnapshot-program.pl.orig	2018-06-09 23:14:10.000000000 +0900
+++ rsnapshot-program.pl	2018-06-09 23:17:47.010919314 +0900
@@ -4666,9 +4666,8 @@
 
 	# ROTATE DIRECTORIES
 	#
-	# delete the oldest one (if we're keeping more than one)
-	if (-d "$config_vars{'snapshot_root'}/$interval.$interval_max") {
-
+	# delete the oldest one (if we're keeping more than one), if needed
+	if ((-d "$config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max") and (-d "$config_vars{'snapshot_root'}/$interval.$interval_max")) { 
 		# if use_lazy_deletes is set move the oldest directory to _delete.$$
 		# otherwise preform the default behavior
 		if (1 == $use_lazy_deletes) {
@@ -4703,35 +4702,45 @@
 		}
 
 	}
-	else {
+	elsif (-d "$config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max") {
 		print_msg(
 			"$config_vars{'snapshot_root'}/$interval.$interval_max not present (yet), nothing to delete", 4);
 	}
+	else {
+		print_msg(
+			"$config_vars{'snapshot_root'}/$interval.$prev_interval_max not present (yet), no need to delete $config_vars{'snapshot_root'}/$interval.$interval_max", 4);
+	}
 
 	# rotate the middle ones
-	for (my $i = ($interval_max - 1); $i >= 0; $i--) {
-		if (-d "$config_vars{'snapshot_root'}/$interval.$i") {
-			print_cmd(
-				"mv $config_vars{'snapshot_root'}/$interval.$i/ ",
-				"$config_vars{'snapshot_root'}/$interval." . ($i + 1) . "/"
-			);
-
-			if (0 == $test) {
-				my $result = safe_rename(
-					"$config_vars{'snapshot_root'}/$interval.$i",
-					("$config_vars{'snapshot_root'}/$interval." . ($i + 1))
+	if (-d "$config_vars{'snapshot_root'}/$prev_interval.$prev_interval_max") {
+		for (my $i = ($interval_max - 1); $i >= 0; $i--) {
+			if (-d "$config_vars{'snapshot_root'}/$interval.$i") {
+				print_cmd(
+					"mv $config_vars{'snapshot_root'}/$interval.$i/ ",
+					"$config_vars{'snapshot_root'}/$interval." . ($i + 1) . "/"
 				);
-				if (0 == $result) {
-					my $errstr = '';
-					$errstr .= "Error! safe_rename(\"$config_vars{'snapshot_root'}/$interval.$i/\", \"";
-					$errstr .= "$config_vars{'snapshot_root'}/$interval." . ($i + 1) . '/' . "\")";
-					bail($errstr);
+	
+				if (0 == $test) {
+					my $result = safe_rename(
+						"$config_vars{'snapshot_root'}/$interval.$i",
+						("$config_vars{'snapshot_root'}/$interval." . ($i + 1))
+					);
+					if (0 == $result) {
+						my $errstr = '';
+						$errstr .= "Error! safe_rename(\"$config_vars{'snapshot_root'}/$interval.$i/\", \"";
+						$errstr .= "$config_vars{'snapshot_root'}/$interval." . ($i + 1) . '/' . "\")";
+						bail($errstr);
+					}
 				}
 			}
+			else {
+				print_msg("$config_vars{'snapshot_root'}/$interval.$i not present (yet), nothing to rotate", 4);
+			}
 		}
-		else {
-			print_msg("$config_vars{'snapshot_root'}/$interval.$i not present (yet), nothing to rotate", 4);
-		}
+	}
+	else {
+		print_msg(
+			"$config_vars{'snapshot_root'}/$interval.$prev_interval_max not present (yet), no need to rotate $config_vars{'snapshot_root'}/$interval.$interval.*", 4);
 	}
 
 	# prev.max and interval.0 require more attention

Reply via email to