[pve-devel] [PATCH installer] show multiple disks in ack screen when zfs is used
Thomas Lamprecht
t.lamprecht at proxmox.com
Thu Jan 10 18:09:04 CET 2019
On 1/10/19 11:54 AM, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
result looks OK, some coding (style) nits inline:
> ---
> proxinstall | 51 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/proxinstall b/proxinstall
> index ae4d1fe..1dfe84c 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -2187,6 +2187,26 @@ sub create_ipconf_view {
> $hostentry->grab_focus();
> }
>
> +my $get_raid_devlist = sub {
> +
> + my $dev_name_hash = {};
> +
> + my $devlist = [];
> + for (my $i = 0; $i < @$hds; $i++) {
> + if (my $hd = $config_options->{"disksel$i"}) {
> + my ($disk, $devname, $size, $model) = @$hd;
> + die "device '$devname' is used more than once\n"
> + if $dev_name_hash->{$devname};
> + $dev_name_hash->{$devname} = $hd;
> + push @$devlist, $hd;
> + }
> + }
> +
> + return $devlist;
> +};
> +
> +
> +
> sub create_ack_view {
>
> cleanup_view();
> @@ -2195,7 +2215,7 @@ sub create_ack_view {
> my $ack_html = "${proxmox_libdir}/html/$steps[$step_number]->{html}";
> my $html_data = file_get_contents($ack_template);
>
> - my %config_values = (
> + my $config_values = {
> __target_hd__ => $target_hd,
> __target_fs__ => $config_options->{filesys},
> __country__ => $cmap->{country}->{$country}->{name},
> @@ -2208,9 +2228,16 @@ sub create_ack_view {
> __netmask__ => $netmask,
> __gateway__ => $gateway,
> __dnsserver__ => $dnsserver,
> - );
> + };
the above change from an hash to a hashref is not directly related to
the logical change of this patch and adds a bit noise.
>
> - while ( my ($k, $v) = each %config_values) {
> + if ($config_options->{filesys} =~ m/zfs/) {
for completeness sake I'd do:
=~ /zfs|btrfs/
> + my $raid_devlist = &$get_raid_devlist();
indent whitespace errors below
> + my @raid_disks = map { $_->[1] } @{$raid_devlist};
> + my $used_disks = join(' / ', @raid_disks);
> + $config_values->{__target_hd__} = $used_disks;
> + }
> +
> + while ( my ($k, $v) = each %{$config_values}) {
hmm, maybe it would be easier to do something akin to:
(only quickly tested)
----8<----
diff --git a/proxinstall b/proxinstall
index ae4d1fe..8b41bfe 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2196,7 +2196,7 @@ sub create_ack_view {
my $html_data = file_get_contents($ack_template);
my %config_values = (
- __target_hd__ => $target_hd,
+ __target_hd__ => join(' | ', @{$config_options->{target_hds}}),
__target_fs__ => $config_options->{filesys},
__country__ => $cmap->{country}->{$country}->{name},
__timezone__ => $timezone,
@@ -3076,24 +3076,27 @@ sub create_hdsel_view {
set_next(undef, sub {
if ($config_options->{filesys} =~ m/zfs/) {
- eval { get_zfs_raid_setup(); };
+ my ($devlist) = eval { get_zfs_raid_setup() };
if (my $err = $@) {
display_message("Warning: $err\n" .
"Please fix ZFS setup first.");
} else {
+ $config_options->{target_hds} = [ map { $_->[1] } @$devlist ];
$step_number++;
create_country_view();
}
} elsif ($config_options->{filesys} =~ m/btrfs/) {
- eval { get_btrfs_raid_setup(); };
+ my ($devlist) = eval { get_btrfs_raid_setup() };
if (my $err = $@) {
display_message("Warning: $err\n" .
"Please fix BTRFS setup first.");
} else {
+ $config_options->{target_hds} = [ map { $_->[1] } @$devlist ];
$step_number++;
create_country_view();
}
} else {
+ $config_options->{target_hds} = [ $target_hd ];
$step_number++;
create_country_view();
}
--
reuses get_*raid_setup calls already present and the ack screen does not need to
have file-system type depend logic (less coupling). What do you think?
> $html_data =~ s/$k/$v/g;
> }
>
> @@ -2917,24 +2944,6 @@ sub create_hdoption_view {
> $dialog->destroy();
> }
>
> -my $get_raid_devlist = sub {
> -
> - my $dev_name_hash = {};
> -
> - my $devlist = [];
> - for (my $i = 0; $i < @$hds; $i++) {
> - if (my $hd = $config_options->{"disksel$i"}) {
> - my ($disk, $devname, $size, $model) = @$hd;
> - die "device '$devname' is used more than once\n"
> - if $dev_name_hash->{$devname};
> - $dev_name_hash->{$devname} = $hd;
> - push @$devlist, $hd;
> - }
> - }
> -
> - return $devlist;
> -};
> -
> sub zfs_mirror_size_check {
> my ($expected, $actual) = @_;
>
>
More information about the pve-devel
mailing list