[pve-devel] [PATCH installer 3/6] warn users about existing ESPs on unused disks

[Thomas Lamprecht]

On 11/29/19 11:53 AM, Stoiko Ivanov wrote:
> When doing the sanity-checks for the chosen harddisk setup, also check for
> existing ESPs on those disks the installer will not touch.
> 
> Since they are not touched, the ESP remains on the disk, which can lead to
> unbootable systems and other surprises (e.g. when the UEFI implementation
> decides to ignore the new bootentry put in front of the list by
> `bootctl install` (via `pve-efiboot-tool`) and boots an old system).

why not add those to our synced partitions? Or ask for that?

> 
> If an ESP is detected we warn the user directly at the disk-selection view, and
> leave them the option to ignore it or cancel the installation, since wiping
> the ESP by clicking an 'OK' button might lead to unexpected data-loss.
> 
> ESPs are identified by their partition-type, using `lsblk`'s json output
> 
> The message dialog generation is factored in a sub of its own for later reuse.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>  debian/control |  2 ++
>  proxinstall    | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/debian/control b/debian/control
> index 97bf20b..dc1a0f3 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -14,6 +14,7 @@ Package: pve-installer
>  Architecture: all
>  Depends: geoip-bin,
>           libgtk3-webkit2-perl,
> +         libjson-perl,
>           pve-kernel-helper,
>           squashfs-tools,
>           ${perl:Depends},
> @@ -24,6 +25,7 @@ Package: pmg-installer
>  Architecture: all
>  Depends: geoip-bin,
>           libgtk3-webkit2-perl,
> +         libjson-perl,
>           pve-kernel-helper,
>           squashfs-tools,
>           ${perl:Depends},
> diff --git a/proxinstall b/proxinstall
> index 911ee3d..47faa09 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -20,6 +20,7 @@ use Data::Dumper;
>  use File::Basename;
>  use File::Path;
>  use Time::HiRes;
> +use JSON;
>  
>  use ProxmoxInstallerSetup;
>  
> @@ -3316,6 +3317,7 @@ sub create_hdsel_view {
>  
>      set_next(undef, sub {
>  
> +	my $unused = {};
>  	if ($config_options->{filesys} =~ m/zfs/) {
>  	    my ($devlist) = eval { get_zfs_raid_setup() };
>  	    if (my $err = $@) {
> @@ -3323,6 +3325,7 @@ sub create_hdsel_view {
>  		return;
>  	    }
>  	    $config_options->{target_hds} = [ map { $_->[1] } @$devlist ];
> +	    $unused = get_unused_bdevs($config_options->{target_hds});
>  	} elsif ($config_options->{filesys} =~ m/btrfs/) {
>  	    my ($devlist) = eval { get_btrfs_raid_setup() };
>  	    if (my $err = $@) {
> @@ -3330,6 +3333,7 @@ sub create_hdsel_view {
>  		return;
>  	    }
>  	    $config_options->{target_hds} = [ map { $_->[1] } @$devlist ];
> +	    $unused = get_unused_bdevs($config_options->{target_hds});
>  	} else {
>  	    eval { legacy_bios_4k_check(logical_blocksize($target_hd)) };
>  	    if (my $err = $@) {
> @@ -3337,13 +3341,74 @@ sub create_hdsel_view {
>  		return;
>  	    }
>  	    $config_options->{target_hds} = [ $target_hd ];
> +	    $unused = get_unused_bdevs($config_options->{target_hds});
>  	}
>  
> +	warn_existing_esp_ignore_or_abort($unused);
> +
>  	$step_number++;
>  	create_country_view();
>      });
>  }
>  
> +sub get_unused_bdevs {
> +    my ($used_disks) = @_;
> +
> +    my @unused_hds;
> +    foreach my $hd (@$hds) {
> +	my $devname = @$hd[1];
> +	if ( ! grep { $devname eq $_ } @$used_disks ){
> +	    push @unused_hds, $devname;
> +	}
> +    }
> +
> +    my $jsonoutput = '';
> +    run_command(['lsblk', '--bytes', '--json',
> +        '-o', 'PATH,UUID,FSTYPE,PARTTYPE,LABEL', @unused_hds ], sub {
> +            my ($line) = @_;
> +            $jsonoutput .= $line;
> +        });
> +
> +    my $lsblk = decode_json($jsonoutput);
> +
> +    return $lsblk->{blockdevices};
> +}
> +
> +sub warn_existing_esp_ignore_or_abort {
> +    my ($unused_devs) = @_;
> +
> +    return if ($boot_type ne 'efi');
> +
> +    my @esps = map { $_->{parttype} eq "c12a7328-f81f-11d2-ba4b-00a0c93ec93b" ? ($_->{path}) : () } @$unused_devs;
> +    return if !scalar(@esps);
> +
> +    my $message = "Detected existing ESPs:\n- " . join("\n- ", @esps);
> +    $message .= <<'EOF';
> +
> +
> +This could prevent correct booting!
> +Do you want to ignore or cancel the installation?
> +EOF
> +
> +    ask_existing_diskconfig_or_abort($message, "existing ESPs");
> +}
> +
> +sub ask_existing_diskconfig_or_abort {
> +    my ($message, $fail_reason, $ok_action) = @_;
> +
> +    my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'question', 'ok-cancel', $message);
> +    my $response = $dialog->run();
> +    $dialog->destroy();
> +
> +    if ($response eq 'ok') {
> +	$ok_action->() if defined($ok_action);
> +    } else {
> +	set_next("_Reboot", sub { exit (0); } );
> +	display_html("fail.htm");
> +	die "Cancled installation by user, due to $fail_reason\n";
> +    }
> +}
> +
>  sub create_extract_view {
>  
>      cleanup_view();
>