Re: [pve-devel] [PATCH installer 1/3] low-level: add zfs module for retrieving importable zpool info

[Aaron Lauterer <a.lauterer@proxmox.com>]

On  2024-05-16  12:28, Christoph Heiss wrote:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>   Proxmox/Makefile          |  1 +
>   Proxmox/Sys/ZFS.pm        | 43 ++++++++++++++++++++++++++++++++++
>   test/Makefile             |  6 +++++
>   test/zfs-get-pool-list.pl | 49 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 99 insertions(+)
>   create mode 100644 Proxmox/Sys/ZFS.pm
>   create mode 100755 test/zfs-get-pool-list.pl
> 
> diff --git a/Proxmox/Makefile b/Proxmox/Makefile
> index 9561d9b..035626b 100644
> --- a/Proxmox/Makefile
> +++ b/Proxmox/Makefile
> @@ -17,6 +17,7 @@ PERL_MODULES=\
>       Sys/File.pm \
>       Sys/Net.pm \
>       Sys/Udev.pm \
> +    Sys/ZFS.pm \
>       UI.pm \
>       UI/Base.pm \
>       UI/Gtk3.pm \
> diff --git a/Proxmox/Sys/ZFS.pm b/Proxmox/Sys/ZFS.pm
> new file mode 100644
> index 0000000..4c732ca
> --- /dev/null
> +++ b/Proxmox/Sys/ZFS.pm
> @@ -0,0 +1,43 @@
> +package Proxmox::Sys::ZFS;
> +
> +use strict;
> +use warnings;
> +
> +use Proxmox::Sys::Command qw(run_command);
> +
> +use base qw(Exporter);
> +our @EXPORT_OK = qw(get_exported_pools);
> +

Some of the flow in this function is difficult to understand without 
having a sample of the text it is parsing. Could we have a small 
example, maybe added as comment?
That could help people to see what is it trying to parse, even if they 
are not too familiar with the expected output

> +my sub parse_pool_list {
> +    my ($fh) = @_;
> +
> +    my @pools;
> +    my $pool = {}; # last found pool in output
> +
> +    while (my $line = <$fh>) {
> +	if ($line =~ /^\s+pool: (.+)$/) {
> +	    push @pools, $pool if %$pool;
> +	    $pool = { name => $1 };
> +	    next;
> +	}
> +
> +	next if !%$pool;
> +
> +	if ($line =~ /^\s*(id|state|status|action): (.+)$/) {
> +	    chomp($pool->{$1} = $2);
> +	    next;
> +	}
> +    }
> +
> +    push @pools, $pool if %$pool;
> +    return \@pools;

not too sure, but we usually tend to use anonymous arrays, $pools = [];
then we could just return $pools
The downside is of course that we need to dereference it in all the 
other places, AFAICT all the `push` lines:
push @$pools ...

But IME this is more in line with how usually handle such code.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel