All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC: Make kernel upgrades via dnf work like on Red Hat
@ 2021-09-28 14:10 Zoltan Boszormenyi
  2021-09-28 14:10 ` [PATCH 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
  2021-09-28 14:10 ` [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
  0 siblings, 2 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-28 14:10 UTC (permalink / raw)
  To: openembedded-core

I have observed two issues when upgrading to kernel versions
successively.

One is that when installonly_limit is reached in dnf, only the
main kernel package was removed. Patch 1 fixes this by adding
extra RDEPENDS to the kernel subpackages on the main kernel package.
This circular dependency helps dnf to remove all subpackages.

The second issue is that when the oldest kernel version is
removed by dnf, the /boot/bzImage symlink is also gone.

Fix this by using update-alternatives instead of hardcoded ln -s
and rm -f. This is configurable via a new variable.

This is only an RFC at this point, because
1) The first fix is only applied in the KERNEL_SPLIT_MODULES=0 case,
   it may need its own knob or applied unconditionally.

2) There's an extra issue I found while implementing the second patch:
   instead of using KERNEL_VERSION, I had to resort to using PV
   because the former is dynamically set via a python function
   and it changes its value at some point between build phases.
   AFAIK, there are kernel recipes out there using only the major
   version with two numbers in PV instead of the full version triplet.
   I can live with this as my kernel recipe uses the version triplet.

Please advise how to fix and make it final so these fixes can be
accepted into openembedded-core.

Best regards,
Zoltán Böszörményi



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

