[pve-devel] [PATCH installer v3 0/7] don't create swap on ZFS as default

[Stoiko Ivanov]

Changes from v2:
* Unify partitioning between ZFS and other (LVM, Btrfs) cases.
* Changes for the partitioning:
* drop the 8M partition (number 9) at the end of the disk for ZFS
(should people need to replace one disk by a slightly smaller one, they can
shrink the efi-partition by this amount)
* create the bios_boot partition from sectors 34:2047 (remaining space after
the primary GPT)
* change in semantics: users can provide the `hdsize` parameter (size that is
used), instead of minfree (size that is left unpartitioned).
* fixed a typo in an error-message
* readd the patch to use truncate instead of dd for the test.img creation

Stoiko Ivanov (7):
Unify and adapt disk partitioning
Remove partition_bootable_zfs_disk
Refactor $maxhdsize computation
Fix typo in error message.
Add hdsize spinbutton to ZFS grid
Do not create a swap zvol
buildsys: use truncate to create test.img

Makefile | 2 +-
proxinstall | 139 ++++++++++++++----------------------------------------------
2 files changed, 33 insertions(+), 108 deletions(-)

[Stoiko Ivanov]

As it is not used anymore

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 51 ---------------------------------------------------
1 file changed, 51 deletions(-)

diff --git a/proxinstall b/proxinstall
index 0a4fb8a..b19c8c9 100755
--- a/proxinstall
+++ b/proxinstall
@@ -924,57 +924,6 @@ sub partition_bootable_disk {
return ($os_size, $osdev, $efibootdev);
}

