Re: [pve-devel] [PATCH storage v2 1/3] compact regex for backup file filter
On Wed, Feb 12, 2020 at 11:19:06AM +0100, Thomas Lamprecht wrote:
> On 1/31/20 5:00 PM, Alwin Antreich wrote:
> > --- a/PVE/Storage/Plugin.pm
> > +++ b/PVE/Storage/Plugin.pm
> > @@ -423,7 +423,7 @@ sub parse_volname {
> > return ('vztmpl', $1);
> > } elsif ($volname =~ m!^rootdir/(\d+)$!) {
> > return ('rootdir', $1, $1);
> > -} elsif ($volname =~
> > m!^backup/([^/]+(\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo)))$!) {
> > +} elsif ($volname =~
> > m!^backup/([^/]+(\.(tgz|((tar|vma)(\.(gz|lzo))?$!) {
> > my $fn = $1;
> > if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
> > return ('backup', $fn, $2);
> > @@ -910,7 +910,7 @@ my $get_subdir_files = sub {
> >
> > } elsif ($tt eq 'backup') {
> > next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
> > - next if $fn !~
> > m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
> > + next if $fn !~ m!/([^/]+\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!;
> >
> > $info = { volid => "$sid:backup/$1", format => $2 };
> writing test for this up front as a first patch would be great.
> It should be pretty simple, i.e., have a array of maps with params and
> expected
> result as two key => value entries. Additionally "expected error" checks
> would be
> great too.
> While there's already a sub test in zfspoolplugin test I'd just do a new one.
>
> This way we would have a safety net for such and future changes.
There are no negative tests or maps, but I once posted some tests for
parse_volname
[0] to the list. Maybe they could be a starting point.
[0] https://pve.proxmox.com/pipermail/pve-devel/2019-December/040887.html
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v2 1/3] compact regex for backup file filter
On 1/31/20 5:00 PM, Alwin Antreich wrote:
> this, more compact form of the regex should allow easier addition of new
> file extensions.
>
> Signed-off-by: Alwin Antreich
> ---
> PVE/Storage.pm| 2 +-
> PVE/Storage/Plugin.pm | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 0bd103e..1688077 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -514,7 +514,7 @@ sub path_to_volume_id {
> } elsif ($path =~ m!^$privatedir/(\d+)$!) {
> my $vmid = $1;
> return ('rootdir', "$sid:rootdir/$vmid");
> - } elsif ($path =~
> m!^$backupdir/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!) {
> + } elsif ($path =~
> m!^$backupdir/([^/]+\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!) {
the "feature" of the old regex was that $2 included the extension,
you want to throw some non-capturing groups (?:) in there for that.
Maybe we could strike a balance by pulling out the compressors into a regex
variable?
(tar(?:\.$COMPRESSOR_RE)?|tgz|vma(?:\.$COMPRESSOR_RE))
Would allow for reuse below.
> my $name = $1;
> return ('iso', "$sid:backup/$name");
> }
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 0c39cbd..58a801a 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -423,7 +423,7 @@ sub parse_volname {
> return ('vztmpl', $1);
> } elsif ($volname =~ m!^rootdir/(\d+)$!) {
> return ('rootdir', $1, $1);
> -} elsif ($volname =~
> m!^backup/([^/]+(\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo)))$!) {
> +} elsif ($volname =~
> m!^backup/([^/]+(\.(tgz|((tar|vma)(\.(gz|lzo))?$!) {
> my $fn = $1;
> if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
> return ('backup', $fn, $2);
> @@ -910,7 +910,7 @@ my $get_subdir_files = sub {
>
> } elsif ($tt eq 'backup') {
> next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
> - next if $fn !~
> m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
> + next if $fn !~ m!/([^/]+\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!;
>
> $info = { volid => "$sid:backup/$1", format => $2 };
>
>
writing test for this up front as a first patch would be great.
It should be pretty simple, i.e., have a array of maps with params and expected
result as two key => value entries. Additionally "expected error" checks would
be
great too.
While there's already a sub test in zfspoolplugin test I'd just do a new one.
This way we would have a safety net for such and future changes.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH storage v2 1/3] compact regex for backup file filter
this, more compact form of the regex should allow easier addition of new
file extensions.
Signed-off-by: Alwin Antreich
---
PVE/Storage.pm| 2 +-
PVE/Storage/Plugin.pm | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 0bd103e..1688077 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -514,7 +514,7 @@ sub path_to_volume_id {
} elsif ($path =~ m!^$privatedir/(\d+)$!) {
my $vmid = $1;
return ('rootdir', "$sid:rootdir/$vmid");
- } elsif ($path =~
m!^$backupdir/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!) {
+ } elsif ($path =~
m!^$backupdir/([^/]+\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!) {
my $name = $1;
return ('iso', "$sid:backup/$name");
}
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 0c39cbd..58a801a 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -423,7 +423,7 @@ sub parse_volname {
return ('vztmpl', $1);
} elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
-} elsif ($volname =~
m!^backup/([^/]+(\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo)))$!) {
+} elsif ($volname =~
m!^backup/([^/]+(\.(tgz|((tar|vma)(\.(gz|lzo))?$!) {
my $fn = $1;
if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
return ('backup', $fn, $2);
@@ -910,7 +910,7 @@ my $get_subdir_files = sub {
} elsif ($tt eq 'backup') {
next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
- next if $fn !~
m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
+ next if $fn !~ m!/([^/]+\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!;
$info = { volid => "$sid:backup/$1", format => $2 };
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
