Adapt error messages around these checks to follow a similar structure
as in the TUI (<filesystem>: <message>) instead of displaying them as
warnings, since they actually stop users from continuing anyway.
Signed-off-by: Michael Köppl <m.koe...@proxmox.com>
---
This is mostly to have both the TUI and GUI display actual errors with
context instead of warnings since users cannot continue because it was a
bit irritating to have the TUI display an error with context and the GUI
just saying "Warning". Additionally, the error messages now have a
similar format in both installers. The TUI uses ZFS (RAID0), BTRFS
(RAID0), etc., whereas the GUI uses zfs (RAID0), etc. I decided not to
change that for now since it would be a bit out of scope for this
series, I think. Unifying any informational or error messages
throughout the installers in general would make sense for a future
series, though.
Proxmox/Install.pm | 16 ++++++++--------
proxinstall | 6 +++---
proxmox-auto-installer/src/utils.rs | 4 ++--
.../btrfs_raid_single_disk.json | 2 +-
.../parse_answer_fail/duplicate_disk.json | 2 +-
.../parse_answer_fail/zfs_raid_single_disk.json | 2 +-
6 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index f852147..c6dd8c4 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -287,7 +287,7 @@ sub get_zfs_raid_setup {
my $devlist = &$get_raid_devlist();
my $diskcount = scalar(@$devlist);
- die "$filesys needs at least one device\n" if $diskcount < 1;
+ die "$filesys: need at least one device\n" if $diskcount < 1;
my $cmd = '';
if ($filesys eq 'zfs (RAID0)') {
@@ -296,7 +296,7 @@ sub get_zfs_raid_setup {
$cmd .= " @$hd[1]";
}
} elsif ($filesys eq 'zfs (RAID1)') {
- die "zfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
+ die "$filesys: need at least 2 devices\n" if $diskcount < 2;
$cmd .= ' mirror ';
my $hd = @$devlist[0];
my $expected_size = @$hd[2]; # all disks need approximately same size
@@ -306,8 +306,8 @@ sub get_zfs_raid_setup {
$cmd .= " @$hd[1]";
}
} elsif ($filesys eq 'zfs (RAID10)') {
- die "zfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
- die "zfs (RAID10) needs an even number of devices\n" if $diskcount & 1;
+ die "$filesys: need at least 4 devices\n" if $diskcount < 4;
+ die "$filesys: need an even number of devices\n" if $diskcount & 1;
for (my $i = 0; $i < $diskcount; $i += 2) {
my $hd1 = @$devlist[$i];
@@ -321,7 +321,7 @@ sub get_zfs_raid_setup {
} elsif ($filesys =~ m/^zfs \(RAIDZ-([123])\)$/) {
my $level = $1;
my $mindisks = 2 + $level;
- die "zfs (RAIDZ-$level) needs at least $mindisks devices\n"
+ die "zfs (RAIDZ-$level): need at least $mindisks devices\n"
if scalar(@$devlist) < $mindisks;
my $hd = @$devlist[0];
my $expected_size = @$hd[2]; # all disks need approximately same size
@@ -355,7 +355,7 @@ sub get_btrfs_raid_setup {
my $devlist = &$get_raid_devlist();
my $diskcount = scalar(@$devlist);
- die "$filesys needs at least one device\n" if $diskcount < 1;
+ die "$filesys: need at least one device\n" if $diskcount < 1;
foreach my $hd (@$devlist) {
legacy_bios_4k_check(@$hd[4]);
@@ -369,10 +369,10 @@ sub get_btrfs_raid_setup {
if ($filesys eq 'btrfs (RAID0)') {
$mode = 'raid0';
} elsif ($filesys eq 'btrfs (RAID1)') {
- die "btrfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
+ die "$filesys: need at least 2 devices\n" if $diskcount < 2;
$mode = 'raid1';
} elsif ($filesys eq 'btrfs (RAID10)') {
- die "btrfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
+ die "$filesys: need at least 4 devices\n" if $diskcount < 4;
$mode = 'raid10';
} else {
die "unknown btrfs mode '$filesys'\n";
diff --git a/proxinstall b/proxinstall
index 84f1a91..5e7eb1b 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1593,7 +1593,7 @@ sub create_hdsel_view {
apply_raid_disk_selection();
my ($devlist) = eval { Proxmox::Install::get_zfs_raid_setup()
};
if (my $err = $@) {
- Proxmox::UI::message("Warning: $err\nPlease fix ZFS setup
first.");
+ Proxmox::UI::message("$err");
return;
}
$target_hds = [map { $_->[1] } @$devlist];
@@ -1601,7 +1601,7 @@ sub create_hdsel_view {
apply_raid_disk_selection();
my ($devlist) = eval {
Proxmox::Install::get_btrfs_raid_setup() };
if (my $err = $@) {
- Proxmox::UI::message("Warning: $err\nPlease fix BTRFS
setup first.");
+ Proxmox::UI::message("$err");
return;
}
$target_hds = [map { $_->[1] } @$devlist];
@@ -1614,7 +1614,7 @@ sub create_hdsel_view {
Proxmox::Install::swapsize_check($hdsize);
};
if (my $err = $@) {
- Proxmox::UI::message("Warning: $err\n");
+ Proxmox::UI::message("$filesys: $err");
return;
}
$target_hds = [$target_hd];
diff --git a/proxmox-auto-installer/src/utils.rs
b/proxmox-auto-installer/src/utils.rs
index 62646f2..7d42f2c 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -395,7 +395,7 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()>
{
let min_disks = answer.disks.fs_type.get_min_disks();
if selection.len() < min_disks {
bail!(
- "{} requires at least {} disks",
+ "{}: need at least {} disks",
answer.disks.fs_type,
min_disks
);
@@ -404,7 +404,7 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()>
{
let mut disk_set = HashSet::new();
for disk in selection {
if !disk_set.insert(disk) {
- bail!("List of disks contains duplicate disk {disk}");
+ bail!("List of disks contains duplicate device {disk}");
}
}
}
diff --git
a/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
index 37f35fe..957cb49 100644
---
a/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
+++
b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
@@ -1,3 +1,3 @@
{
- "error": "BTRFS (RAID1) requires at least 2 disks"
+ "error": "BTRFS (RAID1): need at least 2 disks"
}
diff --git
a/proxmox-auto-installer/tests/resources/parse_answer_fail/duplicate_disk.json
b/proxmox-auto-installer/tests/resources/parse_answer_fail/duplicate_disk.json
index d01fabe..93f6b6c 100644
---
a/proxmox-auto-installer/tests/resources/parse_answer_fail/duplicate_disk.json
+++
b/proxmox-auto-installer/tests/resources/parse_answer_fail/duplicate_disk.json
@@ -1,3 +1,3 @@
{
- "error": "List of disks contains duplicate disk sda"
+ "error": "List of disks contains duplicate device sda"
}
diff --git
a/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
index 9a8cc90..41c99d8 100644
---
a/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
+++
b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
@@ -1,3 +1,3 @@
{
- "error": "ZFS (RAID10) requires at least 4 disks"
+ "error": "ZFS (RAID10): need at least 4 disks"
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
