* [pve-devel] [PATCH-SERIES v2 storage/manager] factoring out RE for backup extension
@ 2021-08-05 7:34 Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions Lorenz Stechauner
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Lorenz Stechauner @ 2021-08-05 7:34 UTC (permalink / raw)
To: pve-devel
changes to v1:
* also renamed iso/vztmpl REs
* new naming schema: $<X>_EXT_RE_<# of capture groups>
for example: $BACKUP_EXT_RE_2
pve-storage:
Lorenz Stechauner (2):
storage: rename REs for iso and vztmpl extensions
storage/plugin: factoring out regex for backup extension re
PVE/API2/Storage/Status.pm | 8 ++++----
PVE/Storage.pm | 23 ++++++++++++++---------
PVE/Storage/Plugin.pm | 12 ++++++------
3 files changed, 24 insertions(+), 19 deletions(-)
pve-manager:
Lorenz Stechauner (1):
api: aplinfo: rename REs for iso and vztmpl extensions
PVE/APLInfo.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions
2021-08-05 7:34 [pve-devel] [PATCH-SERIES v2 storage/manager] factoring out RE for backup extension Lorenz Stechauner
@ 2021-08-05 7:34 ` Lorenz Stechauner
2021-08-13 10:09 ` Fabian Ebner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 manager 1/1] api: aplinfo: rename REs for iso and vztmpl extensions Lorenz Stechauner
2 siblings, 1 reply; 8+ messages in thread
From: Lorenz Stechauner @ 2021-08-05 7:34 UTC (permalink / raw)
To: pve-devel
these changes make it more clear, how many capture groups each
RE inclues.
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/API2/Storage/Status.pm | 8 ++++----
PVE/Storage.pm | 11 +++++++----
PVE/Storage/Plugin.pm | 8 ++++----
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index b838461..3acb647 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -424,12 +424,12 @@ __PACKAGE__->register_method ({
my $path;
if ($content eq 'iso') {
- if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+ if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
- if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
+ if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
@@ -584,12 +584,12 @@ __PACKAGE__->register_method({
my $path;
if ($content eq 'iso') {
- if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+ if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $storage);
} elsif ($content eq 'vztmpl') {
- if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
+ if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index c04b5a2..b5c2460 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -101,9 +101,12 @@ if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
# initialize all plugins
PVE::Storage::Plugin->init();
-our $iso_extension_re = qr/\.(?:iso|img)/i;
+# the following REs indicate the number or capture groups via the trailing digit
+# CAUTION don't forget to update the digits accordingly after messing with the capture groups
-our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
+our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
+
+our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
# PVE::Storage utility functions
@@ -574,10 +577,10 @@ sub path_to_volume_id {
return ('images', $info->{volid});
}
}
- } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
+ } elsif ($path =~ m!^$isodir/([^/]+$ISO_EXT_RE_0)$!) {
my $name = $1;
return ('iso', "$sid:iso/$name");
- } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
+ } elsif ($path =~ m!^$tmpldir/([^/]+$VZTMPL_EXT_RE_1)$!) {
my $name = $1;
return ('vztmpl', "$sid:vztmpl/$name");
} elsif ($path =~ m!^$privatedir/(\d+)$!) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index b1865cb..502951f 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -516,9 +516,9 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
- } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
+ } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
return ('iso', $1);
- } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
+ } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
return ('vztmpl', $1);
} elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
@@ -1045,12 +1045,12 @@ my $get_subdir_files = sub {
my $info;
if ($tt eq 'iso') {
- next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
+ next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i;
$info = { volid => "$sid:iso/$1", format => 'iso' };
} elsif ($tt eq 'vztmpl') {
- next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
+ next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!;
$info = { volid => "$sid:vztmpl/$1", format => "t$2" };
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re
2021-08-05 7:34 [pve-devel] [PATCH-SERIES v2 storage/manager] factoring out RE for backup extension Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions Lorenz Stechauner
@ 2021-08-05 7:34 ` Lorenz Stechauner
2021-08-13 10:10 ` Fabian Ebner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 manager 1/1] api: aplinfo: rename REs for iso and vztmpl extensions Lorenz Stechauner
2 siblings, 1 reply; 8+ messages in thread
From: Lorenz Stechauner @ 2021-08-05 7:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/Storage.pm | 12 +++++++-----
PVE/Storage/Plugin.pm | 4 ++--
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index b5c2460..9bc799d 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -108,6 +108,8 @@ our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
+our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/i;
+
# PVE::Storage utility functions
sub config {
@@ -586,7 +588,7 @@ sub path_to_volume_id {
} elsif ($path =~ m!^$privatedir/(\d+)$!) {
my $vmid = $1;
return ('rootdir', "$sid:rootdir/$vmid");
- } elsif ($path =~ m!^$backupdir/([^/]+\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!) {
+ } elsif ($path =~ m!^$backupdir/([^/]+$BACKUP_EXT_RE_2)$!) {
my $name = $1;
return ('backup', "$sid:backup/$name");
} elsif ($path =~ m!^$snippetsdir/([^/]+)$!) {
@@ -1471,15 +1473,15 @@ sub archive_info {
my $info;
my $volid = basename($archive);
- if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)$/) {
+ if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+$BACKUP_EXT_RE_2)$/) {
my $filename = "$1"; # untaint
- my ($type, $format, $comp) = ($2, $3, $4);
- my $format_re = defined($comp) ? "$format.$comp" : "$format";
+ my ($type, $extension, $comp) = ($2, $3, $4);
+ (my $format = $extension) =~ s/\..*//;
$info = decompressor_info($format, $comp);
$info->{filename} = $filename;
$info->{type} = $type;
- if ($volid =~ /^(vzdump-${type}-([1-9][0-9]{2,8})-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2}))\.${format_re}$/) {
+ if ($volid =~ /^(vzdump-${type}-([1-9][0-9]{2,8})-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2}))\.${extension}$/) {
$info->{logfilename} = "$1.log";
$info->{vmid} = int($2);
$info->{ctime} = timelocal($8, $7, $6, $5, $4 - 1, $3);
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 502951f..b2ffc26 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -522,7 +522,7 @@ sub parse_volname {
return ('vztmpl', $1);
} elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
- } elsif ($volname =~ m!^backup/([^/]+(?:\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\COMPRESSOR_RE}))?))))$!) {
+ } elsif ($volname =~ m!^backup/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!) {
my $fn = $1;
if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
return ('backup', $fn, $2);
@@ -1055,7 +1055,7 @@ my $get_subdir_files = sub {
$info = { volid => "$sid:vztmpl/$1", format => "t$2" };
} elsif ($tt eq 'backup') {
- next if $fn !~ m!/([^/]+\.(tgz|(?:(?:tar|vma)(?:\.(${\COMPRESSOR_RE}))?)))$!;
+ next if $fn !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!;
my $original = $fn;
my $format = $2;
$fn = $1;
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v2 manager 1/1] api: aplinfo: rename REs for iso and vztmpl extensions
2021-08-05 7:34 [pve-devel] [PATCH-SERIES v2 storage/manager] factoring out RE for backup extension Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re Lorenz Stechauner
@ 2021-08-05 7:34 ` Lorenz Stechauner
2 siblings, 0 replies; 8+ messages in thread
From: Lorenz Stechauner @ 2021-08-05 7:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
PVE/APLInfo.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/APLInfo.pm b/PVE/APLInfo.pm
index 5cee1af8..1eff7107 100644
--- a/PVE/APLInfo.pm
+++ b/PVE/APLInfo.pm
@@ -84,7 +84,7 @@ sub read_aplinfo_from_fh {
my $template;
if ($res->{location}) {
$template = $res->{location};
- $template =~ s|.*/([^/]+$PVE::Storage::vztmpl_extension_re)$|$1|;
+ $template =~ s|.*/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$|$1|;
if ($res->{location} !~ m|^([a-zA-Z]+)\://|) {
# relative localtion (no http:// prefix)
$res->{location} = "$source/$res->{location}";
--
2.30.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions Lorenz Stechauner
@ 2021-08-13 10:09 ` Fabian Ebner
2021-08-16 8:46 ` Lorenz Stechauner
0 siblings, 1 reply; 8+ messages in thread
From: Fabian Ebner @ 2021-08-13 10:09 UTC (permalink / raw)
To: pve-devel, l.stechauner
Am 05.08.21 um 09:34 schrieb Lorenz Stechauner:
> these changes make it more clear, how many capture groups each
> RE inclues.
>
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> PVE/API2/Storage/Status.pm | 8 ++++----
> PVE/Storage.pm | 11 +++++++----
> PVE/Storage/Plugin.pm | 8 ++++----
> 3 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index b838461..3acb647 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -424,12 +424,12 @@ __PACKAGE__->register_method ({
> my $path;
>
> if ($content eq 'iso') {
> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> raise_param_exc({ filename => "wrong file extension" });
> }
> $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
> } elsif ($content eq 'vztmpl') {
> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
> raise_param_exc({ filename => "wrong file extension" });
> }
> $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
> @@ -584,12 +584,12 @@ __PACKAGE__->register_method({
>
> my $path;
> if ($content eq 'iso') {
> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> raise_param_exc({ filename => "wrong file extension" });
> }
> $path = PVE::Storage::get_iso_dir($cfg, $storage);
> } elsif ($content eq 'vztmpl') {
> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
> raise_param_exc({ filename => "wrong file extension" });
> }
> $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index c04b5a2..b5c2460 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -101,9 +101,12 @@ if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
> # initialize all plugins
> PVE::Storage::Plugin->init();
>
> -our $iso_extension_re = qr/\.(?:iso|img)/i;
> +# the following REs indicate the number or capture groups via the trailing digit
> +# CAUTION don't forget to update the digits accordingly after messing with the capture groups
>
> -our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
Removing this is not backwards compatible and requires a versioned
Breaks for pve-manager.
> +our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
> +
> +our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
>
> # PVE::Storage utility functions
>
> @@ -574,10 +577,10 @@ sub path_to_volume_id {
> return ('images', $info->{volid});
> }
> }
> - } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
> + } elsif ($path =~ m!^$isodir/([^/]+$ISO_EXT_RE_0)$!) {
> my $name = $1;
> return ('iso', "$sid:iso/$name");
> - } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
> + } elsif ($path =~ m!^$tmpldir/([^/]+$VZTMPL_EXT_RE_1)$!) {
> my $name = $1;
> return ('vztmpl', "$sid:vztmpl/$name");
> } elsif ($path =~ m!^$privatedir/(\d+)$!) {
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index b1865cb..502951f 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -516,9 +516,9 @@ sub parse_volname {
> my ($vmid, $name) = ($1, $2);
> my (undef, $format, $isBase) = parse_name_dir($name);
> return ('images', $name, $vmid, undef, undef, $isBase, $format);
> - } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
> + } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
> return ('iso', $1);
> - } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
> + } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
> return ('vztmpl', $1);
> } elsif ($volname =~ m!^rootdir/(\d+)$!) {
> return ('rootdir', $1, $1);
> @@ -1045,12 +1045,12 @@ my $get_subdir_files = sub {
> my $info;
>
> if ($tt eq 'iso') {
> - next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
> + next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i;
>
> $info = { volid => "$sid:iso/$1", format => 'iso' };
>
> } elsif ($tt eq 'vztmpl') {
> - next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
> + next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!;
>
> $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re Lorenz Stechauner
@ 2021-08-13 10:10 ` Fabian Ebner
0 siblings, 0 replies; 8+ messages in thread
From: Fabian Ebner @ 2021-08-13 10:10 UTC (permalink / raw)
To: pve-devel, l.stechauner
Am 05.08.21 um 09:34 schrieb Lorenz Stechauner:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> PVE/Storage.pm | 12 +++++++-----
> PVE/Storage/Plugin.pm | 4 ++--
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index b5c2460..9bc799d 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -108,6 +108,8 @@ our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
>
> our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
>
> +our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/i;
Please mention in the commit message that this now matches
case-insensitively, which is different from the current behavior.
But matching case-insensitively doesn't work, because at least
decompressor_info() cannot handle uppercase extensions currently.
> +
> # PVE::Storage utility functions
>
> sub config {
> @@ -586,7 +588,7 @@ sub path_to_volume_id {
> } elsif ($path =~ m!^$privatedir/(\d+)$!) {
> my $vmid = $1;
> return ('rootdir', "$sid:rootdir/$vmid");
> - } elsif ($path =~ m!^$backupdir/([^/]+\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!) {
> + } elsif ($path =~ m!^$backupdir/([^/]+$BACKUP_EXT_RE_2)$!) {
> my $name = $1;
> return ('backup', "$sid:backup/$name");
> } elsif ($path =~ m!^$snippetsdir/([^/]+)$!) {
> @@ -1471,15 +1473,15 @@ sub archive_info {
> my $info;
>
> my $volid = basename($archive);
> - if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)$/) {
> + if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+$BACKUP_EXT_RE_2)$/) {
> my $filename = "$1"; # untaint
> - my ($type, $format, $comp) = ($2, $3, $4);
> - my $format_re = defined($comp) ? "$format.$comp" : "$format";
> + my ($type, $extension, $comp) = ($2, $3, $4);
> + (my $format = $extension) =~ s/\..*//;
> $info = decompressor_info($format, $comp);
> $info->{filename} = $filename;
> $info->{type} = $type;
>
> - if ($volid =~ /^(vzdump-${type}-([1-9][0-9]{2,8})-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2}))\.${format_re}$/) {
> + if ($volid =~ /^(vzdump-${type}-([1-9][0-9]{2,8})-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2}))\.${extension}$/) {
> $info->{logfilename} = "$1.log";
> $info->{vmid} = int($2);
> $info->{ctime} = timelocal($8, $7, $6, $5, $4 - 1, $3);
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 502951f..b2ffc26 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -522,7 +522,7 @@ sub parse_volname {
> return ('vztmpl', $1);
> } elsif ($volname =~ m!^rootdir/(\d+)$!) {
> return ('rootdir', $1, $1);
> - } elsif ($volname =~ m!^backup/([^/]+(?:\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\COMPRESSOR_RE}))?))))$!) {
> + } elsif ($volname =~ m!^backup/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!) {
> my $fn = $1;
> if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
> return ('backup', $fn, $2);
> @@ -1055,7 +1055,7 @@ my $get_subdir_files = sub {
> $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
>
> } elsif ($tt eq 'backup') {
> - next if $fn !~ m!/([^/]+\.(tgz|(?:(?:tar|vma)(?:\.(${\COMPRESSOR_RE}))?)))$!;
> + next if $fn !~ m!/([^/]+$PVE::Storage::BACKUP_EXT_RE_2)$!;
> my $original = $fn;
> my $format = $2;
> $fn = $1;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions
2021-08-13 10:09 ` Fabian Ebner
@ 2021-08-16 8:46 ` Lorenz Stechauner
2021-09-01 10:23 ` Fabian Ebner
0 siblings, 1 reply; 8+ messages in thread
From: Lorenz Stechauner @ 2021-08-16 8:46 UTC (permalink / raw)
To: Fabian Ebner, pve-devel
On 13.08.21 12:09, Fabian Ebner wrote:
> Am 05.08.21 um 09:34 schrieb Lorenz Stechauner:
>> these changes make it more clear, how many capture groups each
>> RE inclues.
>>
>> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
>> ---
>> PVE/API2/Storage/Status.pm | 8 ++++----
>> PVE/Storage.pm | 11 +++++++----
>> PVE/Storage/Plugin.pm | 8 ++++----
>> 3 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
>> index b838461..3acb647 100644
>> --- a/PVE/API2/Storage/Status.pm
>> +++ b/PVE/API2/Storage/Status.pm
>> @@ -424,12 +424,12 @@ __PACKAGE__->register_method ({
>> my $path;
>> if ($content eq 'iso') {
>> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
>> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
>> raise_param_exc({ filename => "wrong file extension" });
>> }
>> $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
>> } elsif ($content eq 'vztmpl') {
>> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
>> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
>> raise_param_exc({ filename => "wrong file extension" });
>> }
>> $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
>> @@ -584,12 +584,12 @@ __PACKAGE__->register_method({
>> my $path;
>> if ($content eq 'iso') {
>> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
>> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
>> raise_param_exc({ filename => "wrong file extension" });
>> }
>> $path = PVE::Storage::get_iso_dir($cfg, $storage);
>> } elsif ($content eq 'vztmpl') {
>> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
>> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
>> raise_param_exc({ filename => "wrong file extension" });
>> }
>> $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>> index c04b5a2..b5c2460 100755
>> --- a/PVE/Storage.pm
>> +++ b/PVE/Storage.pm
>> @@ -101,9 +101,12 @@ if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
>> # initialize all plugins
>> PVE::Storage::Plugin->init();
>> -our $iso_extension_re = qr/\.(?:iso|img)/i;
>> +# the following REs indicate the number or capture groups via the
>> trailing digit
>> +# CAUTION don't forget to update the digits accordingly after
>> messing with the capture groups
>> -our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
>
> Removing this is not backwards compatible and requires a versioned
> Breaks for pve-manager.
makes sense. should I split this commit in two, where in the first part
I add the new variable and in the second part remove the old one? or is
it enough to state the need for "breaks" in the commit message?
>
>> +our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
>> +
>> +our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
>> # PVE::Storage utility functions
>> @@ -574,10 +577,10 @@ sub path_to_volume_id {
>> return ('images', $info->{volid});
>> }
>> }
>> - } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
>> + } elsif ($path =~ m!^$isodir/([^/]+$ISO_EXT_RE_0)$!) {
>> my $name = $1;
>> return ('iso', "$sid:iso/$name");
>> - } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
>> + } elsif ($path =~ m!^$tmpldir/([^/]+$VZTMPL_EXT_RE_1)$!) {
>> my $name = $1;
>> return ('vztmpl', "$sid:vztmpl/$name");
>> } elsif ($path =~ m!^$privatedir/(\d+)$!) {
>> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
>> index b1865cb..502951f 100644
>> --- a/PVE/Storage/Plugin.pm
>> +++ b/PVE/Storage/Plugin.pm
>> @@ -516,9 +516,9 @@ sub parse_volname {
>> my ($vmid, $name) = ($1, $2);
>> my (undef, $format, $isBase) = parse_name_dir($name);
>> return ('images', $name, $vmid, undef, undef, $isBase, $format);
>> - } elsif ($volname =~
>> m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
>> + } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
>> return ('iso', $1);
>> - } elsif ($volname =~
>> m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
>> + } elsif ($volname =~
>> m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
>> return ('vztmpl', $1);
>> } elsif ($volname =~ m!^rootdir/(\d+)$!) {
>> return ('rootdir', $1, $1);
>> @@ -1045,12 +1045,12 @@ my $get_subdir_files = sub {
>> my $info;
>> if ($tt eq 'iso') {
>> - next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
>> + next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i;
>> $info = { volid => "$sid:iso/$1", format => 'iso' };
>> } elsif ($tt eq 'vztmpl') {
>> - next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
>> + next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!;
>> $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions
2021-08-16 8:46 ` Lorenz Stechauner
@ 2021-09-01 10:23 ` Fabian Ebner
0 siblings, 0 replies; 8+ messages in thread
From: Fabian Ebner @ 2021-09-01 10:23 UTC (permalink / raw)
To: Lorenz Stechauner, pve-devel
Am 16.08.21 um 10:46 schrieb Lorenz Stechauner:
>
> On 13.08.21 12:09, Fabian Ebner wrote:
>> Am 05.08.21 um 09:34 schrieb Lorenz Stechauner:
>>> these changes make it more clear, how many capture groups each
>>> RE inclues.
>>>
>>> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
>>> ---
>>> PVE/API2/Storage/Status.pm | 8 ++++----
>>> PVE/Storage.pm | 11 +++++++----
>>> PVE/Storage/Plugin.pm | 8 ++++----
>>> 3 files changed, 15 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
>>> index b838461..3acb647 100644
>>> --- a/PVE/API2/Storage/Status.pm
>>> +++ b/PVE/API2/Storage/Status.pm
>>> @@ -424,12 +424,12 @@ __PACKAGE__->register_method ({
>>> my $path;
>>> if ($content eq 'iso') {
>>> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
>>> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
>>> raise_param_exc({ filename => "wrong file extension" });
>>> }
>>> $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
>>> } elsif ($content eq 'vztmpl') {
>>> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
>>> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
>>> raise_param_exc({ filename => "wrong file extension" });
>>> }
>>> $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
>>> @@ -584,12 +584,12 @@ __PACKAGE__->register_method({
>>> my $path;
>>> if ($content eq 'iso') {
>>> - if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
>>> + if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
>>> raise_param_exc({ filename => "wrong file extension" });
>>> }
>>> $path = PVE::Storage::get_iso_dir($cfg, $storage);
>>> } elsif ($content eq 'vztmpl') {
>>> - if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
>>> + if ($filename !~ m![^/]+$PVE::Storage::VZTMPL_EXT_RE_1$!) {
>>> raise_param_exc({ filename => "wrong file extension" });
>>> }
>>> $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
>>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>>> index c04b5a2..b5c2460 100755
>>> --- a/PVE/Storage.pm
>>> +++ b/PVE/Storage.pm
>>> @@ -101,9 +101,12 @@ if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
>>> # initialize all plugins
>>> PVE::Storage::Plugin->init();
>>> -our $iso_extension_re = qr/\.(?:iso|img)/i;
>>> +# the following REs indicate the number or capture groups via the
>>> trailing digit
>>> +# CAUTION don't forget to update the digits accordingly after
>>> messing with the capture groups
>>> -our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
>>
>> Removing this is not backwards compatible and requires a versioned
>> Breaks for pve-manager.
> makes sense. should I split this commit in two, where in the first part
> I add the new variable and in the second part remove the old one? or is
> it enough to state the need for "breaks" in the commit message?
If we want to do the Breaks now, it's enough to mention it. I don't
think you need to split the patch.
If we don't want to do the Breaks now, adding a reminder comment like
# FIXME: Remove with PVE 8.0 and add a versioned breaks for pve-manager
is probably the way to go.
And which of those we want, is for the release team to decide.
>>
>>> +our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i;
>>> +
>>> +our $VZTMPL_EXT_RE_1 = qr/\.tar\.([gx]z)/i;
>>> # PVE::Storage utility functions
>>> @@ -574,10 +577,10 @@ sub path_to_volume_id {
>>> return ('images', $info->{volid});
>>> }
>>> }
>>> - } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
>>> + } elsif ($path =~ m!^$isodir/([^/]+$ISO_EXT_RE_0)$!) {
>>> my $name = $1;
>>> return ('iso', "$sid:iso/$name");
>>> - } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
>>> + } elsif ($path =~ m!^$tmpldir/([^/]+$VZTMPL_EXT_RE_1)$!) {
>>> my $name = $1;
>>> return ('vztmpl', "$sid:vztmpl/$name");
>>> } elsif ($path =~ m!^$privatedir/(\d+)$!) {
>>> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
>>> index b1865cb..502951f 100644
>>> --- a/PVE/Storage/Plugin.pm
>>> +++ b/PVE/Storage/Plugin.pm
>>> @@ -516,9 +516,9 @@ sub parse_volname {
>>> my ($vmid, $name) = ($1, $2);
>>> my (undef, $format, $isBase) = parse_name_dir($name);
>>> return ('images', $name, $vmid, undef, undef, $isBase, $format);
>>> - } elsif ($volname =~
>>> m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
>>> + } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
>>> return ('iso', $1);
>>> - } elsif ($volname =~
>>> m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
>>> + } elsif ($volname =~
>>> m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {
>>> return ('vztmpl', $1);
>>> } elsif ($volname =~ m!^rootdir/(\d+)$!) {
>>> return ('rootdir', $1, $1);
>>> @@ -1045,12 +1045,12 @@ my $get_subdir_files = sub {
>>> my $info;
>>> if ($tt eq 'iso') {
>>> - next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
>>> + next if $fn !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i;
>>> $info = { volid => "$sid:iso/$1", format => 'iso' };
>>> } elsif ($tt eq 'vztmpl') {
>>> - next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
>>> + next if $fn !~ m!/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!;
>>> $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
>>>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-01 10:23 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 7:34 [pve-devel] [PATCH-SERIES v2 storage/manager] factoring out RE for backup extension Lorenz Stechauner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 1/2] storage: rename REs for iso and vztmpl extensions Lorenz Stechauner
2021-08-13 10:09 ` Fabian Ebner
2021-08-16 8:46 ` Lorenz Stechauner
2021-09-01 10:23 ` Fabian Ebner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 storage 2/2] storage/plugin: factoring out regex for backup extension re Lorenz Stechauner
2021-08-13 10:10 ` Fabian Ebner
2021-08-05 7:34 ` [pve-devel] [PATCH v2 manager 1/1] api: aplinfo: rename REs for iso and vztmpl extensions Lorenz Stechauner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal