[pve-devel] [PATCH installer v2 0/4] adaptation to kernel cmdline handling and 2 small ZFS related improvements

[pve-devel] [PATCH installer v2 0/4] adaptation to kernel cmdline handling and 2 small ZFS related improvements

[Stoiko Ivanov]

v1->v2:
* adapted the creation of /var/lib/vz as separate dataset to be only done
  for PVE and not for our other products

original cover-letter for v1:
This patch-series contains mostly unrelated patches, which still can be
tested well together:
* patch 1/4 fixes the parsing of our kernel cmdline parameters during the
  install (they were ignored if being the last on the line).
  additionally everything not used by our installer now end up in the
  target system's kernel cmdline - so if you need to add something for
  the system to boot - you do not need to do so 3 times (for the install,
  for the first boot, for the boot-loader config)
* patch 2/4 is a minor glitch I introduced, which caused me to debug the
  wrong thing for too long
* patch 3/4 has been a long-standing and quite sensible request
* patch 4/4 fixes a minor inconvenience on machines with local users and
  ZFS on root (dmesg gets filled with journald-messages upon
  journal-rotation and user-logins)

Tested the changes a bit (4/4 on my machine, 3/4 was straight-forward, 2/4
with a quick look, 1/4 with a few installer-options and net.ifnames=0)


Stoiko Ivanov (4):
  fix #4747: pass kernel cmdline parameters to target system
  unconfigured.sh: set serial to a number to prevent warning
  fix #1410: zfs: create /var/lib/vz as separate dataset
  zfs: set acltype=posix for root-dataset

 Proxmox/Install.pm        | 21 ++++++++++++---------
 Proxmox/Install/Config.pm | 25 +++++++++++++------------
 unconfigured.sh           |  1 +
 3 files changed, 26 insertions(+), 21 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH installer v2 1/4] fix #4747: pass kernel cmdline parameters to target system

[Stoiko Ivanov]

Parameters needed for booting during installation are best preserved
in the target cmdline as well - e.g. if you need a particular
cmdline switch for your system to boot at all - not having to add it
for the first boot of the installed system and manually adding it to
the bootloader config is an improvement.

This additionally enables us to drop the console parameter handling
for serial consoles (it is just one of the parameters to pass along).

Finally it fixes the regular expressions for the installer settings we
read from the cmdline (swapsize, maxroot,...) which were broken if
added as last entry.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Proxmox/Install.pm        | 11 +++++------
 Proxmox/Install/Config.pm | 25 +++++++++++++------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 66adb2d..c868992 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1152,11 +1152,10 @@ _EOD
 	}
 
 	update_progress(0.8, 0.95, 1, "make system bootable");
-	my $console_param='';
-	if (my $console = Proxmox::Install::Config::get_console()) {
-	    $console_param="console=$console";
-	    my $console_snippet = "GRUB_CMDLINE_LINUX=\"\$GRUB_CMDLINE_LINUX $console_param\"";
-	    file_write_all("$targetdir/etc/default/grub.d/console.cfg", $console_snippet);
+	my $target_cmdline='';
+	if ($target_cmdline = Proxmox::Install::Config::get_target_cmdline()) {
+	    my $target_cmdline_snippet = "GRUB_CMDLINE_LINUX=\"\$GRUB_CMDLINE_LINUX $target_cmdline\"";
+	    file_write_all("$targetdir/etc/default/grub.d/installer.cfg", $target_cmdline_snippet);
 	}
 
 	if ($use_zfs) {
@@ -1164,7 +1163,7 @@ _EOD
 	    my $zfs_snippet = "GRUB_CMDLINE_LINUX=\"\$GRUB_CMDLINE_LINUX root=ZFS=$zfs_pool_name/ROOT/$zfs_root_volume_name boot=zfs\"";
 	    file_write_all("$targetdir/etc/default/grub.d/zfs.cfg", $zfs_snippet);
 
-	    file_write_all("$targetdir/etc/kernel/cmdline", "root=ZFS=$zfs_pool_name/ROOT/$zfs_root_volume_name boot=zfs $console_param\n");
+	    file_write_all("$targetdir/etc/kernel/cmdline", "root=ZFS=$zfs_pool_name/ROOT/$zfs_root_volume_name boot=zfs $target_cmdline\n");
 
 	    zfs_setup_module_conf($targetdir);
 	}
diff --git a/Proxmox/Install/Config.pm b/Proxmox/Install/Config.pm
index 5e80255..b1acebc 100644
--- a/Proxmox/Install/Config.pm
+++ b/Proxmox/Install/Config.pm
@@ -16,36 +16,37 @@ my sub parse_kernel_cmdline {
 
     my $cmdline = Proxmox::Install::RunEnv::get('kernel_cmdline');
 
-    if ($cmdline =~ m/\s(ext4|xfs)(\s.*)?$/) {
+    if ($cmdline =~ s/\b(ext4|xfs)\s?//i) {
 	$cfg->{filesys} = $1;
     }
 
-    if ($cmdline =~ m/hdsize=(\d+(\.\d+)?)[\s\n]/i) {
+    if ($cmdline =~ s/\bhdsize=(\d+(\.\d+)?)\s?//i) {
 	$cfg->{hdsize} = $1;
     }
 
-    if ($cmdline =~ m/swapsize=(\d+(\.\d+)?)[\s\n]/i) {
+    if ($cmdline =~ s/\bswapsize=(\d+(\.\d+)?)\s?//i) {
 	$cfg->{swapsize} = $1;
     }
 
-    if ($cmdline =~ m/maxroot=(\d+(\.\d+)?)[\s\n]/i) {
+    if ($cmdline =~ s/\bmaxroot=(\d+(\.\d+)?)\s?//i) {
 	$cfg->{maxroot} = $1;
     }
 
-    if ($cmdline =~ m/minfree=(\d+(\.\d+)?)[\s\n]/i) {
+    if ($cmdline =~ s/\bminfree=(\d+(\.\d+)?)\s?//i) {
 	$cfg->{minfree} = $1;
     }
 
     my $iso_env = Proxmox::Install::ISOEnv::get();
     if ($iso_env->{product} eq 'pve') {
-	if ($cmdline =~ m/maxvz=(\d+(\.\d+)?)[\s\n]/i) {
+	if ($cmdline =~ s/\bmaxvz=(\d+(\.\d+)?)\s?//i) {
 	    $cfg->{maxvz} = $1;
 	}
     }
 
-    if ($cmdline =~ m/console=(\S+)[\s\n]?/i) {
-	    $cfg->{console} = $1;
-    }
+    $cmdline =~ s/(?:BOOT_IMAGE|root|ramdisk_size|splash|vga)=\S+\s?//gi;
+    $cmdline =~ s/ro|rw|quiet|proxdebug|proxtui|nomodeset//gi;
+
+    $cfg->{target_cmdline}= $cmdline;
 
     return $cfg;
 }
@@ -101,7 +102,7 @@ my sub init_cfg {
 	cidr => undef,
 	gateway => undef,
 	dns => undef,
-	console => undef,
+	target_cmdline => undef,
     };
 
     $initial = parse_kernel_cmdline($initial);
@@ -235,8 +236,8 @@ sub get_gateway { return get('gateway'); }
 sub set_dns { set_key('dns', $_[0]); }
 sub get_dns { return get('dns'); }
 
-sub set_console { set_key('console', $_[0]); }
-sub get_console { return get('console'); }
+sub set_target_cmdline { set_key('target_cmdline', $_[0]); }
+sub get_target_cmdline { return get('target_cmdline'); }
 
 
 1;
-- 
2.39.2

[pve-devel] [PATCH installer v2 2/4] unconfigured.sh: set serial to a number to prevent warning

[Stoiko Ivanov]

caught me off-guard while debugging other things - the message:
` [: : integer expression expected`
made me look a bit too long at the wrong place.

Fixes: a31259b1597447a0b431cd5c81a6db2bc80f1ddf
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 unconfigured.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/unconfigured.sh b/unconfigured.sh
index a361a20..6b3a8bf 100755
--- a/unconfigured.sh
+++ b/unconfigured.sh
@@ -7,6 +7,7 @@ trap "err_reboot" ERR
 parse_cmdline() {
     proxdebug=0
     proxtui=0
+    serial=0
     # shellcheck disable=SC2013 # per word splitting is wanted here
     for par in $(cat /proc/cmdline); do
         case $par in
-- 
2.39.2

[pve-devel] [PATCH installer v2 3/4] fix #1410: zfs: create /var/lib/vz as separate dataset

[Stoiko Ivanov]

this enables the users to set reservations on / separate from
/var/lib/vz - where backups, ISOs, and other data might fill the
complete pool.

moved the creation of the root-dataset above the creation of
rpool/data, so that the pve-specifics can remain in one if block.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Proxmox/Install.pm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index c868992..48c157a 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -182,13 +182,15 @@ sub zfs_create_rpool {
 
     syscmd("zfs create $pool_name/ROOT")  == 0 || die "unable to create zfs $pool_name/ROOT volume\n";
 
+    syscmd("zfs create $pool_name/ROOT/$root_volume_name")  == 0 ||
+	die "unable to create zfs $pool_name/ROOT/$root_volume_name volume\n";
+
     if ($iso_env->{product} eq 'pve') {
 	syscmd("zfs create $pool_name/data")  == 0 || die "unable to create zfs $pool_name/data volume\n";
+	syscmd("zfs create -p $pool_name/ROOT/$root_volume_name/var/lib/vz")  == 0 ||
+	    die "unable to create zfs $pool_name/ROOT/$root_volume_name/var/lib/vz volume\n";
     }
 
-    syscmd("zfs create $pool_name/ROOT/$root_volume_name")  == 0 ||
-	die "unable to create zfs $pool_name/ROOT/$root_volume_name volume\n";
-
     # default to `relatime` on, fast enough for the installer and production
     syscmd("zfs set atime=on relatime=on $pool_name") == 0 || die "unable to set zfs properties\n";
 
-- 
2.39.2

[pve-devel] [PATCH installer v2 4/4] zfs: set acltype=posix for root-dataset

[Stoiko Ivanov]

journald as a core component tries setting a ACL on the journal files
for (non-root) users and fails on our ZFS installs.
Resulting in dmesg being spammed with messages from journald upon each
journal-rotation for each user upon their first login.

This is also suggested by OpenZFS in their Debian guide for root on
ZFS:
https://openzfs.github.io/openzfs-docs/Getting%20Started/Debian/Debian%20Bookworm%20Root%20on%20ZFS.html

Tested by setting this on a machine of mine, where this has been
bugging for quite a while.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Proxmox/Install.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 48c157a..1a4ee93 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -202,6 +202,8 @@ sub zfs_create_rpool {
 
     $value = $zfs_opts->{copies} // 1;
     syscmd("zfs set copies=$value $pool_name") if defined($value) && $value != 1;
+
+    syscmd("zfs set acltype=posix $pool_name/ROOT/$root_volume_name");
 }
 
 my $get_raid_devlist = sub {
-- 
2.39.2