[pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups

[pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups

[Fabian Grünbichler]

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

[pve-devel] [PATCH storage v2 1/3] storage: plugin: return 'raw' format when parsing non-image volumes

[Fabian Grünbichler]

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

[pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes

[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 ;
+    }
 
     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

[pve-devel] [PATCH storage v2 3/3] file_size_info: add warning when falling back to raw format

[Fabian Grünbichler]

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

Re: [pve-devel] [PATCH storage v2 2/3] esxi: fix return value of volume_size_info for vmx volumes

[Fiona Ebner]

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

Re: [pve-devel] [PATCH storage v2 0/3] file_size_info follow-ups

[Fiona Ebner]

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

[pve-devel] applied-series: [PATCH storage v2 0/3] file_size_info follow-ups

[Fabian Grünbichler]

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