public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements
@ 2021-07-07 21:09 Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: ignore call to grub-install from grub maintscripts Stoiko Ivanov
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

The following patchset addresses a few small issues reported during the PVE
7.0 beta and after the 7.0 stable release.

* patches 1+2 deal with grub-install being called during a distribution
  upgrade on some systems (I did not manage to get a VM installed with PVE
  6.4 to run into the issue)
* patch 3 addresses an issue where once someone removes pve-kernel-helper
  (without purging it) it becomes quite difficult to get it installed again
  (to remove pve-kernel-helper you need also to remove proxmox-ve, but as
  our forum shows [0] - this happens without the user noticing sometimes)
* patch 4+5 are a few improvements to the `p-b-t status` I consider
  worthwhile


Stoiko Ivanov (5):
  proxmox-boot: ignore call to grub-install from grub maintscripts
  proxmox-boot: divert call to grub-install to p-b-t init
  proxmox-boot: maintscript: change logic whether to add diversion
  proxmox-boot: print current boot mode with status output
  proxmox-boot: status: print present kernel versions

 bin/grub-install-wrapper         | 27 +++++++++++++++++++++++++++
 bin/proxmox-boot-tool            | 13 ++++++++++---
 debian/pve-kernel-helper.preinst |  2 +-
 3 files changed, 38 insertions(+), 4 deletions(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: ignore call to grub-install from grub maintscripts
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
@ 2021-07-07 21:09 ` Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init Stoiko Ivanov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

in certain cases the postinst script of grub-pc runs grub-install on
the disks it gets from debconf. Simply warn and exit with 0 if
grub-install is called by dpkg and from a grub related package

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/grub-install-wrapper | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/bin/grub-install-wrapper b/bin/grub-install-wrapper
index a61e984..35f03fa 100755
--- a/bin/grub-install-wrapper
+++ b/bin/grub-install-wrapper
@@ -4,6 +4,12 @@ set -e
 . /usr/share/pve-kernel-helper/scripts/functions
 
 if proxmox-boot-tool status --quiet; then
+	#detect when being called by dpkg (e.g. grub-pc.postinst
+	if [ -n "$DPKG_RUNNING_VERSION" ] && \
+	echo "$DPKG_MAINTSCRIPT_PACKAGE" | grep -sq "^grub-"; then
+		warn "This system is booted via proxmox-boot-tool, ignoring dpkg call to grub-install"
+		exit 0
+	fi
 	warn "grub-install is disabled because this system is booted via proxmox-boot-tool, if you really need to run it, run /usr/sbin/grub-install.real"
 	exit 1
 else
-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: ignore call to grub-install from grub maintscripts Stoiko Ivanov
@ 2021-07-07 21:09 ` Stoiko Ivanov
  2021-07-08  7:06   ` Fabian Grünbichler
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 3/5] proxmox-boot: maintscript: change logic whether to add diversion Stoiko Ivanov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

This way all ESPs (in case of a legacy booted system) get an
updated grub installation.

running only once between reboots (the markerfile is in /tmp) should
be enough. Sadly the environment does not provide a hint which version
grub is installed to.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/grub-install-wrapper | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/bin/grub-install-wrapper b/bin/grub-install-wrapper
index 35f03fa..2e70789 100755
--- a/bin/grub-install-wrapper
+++ b/bin/grub-install-wrapper
@@ -3,12 +3,33 @@ set -e
 
 . /usr/share/pve-kernel-helper/scripts/functions
 
+init_boot_disks() {
+	if ! (echo "${curr_uuid}" | grep -qE '[0-9a-fA-F]{4}-[0-9a-fA-F]{4}'); then
+		warn "WARN: ${curr_uuid} read from ${ESP_LIST} does not look like a VFAT-UUID - skipping"
+		return
+	fi
+
+	path="/dev/disk/by-uuid/$curr_uuid"
+	if [ ! -e "${path}" ]; then
+		warn "WARN: ${path} does not exist - clean '${ESP_LIST}'! - skipping"
+		return
+	fi
+	proxmox-boot-tool init "$path"
+}
+
 if proxmox-boot-tool status --quiet; then
 	#detect when being called by dpkg (e.g. grub-pc.postinst
 	if [ -n "$DPKG_RUNNING_VERSION" ] && \
 	echo "$DPKG_MAINTSCRIPT_PACKAGE" | grep -sq "^grub-"; then
-		warn "This system is booted via proxmox-boot-tool, ignoring dpkg call to grub-install"
-		exit 0
+		MARKER_FILE="/tmp/proxmox-boot-tool.dpkg.marker"
+		if [ ! -e "$MARKER_FILE" ]; then
+		    warn "This system is booted via proxmox-boot-tool, running proxmox-boot-tool init for all configured bootdisks"
+		    loop_esp_list init_boot_disks
+		    touch "$MARKER_FILE"
+		    exit 0
+		else
+		    exit 0
+		fi
 	fi
 	warn "grub-install is disabled because this system is booted via proxmox-boot-tool, if you really need to run it, run /usr/sbin/grub-install.real"
 	exit 1
-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH pve-kernel-meta 3/5] proxmox-boot: maintscript: change logic whether to add diversion
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: ignore call to grub-install from grub maintscripts Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init Stoiko Ivanov
@ 2021-07-07 21:09 ` Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 4/5] proxmox-boot: print current boot mode with status output Stoiko Ivanov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

Deciding whether or not to add the diversion based on the version
alone fails quite hard in case pve-kernel-helper is in dpkg-state 'rc'
(removed not purged) as reported in our community forum[0]:
* removing pve-kernel-helper removes the diversion of grub-install
* if config-files are still present the preinst script gets called
  with the version of the config-files (the version that got removed)
* if the version was newer than 6.4-1~ then no diversion is added
* unpacking fails, because grub-install would be overwritten leaving
  pve-kernel-helper in state 'ic'

Explicitly checking whether the diversion is in place sounds like a
robust approach here.

downside: documentation on dpkg-divert in maintainer scripts [1] uses
the version approach.

[0] https://forum.proxmox.com/threads/pve-kernel-helper-wont-install.90029/
[1] https://www.debian.org/doc/debian-policy/ap-pkg-diversions.html

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 debian/pve-kernel-helper.preinst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/pve-kernel-helper.preinst b/debian/pve-kernel-helper.preinst
index 9ec726d..e2464c9 100644
--- a/debian/pve-kernel-helper.preinst
+++ b/debian/pve-kernel-helper.preinst
@@ -4,7 +4,7 @@ set -e
 
 case "$1" in
     install|upgrade)
-        if dpkg --compare-versions "$2" lt "6.4-1~"; then
+        if ! dpkg -S /usr/sbin/grub-install|grep -q 'diversion by pve-kernel-helper'; then
             dpkg-divert --package pve-kernel-helper --add --rename \
                 --divert /usr/sbin/grub-install.real /usr/sbin/grub-install
         fi
-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH pve-kernel-meta 4/5] proxmox-boot: print current boot mode with status output
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
                   ` (2 preceding siblings ...)
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 3/5] proxmox-boot: maintscript: change logic whether to add diversion Stoiko Ivanov
@ 2021-07-07 21:09 ` Stoiko Ivanov
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 5/5] proxmox-boot: status: print present kernel versions Stoiko Ivanov
  2021-07-08  8:05 ` [pve-devel] applied-series: [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Thomas Lamprecht
  5 siblings, 0 replies; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

most support questions w.r.t. proxmox-boot-tool do have us
asking for `stat /sys/firmware/efi` output anyways

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 bin/proxmox-boot-tool | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 079fa26..1e984d6 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -381,6 +381,11 @@ status() {
 		exit 2
 	fi
 	if [ -z "$quiet" ]; then
+		if [ -d /sys/firmware/efi ]; then
+		    echo "System currently booted with uefi"
+		else
+		    echo "System currently booted with legacy bios"
+		fi
 		loop_esp_list _status_detail
 	fi
 }
-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH pve-kernel-meta 5/5] proxmox-boot: status: print present kernel versions
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
                   ` (3 preceding siblings ...)
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 4/5] proxmox-boot: print current boot mode with status output Stoiko Ivanov
@ 2021-07-07 21:09 ` Stoiko Ivanov
  2021-07-08  8:05 ` [pve-devel] applied-series: [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Thomas Lamprecht
  5 siblings, 0 replies; 8+ messages in thread
From: Stoiko Ivanov @ 2021-07-07 21:09 UTC (permalink / raw)
  To: pve-devel

gives a better overview in case the system was switched at one time
from uefi to legacy (or the other way around).

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

diff --git a/bin/proxmox-boot-tool b/bin/proxmox-boot-tool
index 1e984d6..93760fb 100755
--- a/bin/proxmox-boot-tool
+++ b/bin/proxmox-boot-tool
@@ -353,16 +353,18 @@ _status_detail() {
 
 	result=""
 	if [ -f "${mountpoint}/$PMX_LOADER_CONF" ]; then
-		result="uefi"
 		if [ ! -d "${mountpoint}/$PMX_ESP_DIR" ]; then
 			warn "${path}/$PMX_ESP_DIR does not exist"
 		fi
+		versions_uefi=$(ls -1 ${mountpoint}/$PMX_ESP_DIR | awk '{printf (NR>1?", ":"") $0}')
+		result="uefi (versions: ${versions_uefi})"
 	fi
 	if [ -d "${mountpoint}/grub" ]; then
+		versions_grub=$(ls -1 ${mountpoint}/vmlinuz-* | awk '{ gsub(/.*\/vmlinuz-/, ""); printf (NR>1?", ":"") $0 }')
 		if [ -n "$result" ]; then
-		    result="${result},grub"
+		    result="${result}, grub (versions: ${versions_grub})"
 		else
-		    result="grub"
+		    result="grub (versions: ${versions_grub})"
 		fi
 	fi
 	echo "$curr_uuid is configured with: $result"
-- 
2.30.2





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init Stoiko Ivanov
@ 2021-07-08  7:06   ` Fabian Grünbichler
  0 siblings, 0 replies; 8+ messages in thread
From: Fabian Grünbichler @ 2021-07-08  7:06 UTC (permalink / raw)
  To: Proxmox VE development discussion

On July 7, 2021 11:09 pm, Stoiko Ivanov wrote:
> This way all ESPs (in case of a legacy booted system) get an
> updated grub installation.
> 
> running only once between reboots (the markerfile is in /tmp) should
> be enough. Sadly the environment does not provide a hint which version
> grub is installed to.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  bin/grub-install-wrapper | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/bin/grub-install-wrapper b/bin/grub-install-wrapper
> index 35f03fa..2e70789 100755
> --- a/bin/grub-install-wrapper
> +++ b/bin/grub-install-wrapper
> @@ -3,12 +3,33 @@ set -e
>  
>  . /usr/share/pve-kernel-helper/scripts/functions
>  
> +init_boot_disks() {
> +	if ! (echo "${curr_uuid}" | grep -qE '[0-9a-fA-F]{4}-[0-9a-fA-F]{4}'); then
> +		warn "WARN: ${curr_uuid} read from ${ESP_LIST} does not look like a VFAT-UUID - skipping"
> +		return
> +	fi
> +
> +	path="/dev/disk/by-uuid/$curr_uuid"
> +	if [ ! -e "${path}" ]; then
> +		warn "WARN: ${path} does not exist - clean '${ESP_LIST}'! - skipping"
> +		return
> +	fi
> +	proxmox-boot-tool init "$path"
> +}
> +
>  if proxmox-boot-tool status --quiet; then
>  	#detect when being called by dpkg (e.g. grub-pc.postinst
>  	if [ -n "$DPKG_RUNNING_VERSION" ] && \
>  	echo "$DPKG_MAINTSCRIPT_PACKAGE" | grep -sq "^grub-"; then

this could have another guard for whether the system is even booted with 
grub (as if the system was booted using EFI, re-initing all ESPs is just 
busy-work), but the current iteration is better than what we currently 
have so that can be a follow-up..

> -		warn "This system is booted via proxmox-boot-tool, ignoring dpkg 
> call to grub-install"
> -		exit 0
> +		MARKER_FILE="/tmp/proxmox-boot-tool.dpkg.marker"
> +		if [ ! -e "$MARKER_FILE" ]; then
> +		    warn "This system is booted via proxmox-boot-tool, running 
> proxmox-boot-tool init for all configured bootdisks"
> +		    loop_esp_list init_boot_disks
> +		    touch "$MARKER_FILE"
> +		    exit 0
> +		else

this else branch could also have some output - it will be rarely hit 
(basically only with repeated reinstalls of grub-pc or systems that are 
never rebooted), but we still ignore the grub-install call here..

> +		    exit 0
> +		fi
>  	fi
>  	warn "grub-install is disabled because this system is booted via 
>  	proxmox-boot-tool, if you really need to run it, run 
>  	/usr/sbin/grub-install.real"
>  	exit 1
> -- 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] applied-series: [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements
  2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
                   ` (4 preceding siblings ...)
  2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 5/5] proxmox-boot: status: print present kernel versions Stoiko Ivanov
@ 2021-07-08  8:05 ` Thomas Lamprecht
  5 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2021-07-08  8:05 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stoiko Ivanov

On 07.07.21 23:09, Stoiko Ivanov wrote:
> The following patchset addresses a few small issues reported during the PVE
> 7.0 beta and after the 7.0 stable release.
> 
> * patches 1+2 deal with grub-install being called during a distribution
>   upgrade on some systems (I did not manage to get a VM installed with PVE
>   6.4 to run into the issue)
> * patch 3 addresses an issue where once someone removes pve-kernel-helper
>   (without purging it) it becomes quite difficult to get it installed again
>   (to remove pve-kernel-helper you need also to remove proxmox-ve, but as
>   our forum shows [0] - this happens without the user noticing sometimes)
> * patch 4+5 are a few improvements to the `p-b-t status` I consider
>   worthwhile
> 
> 
> Stoiko Ivanov (5):
>   proxmox-boot: ignore call to grub-install from grub maintscripts
>   proxmox-boot: divert call to grub-install to p-b-t init
>   proxmox-boot: maintscript: change logic whether to add diversion
>   proxmox-boot: print current boot mode with status output
>   proxmox-boot: status: print present kernel versions
> 
>  bin/grub-install-wrapper         | 27 +++++++++++++++++++++++++++
>  bin/proxmox-boot-tool            | 13 ++++++++++---
>  debian/pve-kernel-helper.preinst |  2 +-
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 



applied series with Fabians comments as followup, thanks!




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-08  8:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 21:09 [pve-devel] [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Stoiko Ivanov
2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 1/5] proxmox-boot: ignore call to grub-install from grub maintscripts Stoiko Ivanov
2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 2/5] proxmox-boot: divert call to grub-install to p-b-t init Stoiko Ivanov
2021-07-08  7:06   ` Fabian Grünbichler
2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 3/5] proxmox-boot: maintscript: change logic whether to add diversion Stoiko Ivanov
2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 4/5] proxmox-boot: print current boot mode with status output Stoiko Ivanov
2021-07-07 21:09 ` [pve-devel] [PATCH pve-kernel-meta 5/5] proxmox-boot: status: print present kernel versions Stoiko Ivanov
2021-07-08  8:05 ` [pve-devel] applied-series: [PATCH pve-kernel-meta 0/5] proxmox-boot-tool improvements Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal