* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox