Re: [pve-devel] [PATCH storage] ZFSPoolPlugin: fix #2662 get volume size correctly
On April 7, 2020 11:28 am, Aaron Lauterer wrote:
>
>
> On 4/3/20 5:07 PM, Fabian Grünbichler wrote:
>> there's another instance of 'zfs list ...' in PVE::Storage that could
>> also be switched to '-p'
>
> Which one do you mean? Line 610?
>
>1 if ($format eq 'subvol') {
>
> 610 my $size = $class->zfs_request($scfg, undef, 'list', '-H',
> '-o', 'refquota', "$scfg->{pool}/$basename");
>1 chomp($size);
>
> I don't see much chances for a bug here as the value is used right away
> in the next step but it would definitely be more precise to use the -p flag.
the bug is that the parse_size sub is very broken, so we should get rid
of it ;)
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage] ZFSPoolPlugin: fix #2662 get volume size correctly
On 4/3/20 5:07 PM, Fabian Grünbichler wrote:
there's another instance of 'zfs list ...' in PVE::Storage that could
also be switched to '-p'
Which one do you mean? Line 610?
1 if ($format eq 'subvol') {
610 my $size = $class->zfs_request($scfg, undef, 'list', '-H',
'-o', 'refquota', "$scfg->{pool}/$basename");
1 chomp($size);
I don't see much chances for a bug here as the value is used right away
in the next step but it would definitely be more precise to use the -p flag.
On April 3, 2020 2:29 pm, Aaron Lauterer wrote:
Getting the volume sizes as byte values instead of converted to human
readable units helps to avoid rounding errors in the further processing
if the volume size is more on the odd side.
The `zfs list` command supports the p(arseable) flag since a few years
now.
When returning the size in bytes there is no calculation performed and
thus we need to explicitly cast the size to an integer before returning
it.
Signed-off-by: Aaron Lauterer
---
I don't think we need to worry about other ZFS implementations regarding
ZFS over iSCSI. FreeBSD supports it since 9.3 [0], released mid 2014.
Illumos added it in 2013 [1].
[0]
https://www.freebsd.org/cgi/man.cgi?query=zfs=0=0=FreeBSD+9.3-RELEASE=default=html
[1]
https://github.com/illumos/illumos-gate/commit/43d68d68c1ce08fb35026bebfb141af422e7082e#diff-b138320fc5f9d5c48bb4b03a5e4e4cbb
PVE/Storage/ZFSPoolPlugin.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index b538e3b..cb3f2f0 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -81,7 +81,7 @@ sub zfs_parse_size {
$size = ceil($size);
}
and then the whole zfs_parse_size sub which is completely broken can be
dropped :)
- return $size;
+ return $size + 0;
but untainting/converting to int is still needed for those 4 current
call sites
Will do in V2
}
@@ -400,7 +400,7 @@ sub zfs_delete_zvol {
sub zfs_list_zvol {
my ($class, $scfg) = @_;
-my $text = $class->zfs_request($scfg, 10, 'list', '-o', 'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hr');
+my $text = $class->zfs_request($scfg, 10, 'list', '-o',
'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hrp');
my $zvols = zfs_parse_zvol_list($text);
return undef if !$zvols;
--
2.20.1
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage] ZFSPoolPlugin: fix #2662 get volume size correctly
there's another instance of 'zfs list ...' in PVE::Storage that could
also be switched to '-p'
On April 3, 2020 2:29 pm, Aaron Lauterer wrote:
> Getting the volume sizes as byte values instead of converted to human
> readable units helps to avoid rounding errors in the further processing
> if the volume size is more on the odd side.
>
> The `zfs list` command supports the p(arseable) flag since a few years
> now.
> When returning the size in bytes there is no calculation performed and
> thus we need to explicitly cast the size to an integer before returning
> it.
>
> Signed-off-by: Aaron Lauterer
> ---
>
> I don't think we need to worry about other ZFS implementations regarding
> ZFS over iSCSI. FreeBSD supports it since 9.3 [0], released mid 2014.
> Illumos added it in 2013 [1].
>
> [0]
> https://www.freebsd.org/cgi/man.cgi?query=zfs=0=0=FreeBSD+9.3-RELEASE=default=html
> [1]
> https://github.com/illumos/illumos-gate/commit/43d68d68c1ce08fb35026bebfb141af422e7082e#diff-b138320fc5f9d5c48bb4b03a5e4e4cbb
>
> PVE/Storage/ZFSPoolPlugin.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index b538e3b..cb3f2f0 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -81,7 +81,7 @@ sub zfs_parse_size {
> $size = ceil($size);
> }
and then the whole zfs_parse_size sub which is completely broken can be
dropped :)
>
> - return $size;
> + return $size + 0;
but untainting/converting to int is still needed for those 4 current
call sites
>
> }
>
> @@ -400,7 +400,7 @@ sub zfs_delete_zvol {
> sub zfs_list_zvol {
> my ($class, $scfg) = @_;
>
> -my $text = $class->zfs_request($scfg, 10, 'list', '-o',
> 'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hr');
> +my $text = $class->zfs_request($scfg, 10, 'list', '-o',
> 'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hrp');
> my $zvols = zfs_parse_zvol_list($text);
> return undef if !$zvols;
>
> --
> 2.20.1
>
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