* [PATCH 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package
  2021-09-28 14:10 RFC: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
@ 2021-09-28 14:10 ` Zoltan Boszormenyi
  2021-09-28 14:10 ` [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
  1 sibling, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-28 14:10 UTC (permalink / raw)
  To: openembedded-core; +Cc: Zoltán Böszörményi

From: Zoltán Böszörményi <zboszor@gmail.com>

Although this creates a circular dependency between the main
kernel package and its subpackages, it helps dnf to automatically
remove all kernel packages of the same version.

Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
---
 meta/classes/kernel.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index d13c38fb02..deccc0e58c 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -105,6 +105,14 @@ python __anonymous () {
             d.appendVar('RDEPENDS:%s-image-%s' % (kname, typelower), ' %s-modules-${KERNEL_VERSION_PKG_NAME} (= ${EXTENDPKGV})' % kname)
             d.setVar('PKG:%s-modules' % kname, '%s-modules-${KERNEL_VERSION_PKG_NAME}' % kname)
             d.appendVar('RPROVIDES:%s-modules' % kname, '%s-modules-${KERNEL_VERSION_PKG_NAME}' % kname)
+            # Reverse RDEPENDS on main kernel package so dnf upgrades can honor installonly_limit
+            # and remove all subpackages of old versions automatically
+            d.appendVar('RDEPENDS:%s-base' % kname, ' %s (= ${EXTENDPKGV})' % kname)
+            d.appendVar('RDEPENDS:%s-image' % kname, ' %s (= ${EXTENDPKGV})' % kname)
+            d.appendVar('RDEPENDS:%s-image-%s' % (kname, typelower), ' %s (= ${EXTENDPKGV})' % kname)
+            d.appendVar('RDEPENDS:%s-modules' % kname, ' %s (= ${EXTENDPKGV})' % kname)
+            if d.getVar('KERNEL_IMAGETYPE') == 'vmlinux':
+                d.appendVar('RDEPENDS:%s-vmlinux' % kname,   ' %s (= ${EXTENDPKGV})' % kname)
 
         d.setVar('PKG:%s-image-%s' % (kname,typelower), '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
         d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
-- 
2.31.1


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

* [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-28 14:10 RFC: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
  2021-09-28 14:10 ` [PATCH 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
@ 2021-09-28 14:10 ` Zoltan Boszormenyi
  2021-09-28 20:27   ` [OE-core] " Bruce Ashfield
  1 sibling, 1 reply; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-28 14:10 UTC (permalink / raw)
  To: openembedded-core; +Cc: Zoltán Böszörményi

From: Zoltán Böszörményi <zboszor@gmail.com>

When using dnf to install new kernel versions and installonly_limit
is reached, dnf automatically removes the oldest kernel version.

However, the /boot/bzImage symlink (or whatever image type is used)
is removed unconditionally.

Allow using the alternative symlink machinery so the highest
kernel version takes precedence and the symlink stays in place.

Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
---
 meta/classes/kernel.bbclass | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index deccc0e58c..a687e5259d 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -43,9 +43,17 @@ KERNEL_VERSION_PKG_NAME = "${@legitimize_package_name(d.getVar('KERNEL_VERSION')
 KERNEL_VERSION_PKG_NAME[vardepvalue] = "${LINUX_VERSION}"
 
 python __anonymous () {
+    import re
     pn = d.getVar("PN")
     kpn = d.getVar("KERNEL_PACKAGE_NAME")
 
+    # KERNEL_VERSION cannot be used here as it would cause
+    # "basehash value changed" issues.
+    kver =  d.getVar("PV")
+    kverp = re.compile('[\.-]')
+    kvparts = kverp.split(kver)
+    kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
+
     # XXX Remove this after bug 11905 is resolved
     #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
     if kpn == pn:
@@ -117,6 +125,9 @@ python __anonymous () {
         d.setVar('PKG:%s-image-%s' % (kname,typelower), '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
         d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
         d.setVar('pkg_postinst:%s-image-%s' % (kname,typelower), """set +e
+if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
+    update-alternatives --install ${KERNEL_IMAGEDEST}/%s %s %s-${KERNEL_VERSION_NAME} %s
+else
 if [ -n "$D" ]; then
     ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
 else
@@ -126,14 +137,19 @@ else
         install -m 0644 ${KERNEL_IMAGEDEST}/%s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s
     fi
 fi
+fi
 set -e
-""" % (type, type, type, type, type, type, type))
+""" % (type, type, type, kverstr, type, type, type, type, type, type, type))
         d.setVar('pkg_postrm:%s-image-%s' % (kname,typelower), """set +e
+if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
+    update-alternatives --remove %s %s-${KERNEL_VERSION_NAME}
+else
 if [ -f "${KERNEL_IMAGEDEST}/%s" -o -L "${KERNEL_IMAGEDEST}/%s" ]; then
     rm -f ${KERNEL_IMAGEDEST}/%s  > /dev/null 2>&1
 fi
+fi
 set -e
-""" % (type, type, type))
+""" % (type, type, type, type, type))
 
 
     image = d.getVar('INITRAMFS_IMAGE')
@@ -214,6 +230,7 @@ KERNEL_RELEASE ?= "${KERNEL_VERSION}"
 # The directory where built kernel lies in the kernel tree
 KERNEL_OUTPUT_DIR ?= "arch/${ARCH}/boot"
 KERNEL_IMAGEDEST ?= "boot"
+KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES ?= "0"
 
 #
 # configuration
-- 
2.31.1


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

* Re: [OE-core] [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-28 14:10 ` [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
@ 2021-09-28 20:27   ` Bruce Ashfield
  2021-09-29  4:02     ` Zoltan Boszormenyi
       [not found]     ` <2609d843-5629-f6c7-f2e6-75fa5888dea9@gmail.com>
  0 siblings, 2 replies; 7+ messages in thread
From: Bruce Ashfield @ 2021-09-28 20:27 UTC (permalink / raw)
  To: Böszörményi Zoltán
  Cc: Patches and discussions about the oe-core layer,
	Zoltán Böszörményi

On Tue, Sep 28, 2021 at 10:16 AM Zoltan Boszormenyi via
lists.openembedded.org <zboszor=pr.hu@lists.openembedded.org> wrote:
>
> From: Zoltán Böszörményi <zboszor@gmail.com>
>
> When using dnf to install new kernel versions and installonly_limit
> is reached, dnf automatically removes the oldest kernel version.

What about other package managers ? Is there a similar limit ? And
does the fallback still work the same ?

I can't say that I'm aware of exactly what this dnf limit is, can we
either link to it, or document the value in the commit message ?

And just so I understand, this is on the install (not the removal)
that dnf is removing the oldest kernel (by its versioning checks) when
the limit is hit ?

>
> However, the /boot/bzImage symlink (or whatever image type is used)
> is removed unconditionally.

And this removal, that's on the package uninstall ? or is that also a
dnf install quirk ?

>
> Allow using the alternative symlink machinery so the highest
> kernel version takes precedence and the symlink stays in place.
>
> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
> ---
>  meta/classes/kernel.bbclass | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index deccc0e58c..a687e5259d 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -43,9 +43,17 @@ KERNEL_VERSION_PKG_NAME = "${@legitimize_package_name(d.getVar('KERNEL_VERSION')
>  KERNEL_VERSION_PKG_NAME[vardepvalue] = "${LINUX_VERSION}"
>
>  python __anonymous () {
> +    import re
>      pn = d.getVar("PN")
>      kpn = d.getVar("KERNEL_PACKAGE_NAME")
>
> +    # KERNEL_VERSION cannot be used here as it would cause
> +    # "basehash value changed" issues.
> +    kver =  d.getVar("PV")
> +    kverp = re.compile('[\.-]')
> +    kvparts = kverp.split(kver)
> +    kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)

It would be really nice to avoid this logic, since in my years of
suffering, PV cannot be trusted on this front.

Why can't this use KERNEL_VERSION_PACKAGE_NAME ? It is already used in
this anonymous python code, and as the vardepexclude (which may just
be what you need to use KERNEL_VERSION directly).

> +
>      # XXX Remove this after bug 11905 is resolved
>      #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>      if kpn == pn:
> @@ -117,6 +125,9 @@ python __anonymous () {
>          d.setVar('PKG:%s-image-%s' % (kname,typelower), '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
>          d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
>          d.setVar('pkg_postinst:%s-image-%s' % (kname,typelower), """set +e
> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
> +    update-alternatives --install ${KERNEL_IMAGEDEST}/%s %s %s-${KERNEL_VERSION_NAME} %s
> +else

I know it is just an inline postinst, but this is starting to get
unreadable quickly.

Shouldn't $D come into play here ? i.e. the existing postinst snippet
is taking it into account for doing the install, update alternatives
should also know if it is defined as well. Shouldn't this be in the
else block of the [ -n "$D" ] test ? if it shouldn't, can the entire
else block be indented to show that it is conditional on the variable
you are introducing.

Are there any situations where update-alternatives isn't available ?
kind of like how we test for ln -sf, and do a fallback if it fails. Is
there a similar case for update-alternatives ?

>  if [ -n "$D" ]; then
>      ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>  else
> @@ -126,14 +137,19 @@ else
>          install -m 0644 ${KERNEL_IMAGEDEST}/%s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s
>      fi
>  fi
> +fi
>  set -e
> -""" % (type, type, type, type, type, type, type))
> +""" % (type, type, type, kverstr, type, type, type, type, type, type, type))
>          d.setVar('pkg_postrm:%s-image-%s' % (kname,typelower), """set +e
> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
> +    update-alternatives --remove %s %s-${KERNEL_VERSION_NAME}
> +else
>  if [ -f "${KERNEL_IMAGEDEST}/%s" -o -L "${KERNEL_IMAGEDEST}/%s" ]; then
>      rm -f ${KERNEL_IMAGEDEST}/%s  > /dev/null 2>&1
>  fi
> +fi
>  set -e
> -""" % (type, type, type))
> +""" % (type, type, type, type, type))
>
>
>      image = d.getVar('INITRAMFS_IMAGE')
> @@ -214,6 +230,7 @@ KERNEL_RELEASE ?= "${KERNEL_VERSION}"
>  # The directory where built kernel lies in the kernel tree
>  KERNEL_OUTPUT_DIR ?= "arch/${ARCH}/boot"
>  KERNEL_IMAGEDEST ?= "boot"
> +KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES ?= "0"

We should add documentation around the new variable as well, even if
the existing variables aren't fully documented .. we can start a trend
of being better.

Cheers,

Bruce


>
>  #
>  # configuration
> --
> 2.31.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#156418): https://lists.openembedded.org/g/openembedded-core/message/156418
> Mute This Topic: https://lists.openembedded.org/mt/85925303/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II


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

* Re: [OE-core] [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-28 20:27   ` [OE-core] " Bruce Ashfield
@ 2021-09-29  4:02     ` Zoltan Boszormenyi
       [not found]     ` <2609d843-5629-f6c7-f2e6-75fa5888dea9@gmail.com>
  1 sibling, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  4:02 UTC (permalink / raw)
  To: Bruce Ashfield, Böszörményi Zoltán
  Cc: Patches and discussions about the oe-core layer

Resending from the subscribed address.

On 2021. 09. 28. 22:27, Bruce Ashfield wrote:
> On Tue, Sep 28, 2021 at 10:16 AM Zoltan Boszormenyi via
> lists.openembedded.org <zboszor=pr.hu@lists.openembedded.org> wrote:
>> From: Zoltán Böszörményi <zboszor@gmail.com>
>>
>> When using dnf to install new kernel versions and installonly_limit
>> is reached, dnf automatically removes the oldest kernel version.
> What about other package managers ? Is there a similar limit ? And
> does the fallback still work the same ?

opkg definitely doesn't know about "install-only" entities.

I think apt has something like dnf, otherwise Debian and Ubuntu
wouldn't be able to offer a boot menu with fallback kernels.

> I can't say that I'm aware of exactly what this dnf limit is, can we
> either link to it, or document the value in the commit message ?

It's a konfiguration knob in /etc/dnf/dnf.conf:

[main]
...
installonly_limit=3
...

> And just so I understand, this is on the install (not the removal)
> that dnf is removing the oldest kernel (by its versioning checks) when
> the limit is hit ?

Yes.

One thing I didn't pursue is that on Fedora (and presumably RHEL)
the currently running kernel is exempted from removal. There may
be cases when you upgrade/install but not reboot with the latest
and it may occur that the currently running kernel is the oldest one.
In this case on Fedora, dnf removes the oldest version that's not
running. I think there's a complete kernel version check including
EXTRAVERSION (from the kernel toplevel Makefile) vs "uname -r".
In Yocto, the toplevel Makefile is not patched with the fully formed
PR value.

>> However, the /boot/bzImage symlink (or whatever image type is used)
>> is removed unconditionally.
> And this removal, that's on the package uninstall ? or is that also a
> dnf install quirk ?

That's on package uninstall and controlled by the postrm script
that the kernel.bbclass adds to the kernel-image-bzimage-<version>
package, i.e. explicit
     ln -sf ... /boot/bzImage
to postinst and
     rm -f /boot/bzImage
to postrm.

This is modified to update-alternatives --install/--remove
if the newly added knob is set to 1. dnf can have multiple
kernel versions installed and the implicit ln -sf/rm -f is also
there for package managers that can keep only one version
from every package.

Now I also have a question. Is it only me, or this [PATCH 2/2]
didn't actually reach everyone? I didn't receive the cover mail
and [PATCH 1/2] back from the mailing list, although I can see
them in the web archive:

https://lists.openembedded.org/g/openembedded-core/message/156420
https://lists.openembedded.org/g/openembedded-core/message/156419

>> Allow using the alternative symlink machinery so the highest
>> kernel version takes precedence and the symlink stays in place.
>>
>> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>> ---
>>    meta/classes/kernel.bbclass | 21 +++++++++++++++++++--
>>    1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index deccc0e58c..a687e5259d 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -43,9 +43,17 @@ KERNEL_VERSION_PKG_NAME = "${@legitimize_package_name(d.getVar('KERNEL_VERSION')
>>    KERNEL_VERSION_PKG_NAME[vardepvalue] = "${LINUX_VERSION}"
>>
>>    python __anonymous () {
>> +    import re
>>        pn = d.getVar("PN")
>>        kpn = d.getVar("KERNEL_PACKAGE_NAME")
>>
>> +    # KERNEL_VERSION cannot be used here as it would cause
>> +    # "basehash value changed" issues.
>> +    kver =  d.getVar("PV")
>> +    kverp = re.compile('[\.-]')
>> +    kvparts = kverp.split(kver)
>> +    kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
> It would be really nice to avoid this logic, since in my years of
> suffering, PV cannot be trusted on this front.
>
> Why can't this use KERNEL_VERSION_PACKAGE_NAME ? It is already used in
> this anonymous python code, and as the vardepexclude (which may just
> be what you need to use KERNEL_VERSION directly).
>
>> +
>>        # XXX Remove this after bug 11905 is resolved
>>        #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>>        if kpn == pn:
>> @@ -117,6 +125,9 @@ python __anonymous () {
>>            d.setVar('PKG:%s-image-%s' % (kname,typelower), '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
>>            d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
>>            d.setVar('pkg_postinst:%s-image-%s' % (kname,typelower), """set +e
>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>> +    update-alternatives --install ${KERNEL_IMAGEDEST}/%s %s %s-${KERNEL_VERSION_NAME} %s
>> +else
> I know it is just an inline postinst, but this is starting to get
> unreadable quickly.
>
> Shouldn't $D come into play here ? i.e. the existing postinst snippet
> is taking it into account for doing the install, update alternatives
> should also know if it is defined as well. Shouldn't this be in the
> else block of the [ -n "$D" ] test ? if it shouldn't, can the entire
> else block be indented to show that it is conditional on the variable
> you are introducing.
>
> Are there any situations where update-alternatives isn't available ?
> kind of like how we test for ln -sf, and do a fallback if it fails. Is
> there a similar case for update-alternatives ?
>
>>    if [ -n "$D" ]; then
>>        ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>>    else
>> @@ -126,14 +137,19 @@ else
>>            install -m 0644 ${KERNEL_IMAGEDEST}/%s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s
>>        fi
>>    fi
>> +fi
>>    set -e
>> -""" % (type, type, type, type, type, type, type))
>> +""" % (type, type, type, kverstr, type, type, type, type, type, type, type))
>>            d.setVar('pkg_postrm:%s-image-%s' % (kname,typelower), """set +e
>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>> +    update-alternatives --remove %s %s-${KERNEL_VERSION_NAME}
>> +else
>>    if [ -f "${KERNEL_IMAGEDEST}/%s" -o -L "${KERNEL_IMAGEDEST}/%s" ]; then
>>        rm -f ${KERNEL_IMAGEDEST}/%s  > /dev/null 2>&1
>>    fi
>> +fi
>>    set -e
>> -""" % (type, type, type))
>> +""" % (type, type, type, type, type))
>>
>>
>>        image = d.getVar('INITRAMFS_IMAGE')
>> @@ -214,6 +230,7 @@ KERNEL_RELEASE ?= "${KERNEL_VERSION}"
>>    # The directory where built kernel lies in the kernel tree
>>    KERNEL_OUTPUT_DIR ?= "arch/${ARCH}/boot"
>>    KERNEL_IMAGEDEST ?= "boot"
>> +KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES ?= "0"
> We should add documentation around the new variable as well, even if
> the existing variables aren't fully documented .. we can start a trend
> of being better.
>
> Cheers,
>
> Bruce
>
>
>>    #
>>    # configuration
>> --
>> 2.31.1
>>
>>
>> 
>>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II



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

* Re: [OE-core] [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
       [not found]     ` <2609d843-5629-f6c7-f2e6-75fa5888dea9@gmail.com>
@ 2021-09-29  4:18       ` Zoltan Boszormenyi
       [not found]       ` <16A92FFE7CB66393.23985@lists.openembedded.org>
  1 sibling, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  4:18 UTC (permalink / raw)
  To: Böszörményi Zoltán, Bruce Ashfield
  Cc: Patches and discussions about the oe-core layer

On 2021. 09. 29. 6:01, Böszörményi Zoltán wrote:
> On 2021. 09. 28. 22:27, Bruce Ashfield wrote:
>> On Tue, Sep 28, 2021 at 10:16 AM Zoltan Boszormenyi via
>> lists.openembedded.org <zboszor=pr.hu@lists.openembedded.org> wrote:
>>> From: Zoltán Böszörményi <zboszor@gmail.com>
>>>
>>> When using dnf to install new kernel versions and installonly_limit
>>> is reached, dnf automatically removes the oldest kernel version.
>> What about other package managers ? Is there a similar limit ? And
>> does the fallback still work the same ?
> 
> opkg definitely doesn't know about "install-only" entities.
> 
> I think apt has something like dnf, otherwise Debian and Ubuntu
> wouldn't be able to offer a boot menu with fallback kernels.
> 
>> I can't say that I'm aware of exactly what this dnf limit is, can we
>> either link to it, or document the value in the commit message ?
> 
> It's a konfiguration knob in /etc/dnf/dnf.conf:
> 
> [main]
> ...
> installonly_limit=3
> ...
> 
>> And just so I understand, this is on the install (not the removal)
>> that dnf is removing the oldest kernel (by its versioning checks) when
>> the limit is hit ?
> 
> Yes.
> 
> One thing I didn't pursue is that on Fedora (and presumably RHEL)
> the currently running kernel is exempted from removal. There may
> be cases when you upgrade/install but not reboot with the latest
> and it may occur that the currently running kernel is the oldest one.
> In this case on Fedora, dnf removes the oldest version that's not
> running. I think there's a complete kernel version check including
> EXTRAVERSION (from the kernel toplevel Makefile) vs "uname -r".
> In Yocto, the toplevel Makefile is not patched with the fully formed
> PR value.
> 
>>> However, the /boot/bzImage symlink (or whatever image type is used)
>>> is removed unconditionally.
>> And this removal, that's on the package uninstall ? or is that also a
>> dnf install quirk ?
> 
> That's on package uninstall and controlled by the postrm script
> that the kernel.bbclass adds to the kernel-image-bzimage-<version>
> package, i.e. explicit
>      ln -sf ... /boot/bzImage
> to postinst and
>      rm -f /boot/bzImage
> to postrm.
> 
> This is modified to update-alternatives --install/--remove
> if the newly added knob is set to 1. dnf can have multiple
> kernel versions installed and the implicit ln -sf/rm -f is also
> there for package managers that can keep only one version
> from every package.
> 
> Now I also have a question. Is it only me, or this [PATCH 2/2]
> didn't actually reach everyone?

Sorry, I meant "the other mails from this series besides [PATCH 2/2]"

> I didn't receive the cover mail
> and [PATCH 1/2] back from the mailing list, although I can see
> them in the web archive:
> 
> https://lists.openembedded.org/g/openembedded-core/message/156420
> https://lists.openembedded.org/g/openembedded-core/message/156419
> 
>>> Allow using the alternative symlink machinery so the highest
>>> kernel version takes precedence and the symlink stays in place.
>>>
>>> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>>> ---
>>>   meta/classes/kernel.bbclass | 21 +++++++++++++++++++--
>>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>>> index deccc0e58c..a687e5259d 100644
>>> --- a/meta/classes/kernel.bbclass
>>> +++ b/meta/classes/kernel.bbclass
>>> @@ -43,9 +43,17 @@ KERNEL_VERSION_PKG_NAME = 
>>> "${@legitimize_package_name(d.getVar('KERNEL_VERSION')
>>>   KERNEL_VERSION_PKG_NAME[vardepvalue] = "${LINUX_VERSION}"
>>>
>>>   python __anonymous () {
>>> +    import re
>>>       pn = d.getVar("PN")
>>>       kpn = d.getVar("KERNEL_PACKAGE_NAME")
>>>
>>> +    # KERNEL_VERSION cannot be used here as it would cause
>>> +    # "basehash value changed" issues.
>>> +    kver =  d.getVar("PV")
>>> +    kverp = re.compile('[\.-]')
>>> +    kvparts = kverp.split(kver)
>>> +    kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
>> It would be really nice to avoid this logic, since in my years of
>> suffering, PV cannot be trusted on this front.
>>
>> Why can't this use KERNEL_VERSION_PACKAGE_NAME ? It is already used in
>> this anonymous python code, and as the vardepexclude (which may just
>> be what you need to use KERNEL_VERSION directly).
>>
>>> +
>>>       # XXX Remove this after bug 11905 is resolved
>>>       #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>>>       if kpn == pn:
>>> @@ -117,6 +125,9 @@ python __anonymous () {
>>>           d.setVar('PKG:%s-image-%s' % (kname,typelower), 
>>> '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
>>>           d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
>>>           d.setVar('pkg_postinst:%s-image-%s' % (kname,typelower), """set +e
>>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>>> +    update-alternatives --install ${KERNEL_IMAGEDEST}/%s %s %s-${KERNEL_VERSION_NAME} %s
>>> +else
>> I know it is just an inline postinst, but this is starting to get
>> unreadable quickly.
>>
>> Shouldn't $D come into play here ? i.e. the existing postinst snippet
>> is taking it into account for doing the install, update alternatives
>> should also know if it is defined as well. Shouldn't this be in the
>> else block of the [ -n "$D" ] test ? if it shouldn't, can the entire
>> else block be indented to show that it is conditional on the variable
>> you are introducing.
>>
>> Are there any situations where update-alternatives isn't available ?
>> kind of like how we test for ln -sf, and do a fallback if it fails. Is
>> there a similar case for update-alternatives ?
>>
>>>   if [ -n "$D" ]; then
>>>       ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>>>   else
>>> @@ -126,14 +137,19 @@ else
>>>           install -m 0644 ${KERNEL_IMAGEDEST}/%s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s
>>>       fi
>>>   fi
>>> +fi
>>>   set -e
>>> -""" % (type, type, type, type, type, type, type))
>>> +""" % (type, type, type, kverstr, type, type, type, type, type, type, type))
>>>           d.setVar('pkg_postrm:%s-image-%s' % (kname,typelower), """set +e
>>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>>> +    update-alternatives --remove %s %s-${KERNEL_VERSION_NAME}
>>> +else
>>>   if [ -f "${KERNEL_IMAGEDEST}/%s" -o -L "${KERNEL_IMAGEDEST}/%s" ]; then
>>>       rm -f ${KERNEL_IMAGEDEST}/%s  > /dev/null 2>&1
>>>   fi
>>> +fi
>>>   set -e
>>> -""" % (type, type, type))
>>> +""" % (type, type, type, type, type))
>>>
>>>
>>>       image = d.getVar('INITRAMFS_IMAGE')
>>> @@ -214,6 +230,7 @@ KERNEL_RELEASE ?= "${KERNEL_VERSION}"
>>>   # The directory where built kernel lies in the kernel tree
>>>   KERNEL_OUTPUT_DIR ?= "arch/${ARCH}/boot"
>>>   KERNEL_IMAGEDEST ?= "boot"
>>> +KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES ?= "0"
>> We should add documentation around the new variable as well, even if
>> the existing variables aren't fully documented .. we can start a trend
>> of being better.
>>
>> Cheers,
>>
>> Bruce
>>
>>
>>>   #
>>>   # configuration
>>> -- 
>>> 2.31.1
>>>
>>>
>>> 
>>>
>>
>> -- 
>> - Thou shalt not follow the NULL pointer, for chaos and madness await
>> thee at its end
>> - "Use the force Harry" - Gandalf, Star Trek II
> 


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

* Re: [OE-core] [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
       [not found]       ` <16A92FFE7CB66393.23985@lists.openembedded.org>
@ 2021-09-29  4:41         ` Zoltan Boszormenyi
  0 siblings, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  4:41 UTC (permalink / raw)
  To: openembedded-core

On 2021. 09. 29. 6:18, Zoltan Boszormenyi via lists.openembedded.org wrote:
> On 2021. 09. 29. 6:01, Böszörményi Zoltán wrote:
>> On 2021. 09. 28. 22:27, Bruce Ashfield wrote:
>>> On Tue, Sep 28, 2021 at 10:16 AM Zoltan Boszormenyi via
>>> lists.openembedded.org <zboszor=pr.hu@lists.openembedded.org> wrote:
>>>> From: Zoltán Böszörményi <zboszor@gmail.com>
>>>>
>>>> When using dnf to install new kernel versions and installonly_limit
>>>> is reached, dnf automatically removes the oldest kernel version.
>>> What about other package managers ? Is there a similar limit ? And
>>> does the fallback still work the same ?
>>
>> opkg definitely doesn't know about "install-only" entities.
>>
>> I think apt has something like dnf, otherwise Debian and Ubuntu
>> wouldn't be able to offer a boot menu with fallback kernels.
>>
>>> I can't say that I'm aware of exactly what this dnf limit is, can we
>>> either link to it, or document the value in the commit message ?
>>
>> It's a konfiguration knob in /etc/dnf/dnf.conf:
>>
>> [main]
>> ...
>> installonly_limit=3
>> ...

Sorry for not including the documentation link,
I was before my morning coffee. Here it is:

https://dnf.readthedocs.io/en/latest/conf_ref.html

Look for "installonlypkgs" and "installonly_limit".

FYI, "installonlypkgs" does include the package named
"kernel" by default and the value set in dnf.conf only
appends to the list of package names.

I will resend a v2 with this added to the commit message
and a fix for parsing/splitting PV, which also seems to
be unstable between very early parsing stages and later build
stages but it doesn't cause "basehash value changes" issues
but it does cause "IndexError: list index out of range"
when trying to use elements from the split PV array when
PV is likely not set yet.

>>
>>> And just so I understand, this is on the install (not the removal)
>>> that dnf is removing the oldest kernel (by its versioning checks) when
>>> the limit is hit ?
>>
>> Yes.
>>
>> One thing I didn't pursue is that on Fedora (and presumably RHEL)
>> the currently running kernel is exempted from removal. There may
>> be cases when you upgrade/install but not reboot with the latest
>> and it may occur that the currently running kernel is the oldest one.
>> In this case on Fedora, dnf removes the oldest version that's not
>> running. I think there's a complete kernel version check including
>> EXTRAVERSION (from the kernel toplevel Makefile) vs "uname -r".
>> In Yocto, the toplevel Makefile is not patched with the fully formed
>> PR value.
>>
>>>> However, the /boot/bzImage symlink (or whatever image type is used)
>>>> is removed unconditionally.
>>> And this removal, that's on the package uninstall ? or is that also a
>>> dnf install quirk ?
>>
>> That's on package uninstall and controlled by the postrm script
>> that the kernel.bbclass adds to the kernel-image-bzimage-<version>
>> package, i.e. explicit
>>      ln -sf ... /boot/bzImage
>> to postinst and
>>      rm -f /boot/bzImage
>> to postrm.
>>
>> This is modified to update-alternatives --install/--remove
>> if the newly added knob is set to 1. dnf can have multiple
>> kernel versions installed and the implicit ln -sf/rm -f is also
>> there for package managers that can keep only one version
>> from every package.
>>
>> Now I also have a question. Is it only me, or this [PATCH 2/2]
>> didn't actually reach everyone?
> 
> Sorry, I meant "the other mails from this series besides [PATCH 2/2]"
> 
>> I didn't receive the cover mail
>> and [PATCH 1/2] back from the mailing list, although I can see
>> them in the web archive:
>>
>> https://lists.openembedded.org/g/openembedded-core/message/156420
>> https://lists.openembedded.org/g/openembedded-core/message/156419
>>
>>>> Allow using the alternative symlink machinery so the highest
>>>> kernel version takes precedence and the symlink stays in place.
>>>>
>>>> Signed-off-by: Zoltán Böszörményi <zboszor@gmail.com>
>>>> ---
>>>>   meta/classes/kernel.bbclass | 21 +++++++++++++++++++--
>>>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>>>> index deccc0e58c..a687e5259d 100644
>>>> --- a/meta/classes/kernel.bbclass
>>>> +++ b/meta/classes/kernel.bbclass
>>>> @@ -43,9 +43,17 @@ KERNEL_VERSION_PKG_NAME = 
>>>> "${@legitimize_package_name(d.getVar('KERNEL_VERSION')
>>>>   KERNEL_VERSION_PKG_NAME[vardepvalue] = "${LINUX_VERSION}"
>>>>
>>>>   python __anonymous () {
>>>> +    import re
>>>>       pn = d.getVar("PN")
>>>>       kpn = d.getVar("KERNEL_PACKAGE_NAME")
>>>>
>>>> +    # KERNEL_VERSION cannot be used here as it would cause
>>>> +    # "basehash value changed" issues.
>>>> +    kver =  d.getVar("PV")
>>>> +    kverp = re.compile('[\.-]')
>>>> +    kvparts = kverp.split(kver)
>>>> +    kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
>>> It would be really nice to avoid this logic, since in my years of
>>> suffering, PV cannot be trusted on this front.
>>>
>>> Why can't this use KERNEL_VERSION_PACKAGE_NAME ? It is already used in
>>> this anonymous python code, and as the vardepexclude (which may just
>>> be what you need to use KERNEL_VERSION directly).
>>>
>>>> +
>>>>       # XXX Remove this after bug 11905 is resolved
>>>>       #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>>>>       if kpn == pn:
>>>> @@ -117,6 +125,9 @@ python __anonymous () {
>>>>           d.setVar('PKG:%s-image-%s' % (kname,typelower), 
>>>> '%s-image-%s-${KERNEL_VERSION_PKG_NAME}' % (kname, typelower))
>>>>           d.setVar('ALLOW_EMPTY:%s-image-%s' % (kname, typelower), '1')
>>>>           d.setVar('pkg_postinst:%s-image-%s' % (kname,typelower), """set +e
>>>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>>>> +    update-alternatives --install ${KERNEL_IMAGEDEST}/%s %s %s-${KERNEL_VERSION_NAME} %s
>>>> +else
>>> I know it is just an inline postinst, but this is starting to get
>>> unreadable quickly.
>>>
>>> Shouldn't $D come into play here ? i.e. the existing postinst snippet
>>> is taking it into account for doing the install, update alternatives
>>> should also know if it is defined as well. Shouldn't this be in the
>>> else block of the [ -n "$D" ] test ? if it shouldn't, can the entire
>>> else block be indented to show that it is conditional on the variable
>>> you are introducing.
>>>
>>> Are there any situations where update-alternatives isn't available ?
>>> kind of like how we test for ln -sf, and do a fallback if it fails. Is
>>> there a similar case for update-alternatives ?
>>>
>>>>   if [ -n "$D" ]; then
>>>>       ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>>>>   else
>>>> @@ -126,14 +137,19 @@ else
>>>>           install -m 0644 ${KERNEL_IMAGEDEST}/%s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s
>>>>       fi
>>>>   fi
>>>> +fi
>>>>   set -e
>>>> -""" % (type, type, type, type, type, type, type))
>>>> +""" % (type, type, type, kverstr, type, type, type, type, type, type, type))
>>>>           d.setVar('pkg_postrm:%s-image-%s' % (kname,typelower), """set +e
>>>> +if [ "${KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES}" != "0" ]; then
>>>> +    update-alternatives --remove %s %s-${KERNEL_VERSION_NAME}
>>>> +else
>>>>   if [ -f "${KERNEL_IMAGEDEST}/%s" -o -L "${KERNEL_IMAGEDEST}/%s" ]; then
>>>>       rm -f ${KERNEL_IMAGEDEST}/%s  > /dev/null 2>&1
>>>>   fi
>>>> +fi
>>>>   set -e
>>>> -""" % (type, type, type))
>>>> +""" % (type, type, type, type, type))
>>>>
>>>>
>>>>       image = d.getVar('INITRAMFS_IMAGE')
>>>> @@ -214,6 +230,7 @@ KERNEL_RELEASE ?= "${KERNEL_VERSION}"
>>>>   # The directory where built kernel lies in the kernel tree
>>>>   KERNEL_OUTPUT_DIR ?= "arch/${ARCH}/boot"
>>>>   KERNEL_IMAGEDEST ?= "boot"
>>>> +KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES ?= "0"
>>> We should add documentation around the new variable as well, even if
>>> the existing variables aren't fully documented .. we can start a trend
>>> of being better.
>>>
>>> Cheers,
>>>
>>> Bruce
>>>
>>>
>>>>   #
>>>>   # configuration
>>>> -- 
>>>> 2.31.1
>>>>
>>>>
>>>>
>>>>
>>>
>>> -- 
>>> - Thou shalt not follow the NULL pointer, for chaos and madness await
>>> thee at its end
>>> - "Use the force Harry" - Gandalf, Star Trek II
>>
> 
> 
> 
> 
> 


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

end of thread, other threads:[~2021-09-29  4:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 14:10 RFC: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
2021-09-28 14:10 ` [PATCH 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
2021-09-28 14:10 ` [PATCH 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
2021-09-28 20:27   ` [OE-core] " Bruce Ashfield
2021-09-29  4:02     ` Zoltan Boszormenyi
     [not found]     ` <2609d843-5629-f6c7-f2e6-75fa5888dea9@gmail.com>
2021-09-29  4:18       ` Zoltan Boszormenyi
     [not found]       ` <16A92FFE7CB66393.23985@lists.openembedded.org>
2021-09-29  4:41         ` Zoltan Boszormenyi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.