-# ZFS has this use_whole_disk concept, so we try to partition the same
-# way as zfs does by default. There is room at start of disk to insert
-# a grub boot partition. But adding a EFI ESP is not possible.
-#
-# Note: zfs people think this is just a waste of space an not
-# required. Instead, you should put the ESP on another disk (log,
-# ..).
-
-sub partition_bootable_zfs_disk {
- my ($target_dev) = @_;
-
- die "too dangerous" if $opt_testmode;
-
- syscmd("sgdisk -Z ${target_dev}");
- my $hdsize = hd_size($target_dev); # size in blocks (1024 bytes)
-
- my $hdgb = int($hdsize/(1024*1024));
- die "hardisk '$target_dev' too small (${hdsize}GB)\n" if $hdgb < 8;
-
- # 1 - GRUB boot partition: 1M
- # 2 - OS/Data partition
- # 9 - ZFS reserved partition
-
- my $grubbootdev = get_partition_dev($target_dev, 1);
- my $osdev = get_partition_dev ($target_dev, 2);
-
- my $pcmd = ['sgdisk', '-a1'];
-
- my $pnum = 1;
- push @$pcmd, "-n$pnum:34:2047", "-t$pnum:EF02";
-
- $pnum = 9;
- push @$pcmd, "-n$pnum:-8M:0", "-t$pnum:BF07";
-
- $pnum = 2;
- push @$pcmd, "-n$pnum:2048:0", "-t$pnum:BF01", '-c', "$pnum:zfs";
-
- push @$pcmd, $target_dev;
-
- my $os_size = $hdsize - 1024 - 1024*8;
-
- syscmd($pcmd) == 0 ||
- die "unable to partition harddisk '${target_dev}'\n";
-
- &$udevadm_trigger_block();
-
- syscmd("dd if=/dev/zero of=$osdev bs=1M count=16") if -b $osdev;
-
- return ($os_size, $osdev);
-}
-
sub create_lvm_volumes {
my ($lvmdev, $os_size, $swap_size) = @_;

[Stoiko Ivanov]

Pass $config_options->{hdsize} to partition_bootable_disk, where needed,
and do the unitconversion in partition_bootable_disk.

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/proxinstall b/proxinstall
index b19c8c9..bd9c424 100755
--- a/proxinstall
+++ b/proxinstall
@@ -867,7 +867,7 @@ my $clean_disk = sub {
};

sub partition_bootable_disk {
- my ($target_dev, $maxhdsize, $ptype) = @_;
+ my ($target_dev, $maxhdsizegb, $ptype) = @_;

die "too dangerous" if $opt_testmode;

@@ -878,9 +878,12 @@ sub partition_bootable_disk {
my $hdsize = hd_size($target_dev); # size in KB (1024 bytes)

my $restricted_hdsize_mb = 0; # 0 ==> end of partition
- if ($maxhdsize && ($maxhdsize < $hdsize)) {
- $hdsize = $maxhdsize;
- $restricted_hdsize_mb = int($hdsize/1024) . 'M';
+ if ($maxhdsizegb) {
+ my $maxhdsize = $maxhdsizegb * 1024 * 1024;
+ if ($maxhdsize < $hdsize) {
+ $hdsize = $maxhdsize;
+ $restricted_hdsize_mb = int($hdsize/1024) . 'M';
+ }
}

my $hdgb = int($hdsize/(1024*1024));
@@ -1136,13 +1139,6 @@ sub extract_data {

my ($devlist, $bootdevlist, $vdev) = get_zfs_raid_setup();

- my $maxhdsize;
- if ($config_options->{hdsize}) {
- # max hdsize passed on cmdline (GB)
- $maxhdsize = $config_options->{hdsize}*1024*1024;
- }
-
-
my $disksize;
foreach my $hd (@$devlist) {
&$clean_disk(@$hd[1]);
@@ -1151,7 +1147,7 @@ sub extract_data {
my $devname = @$hd[1];

my ($size, $osdev) =
- partition_bootable_disk($devname, $maxhdsize, 'BF01');
+ partition_bootable_disk($devname, $config_options->{hdsize}, 'BF01');
zfs_mirror_size_check($disksize, $size) if $disksize;
push @$bootdevinfo, { devname => $devname, osdev => $osdev};
$disksize = $size;
@@ -1180,17 +1176,11 @@ sub extract_data {

die "target '$target_hd' is not a valid block device\n" if ! -b $target_hd;

- my $maxhdsize;
- if ($config_options->{hdsize}) {
- # max hdsize passed on cmdline (GB)
- $maxhdsize = $config_options->{hdsize}*1024*1024;
- }
-
&$clean_disk($target_hd);

my ($os_size, $osdev, $efidev);
($os_size, $osdev, $efidev) =
- partition_bootable_disk($target_hd, $maxhdsize, '8E00');
+ partition_bootable_disk($target_hd, $config_options->{hdsize}, '8E00');

&$udevadm_trigger_block();

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxinstall b/proxinstall
index bd9c424..830dcbd 100755
--- a/proxinstall
+++ b/proxinstall
@@ -887,7 +887,7 @@ sub partition_bootable_disk {
}

my $hdgb = int($hdsize/(1024*1024));
- die "hardisk '$target_dev' too small (${hdsize}GB)\n" if $hdgb < 8;
+ die "hardisk '$target_dev' too small (${hdgb}GB)\n" if $hdgb < 8;

# 1 - BIOS boot partition (Grub Stage2): first free 1M
# 2 - EFI ESP: next free 512M

[Stoiko Ivanov]

by creating a shared entrybuffer with the hdsize widget in the regular
partitioning menu.

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/proxinstall b/proxinstall
index 830dcbd..5a857f5 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2581,6 +2581,12 @@ my $create_raid_advanced_grid = sub {
$spinbutton_copies->set_value($config_options->{copies});
push @$labeled_widgets, "copies", $spinbutton_copies;

+ our $hdsize_size_adj;
+ our $hdsize_entry_buffer;
+ my $spinbutton_hdsize = Gtk3::SpinButton->new($hdsize_size_adj, 1, 1);
+ $spinbutton_hdsize->set_buffer($hdsize_entry_buffer);
+ $spinbutton_hdsize->set_tooltip_text("only use specified size (GB) of the harddisk (rest left unpartitioned)");
+ push @$labeled_widgets, "hdsize", $spinbutton_hdsize;
return &$create_label_widget_grid($labeled_widgets);;
};

@@ -2650,8 +2656,10 @@ sub create_hdoption_view {
$hdsize = int((-s $target_hd) / (1024*1024*1024.0));
}

- my $hdsize_size_adj = Gtk3::Adjustment->new($config_options->{hdsize} || $hdsize, 0, $hdsize+1, 1, 1, 1);
+ our $hdsize_size_adj = Gtk3::Adjustment->new($config_options->{hdsize} || $hdsize, 0, $hdsize+1, 1, 1, 1);
+ our $hdsize_entry_buffer = Gtk3::EntryBuffer->new(undef, 1);
my $spinbutton_hdsize = Gtk3::SpinButton->new($hdsize_size_adj, 1, 1);
+ $spinbutton_hdsize->set_buffer($hdsize_entry_buffer);
$spinbutton_hdsize->set_tooltip_text("only use specified size (GB) of the harddisk (rest left unpartitioned)");
push @$hdsize_labeled_widgets, "hdsize", $spinbutton_hdsize;

[Stoiko Ivanov]

saves time during creation and subsequent rsync

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d5e76b6..0fdebbe 100644
--- a/Makefile
+++ b/Makefile
@@ -65,7 +65,7 @@ upload-pve: ${PVE_DEB}
tar cf - ${PVE_DEB} | ssh -X ***@repo.proxmox.com -- upload --product pve --dist stretch

test.img:
- dd if=/dev/zero of=test.img bs=2048 count=1M
+ truncate -s 2G test.img

check-pve: ${PVE_DEB} test.img
rm -rf testdir

[Stoiko Ivanov]

Using zvols as swap devices does create deadlocks for users, as of zfs 0.7.9.
As the discussion of the issue [0] shows, fixing this is involved, and probably
won't be happening anytime soon (before zfs 0.8).

Not creating swap as a zvol seems like the better default setting for now.

Users needing swap can either leave unpartitioned space, by setting minfree,
or create the zvol quite easily after installation

[0] https://github.com/zfsonlinux/zfs/issues/7734

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 33 ---------------------------------
1 file changed, 33 deletions(-)

diff --git a/proxinstall b/proxinstall
index 5a857f5..f1a0262 100755
--- a/proxinstall
+++ b/proxinstall
@@ -814,36 +814,6 @@ sub zfs_create_rpool {
if defined($value) && $value != 1;
}

-sub zfs_create_swap {
- my ($swapsize) = @_;
-
- my $cmd = "zfs create -V ${swapsize}K -b 4K";
-
- $cmd .= " -o com.sun:auto-snapshot=false";
-
- # copies for swap does not make sense
- $cmd .= " -o copies=1";
-
- # reduces memory pressure
- $cmd .= " -o sync=always";
-
- # cheapest compression to drop zero pages
- $cmd .= " -o compression=zle";
-
- # skip log devices
- $cmd .= " -o logbias=throughput";
- # only cache metadata in RAM (caching swap content does not make sense)
- $cmd .= " -o primarycache=metadata";
- # don't cache anything in L2ARC
- $cmd .= " -o secondarycache=none";
-
- $cmd .= " $zfspoolname/swap";
- syscmd ($cmd) == 0 ||
- die "unable to create zfs swap device\n";
-
- return "/dev/zvol/$zfspoolname/swap";
-}
-
my $udevadm_trigger_block = sub {
my ($nowait) = @_;

@@ -1169,9 +1139,6 @@ sub extract_data {

zfs_create_rpool($vdev);

- my $swap_size = compute_swapsize($disksize);
- $swapfile = zfs_create_swap($swap_size) if $swap_size;
-
} else {

die "target '$target_hd' is not a valid block device\n" if ! -b $target_hd;

[Stoiko Ivanov]

Change partition_bootable_disk:
* Increase the efi-esp partitions' size to 512M (from 256M)
* Create the bios_boot partition in the first MB (sectors 34:2047)
* Split the sgdisk invocation into 2, to get 1M alignment for efi and os
partitions

Changes are inspired by the wiki-entries by zol-upstream [0].

Use partition_bootable_disk also for bootable ZFS disks.
This provides users with the opportunity to leave unpartitioned space at
the end of the disk (by setting the hdsize parameter) for use outside of ZFS
(e.g. to create a dedicated swap partition).

[0] https://github.com/zfsonlinux/zfs/wiki/Debian-Stretch-Root-on-ZFS

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
proxinstall | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/proxinstall b/proxinstall
index fac87ee..0a4fb8a 100755
--- a/proxinstall
+++ b/proxinstall
@@ -872,7 +872,7 @@ sub partition_bootable_disk {
die "too dangerous" if $opt_testmode;

die "unknown partition type '$ptype'"
- if !($ptype eq '8E00' || $ptype eq '8300');
+ if !($ptype eq '8E00' || $ptype eq '8300' || $ptype eq 'BF01');

syscmd("sgdisk -Z ${target_dev}");
my $hdsize = hd_size($target_dev); # size in KB (1024 bytes)
@@ -887,7 +887,7 @@ sub partition_bootable_disk {
die "hardisk '$target_dev' too small (${hdsize}GB)\n" if $hdgb < 8;

# 1 - BIOS boot partition (Grub Stage2): first free 1M
- # 2 - EFI ESP: next free 256M
+ # 2 - EFI ESP: next free 512M
# 3 - OS/Data partition: rest, up to $maxhdsize in MB

my $grubbootdev = get_partition_dev($target_dev, 1);
@@ -896,22 +896,25 @@ sub partition_bootable_disk {

my $pcmd = ['sgdisk'];

- my $pnum = 1;
- push @$pcmd, "-n${pnum}:1M:+1M", "-t$pnum:EF02";
-
- $pnum = 2;
- push @$pcmd, "-n${pnum}:2M:+256M", "-t$pnum:EF00";
+ my $pnum = 2;
+ push @$pcmd, "-n${pnum}:1M:+512M", "-t$pnum:EF00";

$pnum = 3;
- push @$pcmd, "-n${pnum}:258M:${restricted_hdsize_mb}", "-t$pnum:$ptype";
+ push @$pcmd, "-n${pnum}:513M:${restricted_hdsize_mb}", "-t$pnum:$ptype";

push @$pcmd, $target_dev;

- my $os_size = $hdsize - 258*1024; # 256M + 1M + 1M alignment
+ my $os_size = $hdsize - 513*1024; # 512M efi + 1M bios_boot + 1M alignment

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];
+
+ syscmd($pcmd) == 0 ||
+ die "unable to create bios_boot partition '${target_dev}'\n";
+
&$udevadm_trigger_block();

foreach my $part ($efibootdev, $osdev) {
@@ -1184,14 +1187,22 @@ sub extract_data {

my ($devlist, $bootdevlist, $vdev) = get_zfs_raid_setup();

+ my $maxhdsize;
+ if ($config_options->{hdsize}) {
+ # max hdsize passed on cmdline (GB)
+ $maxhdsize = $config_options->{hdsize}*1024*1024;
+ }
+
+
my $disksize;
foreach my $hd (@$devlist) {
&$clean_disk(@$hd[1]);
}
foreach my $hd (@$bootdevlist) {
my $devname = @$hd[1];
+
my ($size, $osdev) =
- partition_bootable_zfs_disk($devname);
+ partition_bootable_disk($devname, $maxhdsize, 'BF01');
zfs_mirror_size_check($disksize, $size) if $disksize;
push @$bootdevinfo, { devname => $devname, osdev => $osdev};
$disksize = $size;

[Fabian Grünbichler]

series LGTM, haven't built or tested a CD though.

a followup for pve-docs in the installer section that mentions that ZFS
does no longer setup swap and what to do in case you want to create it
manually would be nice, as well as a short sentence in the
yet-to-be-written 5.3 release notes.

Post by Stoiko Ivanov
* Unify partitioning between ZFS and other (LVM, Btrfs) cases.
* drop the 8M partition (number 9) at the end of the disk for ZFS
(should people need to replace one disk by a slightly smaller one, they can
shrink the efi-partition by this amount)
* create the bios_boot partition from sectors 34:2047 (remaining space after
the primary GPT)
* change in semantics: users can provide the `hdsize` parameter (size that is
used), instead of minfree (size that is left unpartitioned).
* fixed a typo in an error-message
* readd the patch to use truncate instead of dd for the test.img creation
Unify and adapt disk partitioning
Remove partition_bootable_zfs_disk
Refactor $maxhdsize computation
Fix typo in error message.
Add hdsize spinbutton to ZFS grid
Do not create a swap zvol
buildsys: use truncate to create test.img
Makefile | 2 +-
proxinstall | 139 ++++++++++++++----------------------------------------------
2 files changed, 33 insertions(+), 108 deletions(-)
--
2.11.0
_______________________________________________
pve-devel mailing list
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[Thomas Lamprecht]

Post by Fabian Grünbichler
series LGTM, haven't built or tested a CD though.

yeah, I tested applied it now, but followed up with a patch to remove the
newly introduced our usages, allows to reuse code a bit more.

Post by Fabian Grünbichler
a followup for pve-docs in the installer section that mentions that ZFS
does no longer setup swap and what to do in case you want to create it
manually would be nice, as well as a short sentence in the
yet-to-be-written 5.3 release notes.

Post by Stoiko Ivanov
* Unify partitioning between ZFS and other (LVM, Btrfs) cases.
* drop the 8M partition (number 9) at the end of the disk for ZFS
(should people need to replace one disk by a slightly smaller one, they can
shrink the efi-partition by this amount)
* create the bios_boot partition from sectors 34:2047 (remaining space after
the primary GPT)
* change in semantics: users can provide the `hdsize` parameter (size that is
used), instead of minfree (size that is left unpartitioned).
* fixed a typo in an error-message
* readd the patch to use truncate instead of dd for the test.img creation
Unify and adapt disk partitioning
Remove partition_bootable_zfs_disk
Refactor $maxhdsize computation
Fix typo in error message.
Add hdsize spinbutton to ZFS grid
Do not create a swap zvol
buildsys: use truncate to create test.img
Makefile | 2 +-
proxinstall | 139 ++++++++++++++----------------------------------------------
2 files changed, 33 insertions(+), 108 deletions(-)
--
2.11.0

[Thomas Lamprecht]

Post by Stoiko Ivanov
* Unify partitioning between ZFS and other (LVM, Btrfs) cases.
* drop the 8M partition (number 9) at the end of the disk for ZFS
(should people need to replace one disk by a slightly smaller one, they can
shrink the efi-partition by this amount)
* create the bios_boot partition from sectors 34:2047 (remaining space after
the primary GPT)
* change in semantics: users can provide the `hdsize` parameter (size that is
used), instead of minfree (size that is left unpartitioned).
* fixed a typo in an error-message
* readd the patch to use truncate instead of dd for the test.img creation
Unify and adapt disk partitioning
Remove partition_bootable_zfs_disk
Refactor $maxhdsize computation
Fix typo in error message.
Add hdsize spinbutton to ZFS grid
Do not create a swap zvol
buildsys: use truncate to create test.img
Makefile | 2 +-
proxinstall | 139 ++++++++++++++----------------------------------------------
2 files changed, 33 insertions(+), 108 deletions(-)

applied, with the off-list proposed followup to refactor the hdsize spinbutton
from an 'our' into a normal module variable.
Thanks!