* [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path
@ 2021-04-07 10:25 Fabian Ebner
2021-04-07 10:25 ` [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust Fabian Ebner
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Fabian Ebner @ 2021-04-07 10:25 UTC (permalink / raw)
To: pve-devel
Only match against the file name to avoid false positives with directory names
containing "-$vmid-".
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
Found while trying to debug/reproduce a forum thread[0], but the path there
should not be affected by this...
[0]: https://forum.proxmox.com/threads/vzdump-removing-too-many-backups.87072/
PVE/Storage/Plugin.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index d2d8184..05f1701 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1034,11 +1034,13 @@ my $get_subdir_files = sub {
$info = { volid => "$sid:vztmpl/$1", format => "t$2" };
} elsif ($tt eq 'backup') {
- next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
next if $fn !~ m!/([^/]+\.(tgz|(?:(?:tar|vma)(?:\.(${\COMPRESSOR_RE}))?)))$!;
my $original = $fn;
my $format = $2;
$fn = $1;
+
+ next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
+
$info = { volid => "$sid:backup/$fn", format => $format };
my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust
2021-04-07 10:25 [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
@ 2021-04-07 10:25 ` Fabian Ebner
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
2021-04-09 6:59 ` [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
2 siblings, 1 reply; 6+ messages in thread
From: Fabian Ebner @ 2021-04-07 10:25 UTC (permalink / raw)
To: pve-devel
by relying on archive_info's vmid first. archive_info is already used to
determine if it's a standard name, and in that case the vmid is certainly set.
Also add asserts to make sure we got what we expected.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
continuing from previous patch:
...but it really seems that something went wrong with the vmids for the listing,
and making it more robust shouldn't hurt.
PVE/Storage/Plugin.pm | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 05f1701..07eb88f 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1233,9 +1233,9 @@ sub prune_backups {
foreach my $backup (@{$backups}) {
my $volid = $backup->{volid};
- my $backup_vmid = $backup->{vmid};
my $archive_info = eval { PVE::Storage::archive_info($volid) } // {};
my $backup_type = $archive_info->{type} // 'unknown';
+ my $backup_vmid = $archive_info->{vmid} // $backup->{vmid};
next if defined($type) && $type ne $backup_type;
@@ -1248,6 +1248,9 @@ sub prune_backups {
$prune_entry->{vmid} = $backup_vmid if defined($backup_vmid);
if ($archive_info->{is_std_name}) {
+ die "internal error - got no vmid\n" if !defined($backup_vmid);
+ die "internal error - got wrong vmid\n" if defined($vmid) && $backup_vmid ne $vmid;
+
$prune_entry->{ctime} = $archive_info->{ctime};
my $group = "$backup_type/$backup_vmid";
push @{$backup_groups->{$group}}, $prune_entry;
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] applied: [PATCH storage 2/2] prune backups: make vmid filtering more robust
2021-04-07 10:25 ` [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust Fabian Ebner
@ 2021-04-09 10:51 ` Thomas Lamprecht
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2021-04-09 10:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
On 07.04.21 12:25, Fabian Ebner wrote:
> by relying on archive_info's vmid first. archive_info is already used to
> determine if it's a standard name, and in that case the vmid is certainly set.
>
> Also add asserts to make sure we got what we expected.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> continuing from previous patch:
> ...but it really seems that something went wrong with the vmids for the listing,
> and making it more robust shouldn't hurt.
>
> PVE/Storage/Plugin.pm | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>
applied, small followup for VMID casing and including the info about which
values failed in the message, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path
2021-04-07 10:25 [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
2021-04-07 10:25 ` [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust Fabian Ebner
@ 2021-04-09 6:59 ` Fabian Ebner
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
2 siblings, 0 replies; 6+ messages in thread
From: Fabian Ebner @ 2021-04-09 6:59 UTC (permalink / raw)
To: pve-devel
Am 07.04.21 um 12:25 schrieb Fabian Ebner:
> Only match against the file name to avoid false positives with directory names
> containing "-$vmid-".
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> Found while trying to debug/reproduce a forum thread[0], but the path there
> should not be affected by this...
>
This was in fact the issue. The user used a pseudonym for the storage
name/path, and the actual path is affected. So I suppose the link to the
forum thread could go into the commit message for completeness.
> [0]: https://forum.proxmox.com/threads/vzdump-removing-too-many-backups.87072/
>
> PVE/Storage/Plugin.pm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index d2d8184..05f1701 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -1034,11 +1034,13 @@ my $get_subdir_files = sub {
> $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
>
> } elsif ($tt eq 'backup') {
> - next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
> next if $fn !~ m!/([^/]+\.(tgz|(?:(?:tar|vma)(?:\.(${\COMPRESSOR_RE}))?)))$!;
> my $original = $fn;
> my $format = $2;
> $fn = $1;
> +
> + next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
> +
> $info = { volid => "$sid:backup/$fn", format => $format };
>
> my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] applied: [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path
2021-04-07 10:25 [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
2021-04-07 10:25 ` [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust Fabian Ebner
2021-04-09 6:59 ` [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
@ 2021-04-09 10:51 ` Thomas Lamprecht
2021-04-09 12:52 ` Fabian Ebner
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2021-04-09 10:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Ebner
On 07.04.21 12:25, Fabian Ebner wrote:
> Only match against the file name to avoid false positives with directory names
> containing "-$vmid-".
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> Found while trying to debug/reproduce a forum thread[0], but the path there
> should not be affected by this...
>
> [0]: https://forum.proxmox.com/threads/vzdump-removing-too-many-backups.87072/
>
> PVE/Storage/Plugin.pm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>
applied, moved the comment up in the commit message, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] applied: [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
@ 2021-04-09 12:52 ` Fabian Ebner
0 siblings, 0 replies; 6+ messages in thread
From: Fabian Ebner @ 2021-04-09 12:52 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
Am 09.04.21 um 12:51 schrieb Thomas Lamprecht:
> On 07.04.21 12:25, Fabian Ebner wrote:
>> Only match against the file name to avoid false positives with directory names
>> containing "-$vmid-".
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Found while trying to debug/reproduce a forum thread[0], but the path there
>> should not be affected by this...
>>
>> [0]: https://forum.proxmox.com/threads/vzdump-removing-too-many-backups.87072/
>>
>> PVE/Storage/Plugin.pm | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>>
>
> applied, moved the comment up in the commit message, thanks!
>
Thanks for that and the follow-ups! Let's hope nobody sees those messages ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-09 12:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 10:25 [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
2021-04-07 10:25 ` [pve-devel] [PATCH storage 2/2] prune backups: make vmid filtering more robust Fabian Ebner
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
2021-04-09 6:59 ` [pve-devel] [PATCH storage 1/2] plugin: subdir files: backup: don't match for vmid against the full path Fabian Ebner
2021-04-09 10:51 ` [pve-devel] applied: " Thomas Lamprecht
2021-04-09 12:52 ` Fabian Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox