[pve-devel] [PATCH container] backup: warn that tar does not honor exclusion pattern with a trailing slash

[pve-devel] [PATCH container] backup: warn that tar does not honor exclusion pattern with a trailing slash

[Fiona Ebner]

As reported in the community forum [0], for tar, an exclusion pattern
with a trailing slash will not match a folder with that name. For
rsync and proxmox-backup-client however, such a pattern will exclude
a directory with that name, but not a file.

rsync is used for 'suspend' mode backup and tar is used for all
non-PBS backups to create the archive. So currently, in the presence
of an exclusion pattern with a trailing slash, there is inconsistency
between different backup modes (because for 'suspend' mode, rsync will
already do the exclusion too) as well as between PBS and non-PBS
backups.

There doesn't seem to be a straight-forward way to align the behavior
for tar with command-line options exactly. The trailing slash can't be
removed from the pattern, because that would also match files.
Matching with
> some/pattern/*
> some/pattern/.*
rather than
> some/pattern/
gets pretty close, which was suggested by Dominik. Just the empty
directory is still included.

In any case, modifying current behavior would be a breaking change, so
actually aligning the exclusion (more closely) is better done in the
next major release.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

One could argue such a change is a bug-fix and so does not need to
wait until the next major release. I opted for the safer variant for
now, but happy to go with the aligning already if that is preferred.

 src/PVE/VZDump/LXC.pm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index 8c28a5e..914ede2 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -442,7 +442,16 @@ sub archive {
 	    } else {
 		push @findexcl_anchored, $pattern;
 	    }
-	}
+
+	    # NOTE rsync and proxmox-backup-client will match directories, but not files when there
+	    # is a trailing slash, tar won't match either. For suspend mode, rsync already did the
+	    # exclusion, so no need to warn.
+	    # TODO PVE 9 - consider matching "$pattern*" and "$pattern.*" in this case, which will
+	    # only include the empty directory to more closely align the behavior between different
+	    # modes. Don't forget to update the docs!
+	    $self->log("warn", "tar does not match exclusion with a trailing slash '$pattern'")
+		if $pattern =~ m|/$| && $task->{mode} ne 'suspend';
+        }
 
 	push @$tar, '--no-anchored';
 	push @$tar, '--exclude=lost+found' if $userns_cmd;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs] backup: warn that tar does not honor exclusion pattern with a trailing slash

[Fiona Ebner]

As reported in the community forum [0], for tar, an exclusion pattern
with a trailing slash will not match a folder with that name. For
rsync and proxmox-backup-client however, such a pattern will exclude
a directory with that name, but not a file.

rsync is used for 'suspend' mode backup and tar is used for all
non-PBS backups to create the archive. So currently, in the presence
of an exclusion pattern with a trailing slash, there is inconsistency
between different backup modes (because for 'suspend' mode, rsync will
already do the exclusion too) as well as between PBS and non-PBS
backups.

There doesn't seem to be a straight-forward way to align the behavior
for tar with command-line options exactly. The trailing slash can't be
removed from the pattern, because that would also match files.
Matching with
> some/pattern/*
> some/pattern/.*
rather than
> some/pattern/
gets pretty close, which was suggested by Dominik. Just the empty
directory is still included.

In any case, modifying current behavior would be a breaking change, so
actually aligning the exclusion (more closely) is better done in the
next major release.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 vzdump.adoc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/vzdump.adoc b/vzdump.adoc
index 79d4bbc..3918e9a 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -578,6 +578,12 @@ You can also manually specify (additional) exclude paths, for example:
 excludes the directory `/tmp/` and any file or directory named `/var/foo`,
 `/var/foobar`, and so on.
 
+WARNING: For backups to Proxmox Backup Server (PBS) and `suspend` mode backups,
+patterns with a trailing slash will match directories, but not files. On the
+other hand, for non-PBS `snapshot` mode and `stop` mode backups, patterns with a
+trailing slash currently do not match at all, because the `tar` command does not
+support that.
+
 Paths that do not start with a `/` are not anchored to the container's root,
 but will match relative to any subdirectory. For example:
 
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH container] backup: warn that tar does not honor exclusion pattern with a trailing slash

[Thomas Lamprecht]

Am 31/05/2024 um 12:07 schrieb Fiona Ebner:
> As reported in the community forum [0], for tar, an exclusion pattern
> with a trailing slash will not match a folder with that name. For
> rsync and proxmox-backup-client however, such a pattern will exclude
> a directory with that name, but not a file.
> 
> rsync is used for 'suspend' mode backup and tar is used for all
> non-PBS backups to create the archive. So currently, in the presence
> of an exclusion pattern with a trailing slash, there is inconsistency
> between different backup modes (because for 'suspend' mode, rsync will
> already do the exclusion too) as well as between PBS and non-PBS
> backups.
> 
> There doesn't seem to be a straight-forward way to align the behavior
> for tar with command-line options exactly. The trailing slash can't be
> removed from the pattern, because that would also match files.
> Matching with
>> some/pattern/*
>> some/pattern/.*
> rather than
>> some/pattern/
> gets pretty close, which was suggested by Dominik. Just the empty
> directory is still included.
> 
> In any case, modifying current behavior would be a breaking change, so
> actually aligning the exclusion (more closely) is better done in the
> next major release.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> One could argue such a change is a bug-fix and so does not need to
> wait until the next major release. I opted for the safer variant for
> now, but happy to go with the aligning already if that is preferred.

a major release may indeed better for conveying such a change and it
seems not _that_ frequent, so fine by me to wait.

> 
>  src/PVE/VZDump/LXC.pm | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH docs] backup: warn that tar does not honor exclusion pattern with a trailing slash

[Fiona Ebner]

Am 31.05.24 um 12:07 schrieb Fiona Ebner:
> As reported in the community forum [0], for tar, an exclusion pattern
> with a trailing slash will not match a folder with that name. For
> rsync and proxmox-backup-client however, such a pattern will exclude
> a directory with that name, but not a file.
> 
> rsync is used for 'suspend' mode backup and tar is used for all
> non-PBS backups to create the archive. So currently, in the presence
> of an exclusion pattern with a trailing slash, there is inconsistency
> between different backup modes (because for 'suspend' mode, rsync will
> already do the exclusion too) as well as between PBS and non-PBS
> backups.
> 
> There doesn't seem to be a straight-forward way to align the behavior
> for tar with command-line options exactly. The trailing slash can't be
> removed from the pattern, because that would also match files.
> Matching with
>> some/pattern/*
>> some/pattern/.*
> rather than
>> some/pattern/
> gets pretty close, which was suggested by Dominik. Just the empty
> directory is still included.
> 
> In any case, modifying current behavior would be a breaking change, so
> actually aligning the exclusion (more closely) is better done in the
> next major release.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Ping (it seems the related docs patch here was not yet applied yet,
while the warning in pve-container was).


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel