All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC [v2]: Make kernel upgrades via dnf work like on Red Hat
@ 2021-09-29  5:33 Zoltan Boszormenyi
  2021-09-29  5:33 ` [PATCH v2 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  5:33 UTC (permalink / raw)
  To: openembedded-core
  Cc: Zoltán Böszörményi, Bruce Ashfield,
	Richard Purdie, Khem Raj

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.

dnf.conf settings "installonlypkgs", "installonly_limit" and
others are documented at https://dnf.readthedocs.io/en/latest/conf_ref.html

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.

3) The new knob name for patch #2 is KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES.
   It is admittedly too chatty, it can be renamed if someone suggests
   a better name.

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 v2 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package
  2021-09-29  5:33 RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
@ 2021-09-29  5:33 ` Zoltan Boszormenyi
  2021-09-29  5:33 ` [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
  2021-09-29 16:06 ` RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Bruce Ashfield
  2 siblings, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  5:33 UTC (permalink / raw)
  To: openembedded-core
  Cc: Zoltán Böszörményi, Bruce Ashfield,
	Richard Purdie, Khem Raj

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 4acec1877e..882858e22e 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 v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-29  5:33 RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
  2021-09-29  5:33 ` [PATCH v2 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
@ 2021-09-29  5:33 ` Zoltan Boszormenyi
  2021-09-29 16:07   ` Bruce Ashfield
  2021-09-29 16:06 ` RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Bruce Ashfield
  2 siblings, 1 reply; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29  5:33 UTC (permalink / raw)
  To: openembedded-core
  Cc: Zoltán Böszörményi, Bruce Ashfield,
	Richard Purdie, Khem Raj

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.

For the dnf.conf documentation on installonlypkgs, installonly_limit
and others settings, see:
https://dnf.readthedocs.io/en/latest/conf_ref.html

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>
---
v2: Mention dnf.conf documentation link
    Protect against "IndexError: list index out of range"

 meta/classes/kernel.bbclass | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 882858e22e..47a78f0dd2 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -43,9 +43,23 @@ 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)
+    # It seems PV is unset or empty for early recipe parsing
+    # but __anonymous functions are run nevertheless.
+    # Protect against "IndexError: list index out of range".
+    if len(kvparts) >= 3:
+        kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
+    else:
+        kverstr = '000000'
+
     # XXX Remove this after bug 11905 is resolved
     #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
     if kpn == pn:
@@ -117,6 +131,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 +143,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 +236,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: RFC [v2]: Make kernel upgrades via dnf work like on Red Hat
  2021-09-29  5:33 RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
  2021-09-29  5:33 ` [PATCH v2 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
  2021-09-29  5:33 ` [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
@ 2021-09-29 16:06 ` Bruce Ashfield
  2021-09-29 17:29   ` Zoltan Boszormenyi
  2 siblings, 1 reply; 7+ messages in thread
From: Bruce Ashfield @ 2021-09-29 16:06 UTC (permalink / raw)
  To: Zoltán Böszörményi
  Cc: Patches and discussions about the oe-core layer,
	Zoltán Böszörményi, Richard Purdie, Khem Raj

On Wed, Sep 29, 2021 at 1:34 AM Zoltán Böszörményi <zboszor@pr.hu> wrote:
>
> 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.
>
> dnf.conf settings "installonlypkgs", "installonly_limit" and
> others are documented at https://dnf.readthedocs.io/en/latest/conf_ref.html
>
> 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.

I'll comment on patch 2/2 directly, but I have a few things here as well.

>
> 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.

What is the outcome of the circular dependency you mention in patch
1/1 ? Does DNF warn, or something else ? What do the other package
managers do when the circular dependency is detected ?

But yes, I agree that we really don't want any more conditional code
in the kernel packaging, since it isn't going to get tested. So
whatever we do needs to work on all package managers and with or
without kernel module splitting.

>
> 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.
>
> 3) The new knob name for patch #2 is KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES.
>    It is admittedly too chatty, it can be renamed if someone suggests
>    a better name.

