[pve-devel] [PATCH installer 3/3] fix #1211: allow install on 4kn disks
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Nov 27 09:16:01 CET 2019
some more nits, in addition to what Dominik and Thomas already said ;)
On November 26, 2019 3:27 pm, Stoiko Ivanov wrote:
> Installation on disks with 4k logical blocksize (4kn) failed, because the
> bios_boot (a.k.a. gdisk partitiontype EF02, place for grub in legacy BIOS
> boot mode) partition is created using start and end sectors (and sector 2047
> is not at 1M, where the ESP starts)
>
> This patch addresses the issue by not creating the bios_boot partition on
> 4kn disks at all - legacy boot from 4kn disks is not supported by most BIOS
> implementations and grub does not support it [0].
>
> The partition numbering was kept (esp is partition 2, data is partition 3,
> for consistency with other installations)
>
> If any of the bootable disks is 4kn then the installation of the grub legacy
> installation is skipped altogether.
>
> Additionally the invokation of mkfs.vfat needs to add the parameter '-s1'
> to create a bootable ESP.
that one might deserve more explanation ;) it's not needed to create a
bootable ESP, it's needed to workaround a bug in dosfstools with non
512b sector sizes. UEFI (and other code) differentiates between FAT,
FAT16 and FAT32 by looking at the number of clusters. we want FAT32, but
the calculation is off and we get a non-FAT32 number of clusters as a
result. thus, our partition gets treated as FAT16 or FAT instead of
FAT32, and brokenness ensues.
at least something like:
---
for a 512M vfat partition on a 512n or 512e disk, mkfs.vfat calculates a
default value of 8 sectors per cluster. for a 512M partition on a 4kn
disk, we need to scale this by 512/4096=8 and explicitly set '-s' to 1
accordingly, since the calculation in mkfs.vfat is brokenly assuming
512b sectors[1].
1: https://github.com/dosfstools/dosfstools/issues/111
---
+ a comment to where we set '-s1'
#FIXME remove once <link to bug> is fixed
alternatively, we could make the ESP bigger on 4kn disks (but for -s8 to
work, we'd need at least 2049M which might cause trouble with stupid UEFI
implementations..).
>
> Tested with a qemu-machine by passing
> 'logical_block_size=4096,physical_block_size=4096' to the disk's device lines
> and installing:
> * ZFS RAIDZ1
> * ZFS single-disk
> * EXT4
>
> [0] http://savannah.gnu.org/bugs/?46700
>
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
> proxinstall | 58 ++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/proxinstall b/proxinstall
> index a0ba91f..af73671 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -913,6 +913,17 @@ my $clean_disk = sub {
> }
> };
>
> +sub get_logical_blocksize {
> + my ($dev) = @_;
> +
> + $dev =~ s!^/dev/!!;
> +
> + my $blocksize = file_read_firstline("/sys/block/$dev/queue/logical_block_size") // '';
> + die "failed to parse logical_block_size of $dev - got $blocksize\n" if $blocksize !~ /^[0-9]+$/;
> +
> + return int($blocksize);
> +}
> +
> sub partition_bootable_disk {
> my ($target_dev, $maxhdsizegb, $ptype) = @_;
>
> @@ -936,6 +947,8 @@ sub partition_bootable_disk {
> my $hdgb = int($hdsize/(1024*1024));
> die "hardisk '$target_dev' too small (${hdgb}GB)\n" if $hdgb < 8;
>
> + my $blocksize = get_logical_blocksize($target_dev);
> +
> # 1 - BIOS boot partition (Grub Stage2): first free 1M
> # 2 - EFI ESP: next free 512M
> # 3 - OS/Data partition: rest, up to $maxhdsize in MB
> @@ -959,11 +972,13 @@ sub partition_bootable_disk {
> syscmd($pcmd) == 0 ||
> die "unable to partition harddisk '${target_dev}'\n";
>
> - $pnum = 1;
> - $pcmd = ['sgdisk', '-a1', "-n$pnum:34:2047", "-t$pnum:EF02" , $target_dev];
> + if ($blocksize != 4096) {
> + $pnum = 1;
> + $pcmd = ['sgdisk', '-a1', "-n$pnum:34:2047", "-t$pnum:EF02" , $target_dev];
>
> - syscmd($pcmd) == 0 ||
> - die "unable to create bios_boot partition '${target_dev}'\n";
> + syscmd($pcmd) == 0 ||
> + die "unable to create bios_boot partition '${target_dev}'\n";
> + }
>
> &$udevadm_trigger_block();
>
> @@ -971,7 +986,7 @@ sub partition_bootable_disk {
> syscmd("dd if=/dev/zero of=$part bs=1M count=256") if -b $part;
> }
>
> - return ($os_size, $osdev, $efibootdev);
> + return ($os_size, $osdev, $efibootdev, $blocksize);
> }
>
> sub get_pv_list_from_vgname {
> @@ -1274,7 +1289,7 @@ sub extract_data {
> foreach my $hd (@$devlist) {
> my $devname = @$hd[1];
> &$clean_disk($devname);
> - my ($size, $osdev, $efidev) =
> + my ($size, $osdev, $efidev, $logical_bsize) =
> partition_bootable_disk($devname, undef, '8300');
> $rootdev = $osdev if !defined($rootdev); # simply point to first disk
> my $by_id = find_stable_path("/dev/disk/by-id", $devname);
> @@ -1283,6 +1298,7 @@ sub extract_data {
> devname => $devname,
> osdev => $osdev,
> by_id => $by_id,
> + logical_bsize => $logical_bsize,
> };
> push @$btrfs_partitions, $osdev;
> $disksize = $size;
> @@ -1304,7 +1320,7 @@ sub extract_data {
> foreach my $hd (@$bootdevlist) {
> my $devname = @$hd[1];
>
> - my ($size, $osdev, $efidev) =
> + my ($size, $osdev, $efidev, $logical_bsize) =
> partition_bootable_disk($devname, $config_options->{hdsize}, 'BF01');
>
> zfs_mirror_size_check($disksize, $size) if $disksize;
> @@ -1312,7 +1328,8 @@ sub extract_data {
> push @$bootdevinfo, {
> esp => $efidev,
> devname => $devname,
> - osdev => $osdev
> + osdev => $osdev,
> + logical_bsize => $logical_bsize,
> };
> $disksize = $size;
> }
> @@ -1336,8 +1353,8 @@ sub extract_data {
>
> &$clean_disk($target_hd);
>
> - my ($os_size, $osdev, $efidev);
> - ($os_size, $osdev, $efidev) =
> + my ($os_size, $osdev, $efidev, $logical_bsize);
> + ($os_size, $osdev, $efidev, $logical_bsize) =
> partition_bootable_disk($target_hd, $config_options->{hdsize}, '8E00');
>
> &$udevadm_trigger_block();
> @@ -1348,6 +1365,7 @@ sub extract_data {
> devname => $target_hd,
> osdev => $osdev,
> by_id => $by_id,
> + logical_bsize => $logical_bsize,
> };
>
> my $swap_size = compute_swapsize($os_size);
> @@ -1374,7 +1392,8 @@ sub extract_data {
>
> foreach my $di (@$bootdevinfo) {
> next if !$di->{esp};
> - syscmd("mkfs.vfat -F32 $di->{esp}") == 0 ||
> + my $vfat_extra_opts = ($di->{logical_bsize} == 4096) ? '-s1' : '';
> + syscmd("mkfs.vfat $vfat_extra_opts -F32 $di->{esp}") == 0 ||
> die "unable to initialize EFI ESP on device $di->{esp}\n";
> }
>
> @@ -1729,13 +1748,20 @@ _EOD
> syscmd("chroot $targetdir /usr/sbin/update-initramfs -c -k $kapi") == 0 ||
> die "unable to install initramfs\n";
>
> + my $native_4k_disk_bootable = 0;
if you resend this, maybe this name could be changed?
$4kn_boot_disk_found
it's not about whether a 4kn disk is bootable, but rather whether any of
the boot(able) disks are 4kn ;)
> + foreach my $di (@$bootdevinfo) {
> + $native_4k_disk_bootable |= ($di->{logical_bsize} == 4096);
> + }
nit: this could be a single line grep
> +
> foreach my $di (@$bootdevinfo) {
> my $dev = $di->{devname};
> - eval {
> - syscmd("chroot $targetdir /usr/sbin/grub-install --target i386-pc --no-floppy --bootloader-id='proxmox' $dev") == 0 ||
> - die "unable to install the i386-pc boot loader on '$dev'\n";
> - };
> - push @$bootloader_err_list, $@ if $@;
> + if (!$native_4k_disk_bootable) {
> + eval {
> + syscmd("chroot $targetdir /usr/sbin/grub-install --target i386-pc --no-floppy --bootloader-id='proxmox' $dev") == 0 ||
> + die "unable to install the i386-pc boot loader on '$dev'\n";
> + };
> + push @$bootloader_err_list, $@ if $@;
> + }
>
> eval {
> if (my $esp = $di->{esp}) {
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
More information about the pve-devel
mailing list