* [pve-devel] [PATCH storage 0/3] file_size_info follow-ups
@ 2024-12-10 11:19 Fabian Grünbichler
2024-12-10 11:19 ` [pve-devel] [PATCH storage 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2024-12-10 11:19 UTC (permalink / raw)
To: pve-devel
these fix various corner cases / spurious warnings for the recently
added file_size_info checks.
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
storage: plugin: volume_size_info: fallback to raw for non-image
volumes
src/PVE/Storage/ESXiPlugin.pm | 4 +++-
src/PVE/Storage/Plugin.pm | 18 ++++++++++++------
src/test/parse_volname_test.pm | 22 +++++++++++++---------
3 files changed, 28 insertions(+), 16 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] 5+ messages in thread* [pve-devel] [PATCH storage 1/3] storage: plugin: return 'raw' format when parsing non-image volumes 2024-12-10 11:19 [pve-devel] [PATCH storage 0/3] file_size_info follow-ups Fabian Grünbichler @ 2024-12-10 11:19 ` Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [RFC storage 3/3] storage: plugin: volume_size_info: fallback to raw for non-image volumes Fabian Grünbichler 2 siblings, 0 replies; 5+ messages in thread From: Fabian Grünbichler @ 2024-12-10 11:19 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> --- 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] 5+ messages in thread
* [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes 2024-12-10 11:19 [pve-devel] [PATCH storage 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler @ 2024-12-10 11:19 ` Fabian Grünbichler 2024-12-10 11:56 ` Fiona Ebner 2024-12-10 11:19 ` [pve-devel] [RFC storage 3/3] storage: plugin: volume_size_info: fallback to raw for non-image volumes Fabian Grünbichler 2 siblings, 1 reply; 5+ messages in thread From: Fabian Grünbichler @ 2024-12-10 11:19 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> --- 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..609f3f9 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 ? ('vmx', 0) : 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] 5+ messages in thread
* Re: [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes 2024-12-10 11:19 ` [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler @ 2024-12-10 11:56 ` Fiona Ebner 0 siblings, 0 replies; 5+ messages in thread From: Fiona Ebner @ 2024-12-10 11:56 UTC (permalink / raw) To: Proxmox VE development discussion, Fabian Grünbichler Am 10.12.24 um 12:19 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> > --- > 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..609f3f9 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 ? ('vmx', 0) : 0 ; The order is wrong. It is ($size, $format, $used, $parent, $ctime) > + } > > 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] 5+ messages in thread
* [pve-devel] [RFC storage 3/3] storage: plugin: volume_size_info: fallback to raw for non-image volumes 2024-12-10 11:19 [pve-devel] [PATCH storage 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler @ 2024-12-10 11:19 ` Fabian Grünbichler 2 siblings, 0 replies; 5+ messages in thread From: Fabian Grünbichler @ 2024-12-10 11:19 UTC (permalink / raw) To: pve-devel as a safeguard for external storage plugins that - override parse_volname and return "weird" formats - but don't override volume_size_info to prevent them from running into new file_size_info checks that only work for certain formats we know about. Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> --- not sure if such plugins even exist? but the warning might help us find out ;) src/PVE/Storage/Plugin.pm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 5875553..1028cc1 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -1129,7 +1129,13 @@ sub update_volume_attribute { sub volume_size_info { my ($class, $scfg, $storeid, $volname, $timeout) = @_; - my $format = ($class->parse_volname($volname))[6]; + my ($vtype, $format) = ($class->parse_volname($volname))[0,6]; + + if ($vtype ne 'images' && $format && $format ne 'raw') { + warn "volume_size_info: '$storeid:$volname': falling back from format '$format' to 'raw'\n"; + $format = 'raw'; + } + my $path = $class->filesystem_path($scfg, $volname); return file_size_info($path, $timeout, $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] 5+ messages in thread
end of thread, other threads:[~2024-12-10 11:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-12-10 11:19 [pve-devel] [PATCH storage 0/3] file_size_info follow-ups Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 1/3] storage: plugin: return 'raw' format when parsing non-image volumes Fabian Grünbichler 2024-12-10 11:19 ` [pve-devel] [PATCH storage 2/3] esxi: fix return value of volume_size_info for vmx volumes Fabian Grünbichler 2024-12-10 11:56 ` Fiona Ebner 2024-12-10 11:19 ` [pve-devel] [RFC storage 3/3] storage: plugin: volume_size_info: fallback to raw for non-image volumes Fabian Grünbichler
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.