* [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
* 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 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
* [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
* 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
* [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
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