I'll ponder this as well, but nothing is immediately coming to mind.

Bruce

>
> 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
>
>


-- 
- 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: [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-29  5:33 ` [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
@ 2021-09-29 16:07   ` Bruce Ashfield
  2021-09-29 17:46     ` Zoltan Boszormenyi
  0 siblings, 1 reply; 7+ messages in thread
From: Bruce Ashfield @ 2021-09-29 16:07 UTC (permalink / raw)
  To: Zoltán Böszörményi
  Cc: Patches and discussions about the oe-core layer,
	Zoltán Böszörményi, Richard Purdie, Khem Raj

On Wed, Sep 29, 2021 at 1:34 AM Zoltán Böszörményi <zboszor@pr.hu> 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.
>
> For the dnf.conf documentation on installonlypkgs, installonly_limit
> and others settings, see:
> https://dnf.readthedocs.io/en/latest/conf_ref.html
>
> 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>
> ---
> v2: Mention dnf.conf documentation link
>     Protect against "IndexError: list index out of range"

There were other comments / questions on v1 that were missed. I'll add them
in this reply again (at least in short form).

We also need to document the new variable that is controlling the functionality

As you mentioned in the RFC cover letter, If this only works with rpm
then that needs to be checked, and we shouldn't allow the enablement
to do anything if it doesn't work or if what update-alternatives + the
package manager of choice isn't fully understood in all cases.

>
>  meta/classes/kernel.bbclass | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 882858e22e..47a78f0dd2 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -43,9 +43,23 @@ 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)
> +    # It seems PV is unset or empty for early recipe parsing
> +    # but __anonymous functions are run nevertheless.
> +    # Protect against "IndexError: list index out of range".
> +    if len(kvparts) >= 3:
> +        kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
> +    else:
> +        kverstr = '000000'
> +

I was asking about using something else than PV, since we are now up
to about three different variables being used to check versions. The
basehash question / issue is valid, but at this point we are packaging
based on something parsed out of the Makefile and then doing postinst
/ update alternatives steps based on PV. It is just going to cause a
lot of pain. And as you mentioned in your cover letter, it isn't
consistently used .. and hence we can't rely on it in this common
code.

I've asked around a bit, and we need to consider getting this code out
of the anonymous python completely. See how classes/fontcache.bbclass
uses PACKAGEFUNCS to create the postfunc steps. Since this isn't used
until packaging time, then it does need to move something that is run
in the proper context and we can avoid basehash and other issues.


>      # XXX Remove this after bug 11905 is resolved
>      #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>      if kpn == pn:
> @@ -117,6 +131,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

Here is where I was asking about why this isn't checking $D like the
previous code block.  Unless we want this running on both the host and
the target, it needs a check.

>  if [ -n "$D" ]; then

And now this block of code isn't indented and is in a big 'else'
block. Sure it is just a scriptlet, but it is getting hard to read and
follow.

Cheers,

Bruce

>      ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>  else
> @@ -126,14 +143,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 +236,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
>


--
- 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: RFC [v2]: Make kernel upgrades via dnf work like on Red Hat
  2021-09-29 16:06 ` RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Bruce Ashfield
@ 2021-09-29 17:29   ` Zoltan Boszormenyi
  0 siblings, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29 17:29 UTC (permalink / raw)
  To: Bruce Ashfield
  Cc: Patches and discussions about the oe-core layer,
	Zoltán Böszörményi, Richard Purdie, Khem Raj

On 2021. 09. 29. 18:06, Bruce Ashfield wrote:
> On Wed, Sep 29, 2021 at 1:34 AM Zoltán Böszörményi <zboszor@pr.hu> wrote:
>>
>> 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.
>>
>> dnf.conf settings "installonlypkgs", "installonly_limit" and
>> others are documented at https://dnf.readthedocs.io/en/latest/conf_ref.html
>>
>> 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.
> 
> I'll comment on patch 2/2 directly, but I have a few things here as well.
> 
>>
>> 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.
> 
> What is the outcome of the circular dependency you mention in patch
> 1/1 ? Does DNF warn, or something else ? What do the other package
> managers do when the circular dependency is detected ?

No warning, it's just that the dnf transaction looks like this:

# dnf --disablerepo="*" install kernel-*
Dependencies resolved.
================================================================================
  Package                     Architecture    Version        Repository     Size
================================================================================
Installing:
  kernel                      intel_corei7_64 5.14.8-r0.0    @commandline  6.4 k
  kernel-5.14.8               intel_corei7_64 5.14.8-r0.0    @commandline   48 k
  kernel-image-5.14.8         intel_corei7_64 5.14.8-r0.0    @commandline  6.6 k
  kernel-image-bzimage-5.14.8 intel_corei7_64 5.14.8-r0.0    @commandline  9.1 M
  kernel-modules-5.14.8       intel_corei7_64 5.14.8-r0.0    @commandline   53 M
Removing:
  kernel                      intel_corei7_64 5.14.5-r0.0    @@commandline  88
Transaction Summary
================================================================================
Install  5 Packages
Remove   1 Packages
Total size: 75 M
Is this ok [y/N]:

instead of:

# dnf --disablerepo="*" install kernel-*
Dependencies resolved.
================================================================================
  Package                     Architecture    Version        Repository     Size
================================================================================
Installing:
  kernel                      intel_corei7_64 5.14.8-r0.0    @commandline  6.4 k
  kernel-5.14.8               intel_corei7_64 5.14.8-r0.0    @commandline   48 k
  kernel-image-5.14.8         intel_corei7_64 5.14.8-r0.0    @commandline  6.6 k
  kernel-image-bzimage-5.14.8 intel_corei7_64 5.14.8-r0.0    @commandline  9.1 M
  kernel-modules-5.14.8       intel_corei7_64 5.14.8-r0.0    @commandline   53 M
Removing:
  kernel                      intel_corei7_64 5.14.5-r0.0    @@commandline  88
Removing dependent packages:
  kernel-5.14.5               intel_corei7_64 5.14.5-r0.0    @@commandline 222 k
  kernel-image-5.14.5         intel_corei7_64 5.14.5-r0.0    @@commandline   0
  kernel-image-bzimage-5.14.5 intel_corei7_64 5.14.5-r0.0    @@commandline 9.2 M
  kernel-modules-5.14.5       intel_corei7_64 5.14.5-r0.0    @@commandline  52 M
Transaction Summary
================================================================================
Install  5 Packages
Remove   5 Packages
Total size: 75 M
Is this ok [y/N]:

Since the dependency matches the exact version via ${EXTENDPKGV},
I would expect other package managers to do what dnf is doing.

At least, IIRC, opkg built with libsolv does.

> But yes, I agree that we really don't want any more conditional code
> in the kernel packaging, since it isn't going to get tested. So
> whatever we do needs to work on all package managers and with or
> without kernel module splitting.
> 
>>
>> 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.
>>
>> 3) The new knob name for patch #2 is KERNEL_IMAGEDEST_USE_UPDATE_ALTERNATIVES.
>>     It is admittedly too chatty, it can be renamed if someone suggests
>>     a better name.
> 
> I'll ponder this as well, but nothing is immediately coming to mind.
> 
> Bruce
> 
>>
>> 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

* Re: [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image
  2021-09-29 16:07   ` Bruce Ashfield
@ 2021-09-29 17:46     ` Zoltan Boszormenyi
  0 siblings, 0 replies; 7+ messages in thread
From: Zoltan Boszormenyi @ 2021-09-29 17:46 UTC (permalink / raw)
  To: Bruce Ashfield
  Cc: Patches and discussions about the oe-core layer,
	Zoltán Böszörményi, Richard Purdie, Khem Raj

On 2021. 09. 29. 18:07, Bruce Ashfield wrote:
> On Wed, Sep 29, 2021 at 1:34 AM Zoltán Böszörményi <zboszor@pr.hu> 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.
>>
>> For the dnf.conf documentation on installonlypkgs, installonly_limit
>> and others settings, see:
>> https://dnf.readthedocs.io/en/latest/conf_ref.html
>>
>> 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>
>> ---
>> v2: Mention dnf.conf documentation link
>>      Protect against "IndexError: list index out of range"
> 
> There were other comments / questions on v1 that were missed. I'll add them
> in this reply again (at least in short form).
> 
> We also need to document the new variable that is controlling the functionality
> 
> As you mentioned in the RFC cover letter, If this only works with rpm
> then that needs to be checked, and we shouldn't allow the enablement
> to do anything if it doesn't work or if what update-alternatives + the
> package manager of choice isn't fully understood in all cases.
> 
>>
>>   meta/classes/kernel.bbclass | 27 +++++++++++++++++++++++++--
>>   1 file changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index 882858e22e..47a78f0dd2 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -43,9 +43,23 @@ 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)
>> +    # It seems PV is unset or empty for early recipe parsing
>> +    # but __anonymous functions are run nevertheless.
>> +    # Protect against "IndexError: list index out of range".
>> +    if len(kvparts) >= 3:
>> +        kverstr = str(kvparts[0])+str(kvparts[1]).zfill(2)+str(kvparts[2]).zfill(3)
>> +    else:
>> +        kverstr = '000000'
>> +
> 
> I was asking about using something else than PV, since we are now up
> to about three different variables being used to check versions. The
> basehash question / issue is valid, but at this point we are packaging
> based on something parsed out of the Makefile and then doing postinst
> / update alternatives steps based on PV. It is just going to cause a
> lot of pain. And as you mentioned in your cover letter, it isn't
> consistently used .. and hence we can't rely on it in this common
> code.

That's why the series is RFC because the added inconsistency was painful to me.

> I've asked around a bit, and we need to consider getting this code out
> of the anonymous python completely. See how classes/fontcache.bbclass
> uses PACKAGEFUNCS to create the postfunc steps. Since this isn't used
> until packaging time, then it does need to move something that is run
> in the proper context and we can avoid basehash and other issues.

I will look at it, thanks.

>>       # XXX Remove this after bug 11905 is resolved
>>       #  FILES:${KERNEL_PACKAGE_NAME}-dev doesn't expand correctly
>>       if kpn == pn:
>> @@ -117,6 +131,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
> 
> Here is where I was asking about why this isn't checking $D like the
> previous code block.  Unless we want this running on both the host and
> the target, it needs a check.

The update-alternatives intercept script is installed for
for the image (to allow packages work that have "inherit
update-alternatives") and it's doing the right thing during
do_rootfs, internally dealing with $D.

Well, except for one thing, which is my fault here that
I discovered today after inspecting the resulting image.

The symlink path must be "/${KERNEL_IMAGEDEST}/%s".
The starter "/" is missing in v2, I will have to send a v3.

Well, here's "release often, release early" for you, with bugs and all.

> 
>>   if [ -n "$D" ]; then
> 
> And now this block of code isn't indented and is in a big 'else'
> block. Sure it is just a scriptlet, but it is getting hard to read and
> follow.

I will fix the indentation.

Thanks for your comments,
Zoltán

> 
> Cheers,
> 
> Bruce
> 
>>       ln -sf %s-${KERNEL_VERSION} $D/${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>>   else
>> @@ -126,14 +143,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 +236,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
>>
> 
> 
> --
> - 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 17:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  5:33 RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Zoltan Boszormenyi
2021-09-29  5:33 ` [PATCH v2 1/2] kernel.bbclass: Add runtime dependency to subpackages on main package Zoltan Boszormenyi
2021-09-29  5:33 ` [PATCH v2 2/2] kernel.bbclass: Allow using update-alternatives for the kernel image Zoltan Boszormenyi
2021-09-29 16:07   ` Bruce Ashfield
2021-09-29 17:46     ` Zoltan Boszormenyi
2021-09-29 16:06 ` RFC [v2]: Make kernel upgrades via dnf work like on Red Hat Bruce Ashfield
2021-09-29 17:29   ` 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.