* [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups
@ 2024-12-10 12:17 Fabian Grünbichler
2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2024-12-10 12:17 UTC (permalink / raw)
To: pve-devel
these fix various corner cases / spurious warnings for the recently
added file_size_info checks.
changes from v2:
- fix esxi return value order
- replace third patch with new one just adding a warning to the existing
fallback mechanism that already covers this
Fabian Grünbichler (3):
storage: plugin: return 'raw' format when parsing non-image volumes
esxi: fix return value of volume_size_info for vmx volumes
file_size_info: add warning when falling back to raw format
src/PVE/Storage/ESXiPlugin.pm | 4 +++-
src/PVE/Storage/Plugin.pm | 16 +++++++++-------
src/test/parse_volname_test.pm | 22 +++++++++++++---------
3 files changed, 25 insertions(+), 17 deletions(-)
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* [pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes 2024-12-10 12:17 [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fabian Grünbichler @ 2024-12-10 12:17 ` Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Fabian Grünbichler @ 2024-12-10 12:17 UTC (permalink / raw) To: pve-devel since `volume_size_info` passes the parsed format to `file_size_info`, which prints a warning if the format is undef before falling back to auto-detection, and these should always be treated as raw files anyway. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- v2: unchanged src/PVE/Storage/Plugin.pm | 10 +++++----- src/test/parse_volname_test.pm | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index ae3c9dc..5875553 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -650,19 +650,19 @@ sub parse_volname { my (undef, $format, $isBase) = parse_name_dir($name); return ('images', $name, $vmid, undef, undef, $isBase, $format); } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) { - return ('iso', $1); + return ('iso', $1, undef, undef, undef, undef, 'raw'); } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) { - return ('vztmpl', $1); + return ('vztmpl', $1, undef, undef, undef, undef, 'raw'); } elsif ($volname =~ m!^rootdir/(\d+)$!) { return ('rootdir', $1, $1); } 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); + return ('backup', $fn, $2, undef, undef, undef, 'raw'); } - return ('backup', $fn); + return ('backup', $fn, undef, undef, undef, undef, 'raw'); } elsif ($volname =~ m!^snippets/([^/]+)$!) { - return ('snippets', $1); + return ('snippets', $1, undef, undef, undef, undef, 'raw'); } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.ova\/${PVE::Storage::OVA_CONTENT_RE_1})$!) { my $packed_image = $1; my $format = $2; diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm index 2739cf1..175500d 100644 --- a/src/test/parse_volname_test.pm +++ b/src/test/parse_volname_test.pm @@ -29,12 +29,12 @@ my $tests = [ { description => 'ISO image, iso', volname => 'iso/some-installation-disk.iso', - expected => ['iso', 'some-installation-disk.iso'], + expected => ['iso', 'some-installation-disk.iso', undef, undef, undef, undef, 'raw'], }, { description => 'ISO image, img', volname => 'iso/some-other-installation-disk.img', - expected => ['iso', 'some-other-installation-disk.img'], + expected => ['iso', 'some-other-installation-disk.img', undef, undef, undef, undef, 'raw'], }, # # container templates @@ -42,17 +42,17 @@ my $tests = [ { description => 'Container template tar.gz', volname => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.gz', - expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.gz'], + expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.gz', undef, undef, undef, undef, 'raw'], }, { description => 'Container template tar.xz', volname => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz', - expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.xz'], + expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.xz', undef, undef, undef, undef, 'raw'], }, { description => 'Container template tar.bz2', volname => 'vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2', - expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.bz2'], + expected => ['vztmpl', 'debian-10.0-standard_10.0-1_amd64.tar.bz2', undef, undef, undef, undef, 'raw'], }, # # container rootdir @@ -70,7 +70,7 @@ my $tests = [ { description => 'Backup archive, no virtualization type', volname => "backup/vzdump-none-$vmid-2020_03_30-21_39_30.tar", - expected => ['backup', "vzdump-none-$vmid-2020_03_30-21_39_30.tar"], + expected => ['backup', "vzdump-none-$vmid-2020_03_30-21_39_30.tar", undef, undef, undef, undef, 'raw'], }, # # Snippets @@ -78,12 +78,12 @@ my $tests = [ { description => 'Snippets, yaml', volname => 'snippets/userconfig.yaml', - expected => ['snippets', 'userconfig.yaml'], + expected => ['snippets', 'userconfig.yaml', undef, undef, undef, undef, 'raw'], }, { description => 'Snippets, perl', volname => 'snippets/hookscript.pl', - expected => ['snippets', 'hookscript.pl'], + expected => ['snippets', 'hookscript.pl', undef, undef, undef, undef, 'raw'], }, # # Import @@ -229,7 +229,11 @@ foreach my $virt (keys %$bkp_suffix) { expected => [ 'backup', "vzdump-$virt-$vmid-2020_03_30-21_12_40.$s", - "$vmid" + "$vmid", + undef, + undef, + undef, + 'raw' ], }, ); -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes 2024-12-10 12:17 [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler @ 2024-12-10 12:17 ` Fabian Grünbichler 2024-12-10 12:34 ` Fiona Ebner 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 3/3] file_size_info: add warning when falling back to raw format Fabian Grünbichler 2024-12-10 12:35 ` [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fiona Ebner 3 siblings, 1 reply; 7+ messages in thread From: Fabian Grünbichler @ 2024-12-10 12:17 UTC (permalink / raw) To: pve-devel in case of an array context, it should also return the format, else a caller might assume it failed. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- v2: fix return value ordering, thanks Fiona! src/PVE/Storage/ESXiPlugin.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm index 37f9e75..70735f9 100644 --- a/src/PVE/Storage/ESXiPlugin.pm +++ b/src/PVE/Storage/ESXiPlugin.pm @@ -535,7 +535,9 @@ sub volume_resize { sub volume_size_info { my ($class, $scfg, $storeid, $volname, $timeout) = @_; - return 0 if $volname =~ /\.vmx$/; + if ($volname =~ /\.vmx$/) { + return wantarray ? (0, 'vmx') : 0 ; + } my $filename = $class->path($scfg, $volname, $storeid, undef); return PVE::Storage::Plugin::file_size_info($filename, $timeout, 'auto-detect'); -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler @ 2024-12-10 12:34 ` Fiona Ebner 0 siblings, 0 replies; 7+ messages in thread From: Fiona Ebner @ 2024-12-10 12:34 UTC (permalink / raw) To: Proxmox VE development discussion, Fabian Grünbichler Am 10.12.24 um 13:17 schrieb Fabian Grünbichler: > in case of an array context, it should also return the format, else a caller > might assume it failed. > > Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> > --- > v2: fix return value ordering, thanks Fiona! > > src/PVE/Storage/ESXiPlugin.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm > index 37f9e75..70735f9 100644 > --- a/src/PVE/Storage/ESXiPlugin.pm > +++ b/src/PVE/Storage/ESXiPlugin.pm > @@ -535,7 +535,9 @@ sub volume_resize { > sub volume_size_info { > my ($class, $scfg, $storeid, $volname, $timeout) = @_; > > - return 0 if $volname =~ /\.vmx$/; > + if ($volname =~ /\.vmx$/) { > + return wantarray ? (0, 'vmx') : 0 ; Style nit: space before semicolon > + } > > my $filename = $class->path($scfg, $volname, $storeid, undef); > return PVE::Storage::Plugin::file_size_info($filename, $timeout, 'auto-detect'); _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH storage v2 3/3] file_size_info: add warning when falling back to raw format 2024-12-10 12:17 [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler @ 2024-12-10 12:17 ` Fabian Grünbichler 2024-12-10 12:35 ` [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fiona Ebner 3 siblings, 0 replies; 7+ messages in thread From: Fabian Grünbichler @ 2024-12-10 12:17 UTC (permalink / raw) To: pve-devel in case this gets called with an explicit format that is none of: - 'auto-detect' - 'subvol' - a member of the list of known "qemu" formats this should only affect third-party storage plugins that either call this directly with a format, or via inherited code that gets a format from parse_volname and passes it to file_size_info. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- v2: new, replacing old patch #3 src/PVE/Storage/Plugin.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 5875553..6ee6938 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -1012,8 +1012,10 @@ sub file_size_info { # TODO PVE 9 - consider upgrading to "die" if an unsupported format is passed in after # evaluating breakage potential. - $file_format = 'raw' if $file_format && !grep { $_ eq $file_format } @checked_qemu_img_formats; - + if ($file_format && !grep { $_ eq $file_format } @checked_qemu_img_formats) { + warn "file_size_info: '$filename': falling back to 'raw' from unknown format '$file_format'\n"; + $file_format = 'raw'; + } my $cmd = ['/usr/bin/qemu-img', 'info', '--output=json', $filename]; push $cmd->@*, '-f', $file_format if $file_format; -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups 2024-12-10 12:17 [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fabian Grünbichler ` (2 preceding siblings ...) 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 3/3] file_size_info: add warning when falling back to raw format Fabian Grünbichler @ 2024-12-10 12:35 ` Fiona Ebner 2024-12-10 13:35 ` [pve-devel] applied-series: " Fabian Grünbichler 3 siblings, 1 reply; 7+ messages in thread From: Fiona Ebner @ 2024-12-10 12:35 UTC (permalink / raw) To: Proxmox VE development discussion, Fabian Grünbichler Am 10.12.24 um 13:17 schrieb Fabian Grünbichler: > these fix various corner cases / spurious warnings for the recently > added file_size_info checks. > Thanks for the follow-ups! consider the series: Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] applied-series: [PATCH storage v2 0/3] file_size_info follow-ups 2024-12-10 12:35 ` [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fiona Ebner @ 2024-12-10 13:35 ` Fabian Grünbichler 0 siblings, 0 replies; 7+ messages in thread From: Fabian Grünbichler @ 2024-12-10 13:35 UTC (permalink / raw) To: Fiona Ebner, Proxmox VE development discussion with your R-B added and the style nit folded in. On December 10, 2024 1:35 pm, Fiona Ebner wrote: > Am 10.12.24 um 13:17 schrieb Fabian Grünbichler: >> these fix various corner cases / spurious warnings for the recently >> added file_size_info checks. >> > > Thanks for the follow-ups! consider the series: > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-12-10 13:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-12-10 12:17 [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler 2024-12-10 12:34 ` Fiona Ebner 2024-12-10 12:17 ` [pve-devel] [PATCH storage v2 3/3] file_size_info: add warning when falling back to raw format Fabian Grünbichler 2024-12-10 12:35 ` [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups Fiona Ebner 2024-12-10 13:35 ` [pve-devel] applied-series: " Fabian Grünbichler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox