[pve-devel] [PATCH installer 0/2] fix 2 small issues with recent installer patches

[pve-devel] [PATCH installer 0/2] fix 2 small issues with recent installer patches

[Stoiko Ivanov]

both issues are unrelated, and can be applied individually

first is that with the recent creation of /var/lib/vz as dedicated dataset,
systems yielded a failed service for var.mount upon shutdown. The fix
simply sets the canmount property for the intermediate dataset var and
var/lib to off - following upstream's guide

the second one fixes that grub did not show up on the serial console after
installation. it follows the recommendations from the archwiki on the
topic:
https://wiki.archlinux.org/title/working_with_the_serial_console#GRUB

minimally tested with a VM here.

Stoiko Ivanov (2):
  zfs: set canmount=off on /var/lib
  serial installer: add serial config for grub to target system

 Proxmox/Install.pm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
2.39.2

[pve-devel] [PATCH installer 1/2] zfs: set canmount=off on /var/lib

[Stoiko Ivanov]

as explained in zfsprops(4) setting canmount to off is similar to
setting mountpoint to none - except that you can still use the dataset
for storing properties to be inherited to children (and we want
/var/lib/vz to have a mountpoint set)

Follows recommendations from upstreams ZFS on / guide:
https://openzfs.github.io/openzfs-docs/Getting%20Started/Debian/Debian%20Bookworm%20Root%20on%20ZFS.html

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

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 1a4ee93..fd9bf84 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -189,6 +189,10 @@ sub zfs_create_rpool {
 	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 set canmount=off $pool_name/ROOT/$root_volume_name/var/lib")  == 0 ||
+	    die "unable to set canmount property on $pool_name/ROOT/$root_volume_name/var/lib\n";
+	syscmd("zfs set canmount=off $pool_name/ROOT/$root_volume_name/var")  == 0 ||
+	    die "unable to set canmount property on $pool_name/ROOT/$root_volume_name/var\n";
     }
 
     # default to `relatime` on, fast enough for the installer and production
-- 
2.39.2

Re: [pve-devel] [PATCH installer 1/2] zfs: set canmount=off on /var/lib

[Fabian Grünbichler]

> Stoiko Ivanov <s.ivanov@proxmox.com> hat am 21.11.2023 12:09 CET geschrieben:
> 
>  
> as explained in zfsprops(4) setting canmount to off is similar to
> setting mountpoint to none - except that you can still use the dataset
> for storing properties to be inherited to children (and we want
> /var/lib/vz to have a mountpoint set)
> 
> Follows recommendations from upstreams ZFS on / guide:
> https://openzfs.github.io/openzfs-docs/Getting%20Started/Debian/Debian%20Bookworm%20Root%20on%20ZFS.html
> 
> Fixes: dd19d40ceac179ba18652f1d6c3e4c23f246af00
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  Proxmox/Install.pm | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
> index 1a4ee93..fd9bf84 100644
> --- a/Proxmox/Install.pm
> +++ b/Proxmox/Install.pm
> @@ -189,6 +189,10 @@ sub zfs_create_rpool {
>  	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 set canmount=off $pool_name/ROOT/$root_volume_name/var/lib")  == 0 ||
> +	    die "unable to set canmount property on $pool_name/ROOT/$root_volume_name/var/lib\n";
> +	syscmd("zfs set canmount=off $pool_name/ROOT/$root_volume_name/var")  == 0 ||
> +	    die "unable to set canmount property on $pool_name/ROOT/$root_volume_name/var\n";

what's the advantage of this as opposed to

zfs create -o mountpoint=/var/lib/vz $pool_name/var-lib-vz

(or some other name for the dataset)

>      }
>  
>      # default to `relatime` on, fast enough for the installer and production
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH installer 2/2] serial installer: add serial config for grub to target system

[Stoiko Ivanov]

Matching if a serial will be needed for grub is based on the target
commandline - the speed is also read from there. The unit is based
on the ttyS device - although I'd assume that this might not always
match up.

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

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index fd9bf84..4293f96 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1162,7 +1162,13 @@ _EOD
 	update_progress(0.8, 0.95, 1, "make system bootable");
 	my $target_cmdline='';
 	if ($target_cmdline = Proxmox::Install::Config::get_target_cmdline()) {
-	    my $target_cmdline_snippet = "GRUB_CMDLINE_LINUX=\"\$GRUB_CMDLINE_LINUX $target_cmdline\"";
+	    my $target_cmdline_snippet = '';
+	    if ($target_cmdline =~ /console=ttyS(\d+),(\d+)/) {
+		$target_cmdline_snippet .= "GRUB_TERMINAL_INPUT=\"console serial\"\n";
+		$target_cmdline_snippet .= "GRUB_TERMINAL_OUTPUT=\"gfxterm serial\"\n";
+		$target_cmdline_snippet .= "GRUB_SERIAL_COMMAND=\"serial --unit=$1 --speed=$2\"\n";
+	    }
+	    $target_cmdline_snippet .= "GRUB_CMDLINE_LINUX=\"\$GRUB_CMDLINE_LINUX $target_cmdline\"";
 	    file_write_all("$targetdir/etc/default/grub.d/installer.cfg", $target_cmdline_snippet);
 	}
 
-- 
2.39.2

[pve-devel] applied: [PATCH installer 2/2] serial installer: add serial config for grub to target system

[Thomas Lamprecht]

Am 21/11/2023 um 12:09 schrieb Stoiko Ivanov:
> Matching if a serial will be needed for grub is based on the target
> commandline - the speed is also read from there. The unit is based
> on the ttyS device - although I'd assume that this might not always
> match up.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  Proxmox/Install.pm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
>

applied this one fore now, thanks!