All of lore.kernel.org
 help / color / mirror / Atom feed
* [OE-core] [PATCH 0/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
@ 2020-06-17  6:20 Xu, Yanfei
  2020-06-17  6:20 ` [OE-core] [PATCH 1/1] " Xu, Yanfei
  0 siblings, 1 reply; 6+ messages in thread
From: Xu, Yanfei @ 2020-06-17  6:20 UTC (permalink / raw)
  To: bruce.ashfield, openembedded-core; +Cc: zhe.he

From: Yanfei Xu <yanfei.xu@windriver.com>

Some explains about this patch:
1.Why does it copy a ${imageType} before postintall?
We should ensure the rpm package contain a iamge named ${imageType} 
before runing on target.
2.Why should we have this patch?
Cause some arm boards always mount their booting partition on /boot 
and then upgrade kernel packages. But if the /boot filesystems don't 
 support symlink, it will fail.


Yanfei Xu (1):
  classes/kernel: Use a copy for kernel*.rpm when fs don't support
    symlink

 meta/classes/kernel.bbclass | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.18.2


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

* [OE-core] [PATCH 1/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
  2020-06-17  6:20 [OE-core] [PATCH 0/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink Xu, Yanfei
@ 2020-06-17  6:20 ` Xu, Yanfei
  2020-06-17 13:48   ` Bruce Ashfield
  0 siblings, 1 reply; 6+ messages in thread
From: Xu, Yanfei @ 2020-06-17  6:20 UTC (permalink / raw)
  To: bruce.ashfield, openembedded-core; +Cc: zhe.he

From: Yanfei Xu <yanfei.xu@windriver.com>

Some filesystems don't support symlink, then you will get failed when
you install or update the kernel rpm package. Now we use a copy for
these filesystems instead of symlink.

Suggested-by: Bruce Ashfield <bruce.ashfield@gmail.com>
Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com>
---
 meta/classes/kernel.bbclass | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 41101a64a0..e9b5a84de7 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -94,6 +94,14 @@ python __anonymous () {
         d.appendVar('RDEPENDS_%s-image' % kname, ' %s-image-%s' % (kname, typelower))
         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_ontarget_%s-image-%s' % (kname,typelower), """
+set +e
+ln -sf %s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "Filesystem on ${KERNEL_IMAGEDEST}/ doesn't support symlink, then use a copy."
+fi
+set -e
+""" % (type, type))
 
     image = d.getVar('INITRAMFS_IMAGE')
     # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
@@ -386,7 +394,7 @@ kernel_do_install() {
 	for imageType in ${KERNEL_IMAGETYPES} ; do
 		install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType} ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
 		if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then
-			ln -sf ${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
+			install -m 0644 ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
 		fi
 	done
 	install -m 0644 System.map ${D}/boot/System.map-${KERNEL_VERSION}
@@ -602,6 +610,7 @@ pkg_postinst_${KERNEL_PACKAGE_NAME}-base () {
 	fi
 }
 
+
 PACKAGESPLITFUNCS_prepend = "split_kernel_packages "
 
 python split_kernel_packages () {
-- 
2.18.2


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

* Re: [OE-core] [PATCH 1/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
  2020-06-17  6:20 ` [OE-core] [PATCH 1/1] " Xu, Yanfei
@ 2020-06-17 13:48   ` Bruce Ashfield
  2020-06-18  6:49     ` Xu, Yanfei
  0 siblings, 1 reply; 6+ messages in thread
From: Bruce Ashfield @ 2020-06-17 13:48 UTC (permalink / raw)
  To: Xu, Yanfei; +Cc: Patches and discussions about the oe-core layer, He Zhe

On Wed, Jun 17, 2020 at 2:20 AM <yanfei.xu@windriver.com> wrote:
>
> From: Yanfei Xu <yanfei.xu@windriver.com>
>
> Some filesystems don't support symlink, then you will get failed when
> you install or update the kernel rpm package. Now we use a copy for
> these filesystems instead of symlink.
>
> Suggested-by: Bruce Ashfield <bruce.ashfield@gmail.com>
> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com>
> ---
>  meta/classes/kernel.bbclass | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 41101a64a0..e9b5a84de7 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -94,6 +94,14 @@ python __anonymous () {
>          d.appendVar('RDEPENDS_%s-image' % kname, ' %s-image-%s' % (kname, typelower))
>          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_ontarget_%s-image-%s' % (kname,typelower), """
> +set +e
> +ln -sf %s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
> +if [ $? -ne 0 ]; then
> +    echo "Filesystem on ${KERNEL_IMAGEDEST}/ doesn't support symlink, then use a copy."

This could say "... doesn't support symlinks, falling back to copied
image (%s)" (and fill in the image file we are actually using).

> +fi

I expected to see the fallback copy here in the postinstall, but this
looks ok to me .. I'm going to just type in my process of reading the
patch so you can correct me if I'm wrong.

The logic is split between the two places. Below during the build
(do_install), and here in the postinstall.

In do_install(), we no longer symlink, but instead copy the versioned
image into the non versioned 'image' file. And then in the
postinstall, we try and make a symlink (which may fail) to the same
thing (the versioned image to a non versioned one), and that's a force
symlink so it will clobber the previously copied one if possible, or
implicitly fall back if the symlink fails.

So yes, that logic seems fine to me.

The reason I went through all that, is that for on-target upgrades.
You won't have the do_install() to place the versioned image, have you
confirmed that the versioned image file is packaged and will be
present on an on-target install ? So the logic is the same with the
symlink ? It should be, otherwise this would never work, but I just
wanted to be sure.

> +set -e
> +""" % (type, type))
>
>      image = d.getVar('INITRAMFS_IMAGE')
>      # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
> @@ -386,7 +394,7 @@ kernel_do_install() {
>         for imageType in ${KERNEL_IMAGETYPES} ; do
>                 install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType} ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
>                 if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then
> -                       ln -sf ${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> +                       install -m 0644 ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}

Looking at this, can the existing line:

 install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType}
${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}

be combined with the one you added ?

We are copying the non-versioned image into a versioned image file,
and then in the new logic, coping that versioned image into a
non-versioned one. Couldn't we just have one copy of the non-versioned
image, into the non-versioned image in KERNEL_IMAGEDEST ?

Cheers,

Bruce

>                 fi
>         done
>         install -m 0644 System.map ${D}/boot/System.map-${KERNEL_VERSION}
> @@ -602,6 +610,7 @@ pkg_postinst_${KERNEL_PACKAGE_NAME}-base () {
>         fi
>  }
>
> +
>  PACKAGESPLITFUNCS_prepend = "split_kernel_packages "
>
>  python split_kernel_packages () {
> --
> 2.18.2
>


-- 
- 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] 6+ messages in thread

* Re: [OE-core] [PATCH 1/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
  2020-06-17 13:48   ` Bruce Ashfield
@ 2020-06-18  6:49     ` Xu, Yanfei
  2020-06-18 12:44       ` Bruce Ashfield
       [not found]       ` <1619A402D254EBD8.22873@lists.openembedded.org>
  0 siblings, 2 replies; 6+ messages in thread
From: Xu, Yanfei @ 2020-06-18  6:49 UTC (permalink / raw)
  To: Bruce Ashfield; +Cc: Patches and discussions about the oe-core layer, He Zhe



On 6/17/20 9:48 PM, Bruce Ashfield wrote:
> On Wed, Jun 17, 2020 at 2:20 AM <yanfei.xu@windriver.com> wrote:
>>
>> From: Yanfei Xu <yanfei.xu@windriver.com>
>>
>> Some filesystems don't support symlink, then you will get failed when
>> you install or update the kernel rpm package. Now we use a copy for
>> these filesystems instead of symlink.
>>
>> Suggested-by: Bruce Ashfield <bruce.ashfield@gmail.com>
>> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com>
>> ---
>>   meta/classes/kernel.bbclass | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index 41101a64a0..e9b5a84de7 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -94,6 +94,14 @@ python __anonymous () {
>>           d.appendVar('RDEPENDS_%s-image' % kname, ' %s-image-%s' % (kname, typelower))
>>           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_ontarget_%s-image-%s' % (kname,typelower), """
>> +set +e
>> +ln -sf %s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
>> +if [ $? -ne 0 ]; then
>> +    echo "Filesystem on ${KERNEL_IMAGEDEST}/ doesn't support symlink, then use a copy."
> 
> This could say "... doesn't support symlinks, falling back to copied
> image (%s)" (and fill in the image file we are actually using).
> 
This discription is more clear. I will rewrite the message.
>> +fi
> 
> I expected to see the fallback copy here in the postinstall, but this
> looks ok to me .. I'm going to just type in my process of reading the
At the beginning, I was going to only place versioned-image in rpm
package, then make symlink or copy in postinstall on target. But it will
cause rpm package don't contain no-versioned image. However, ARM device
may use the no-versioned image for booting in boot partation. So I
didn't implement it like that.
> patch so you can correct me if I'm wrong.
> 
> The logic is split between the two places. Below during the build
> (do_install), and here in the postinstall.
> 
> In do_install(), we no longer symlink, but instead copy the versioned
> image into the non versioned 'image' file. And then in the
> postinstall, we try and make a symlink (which may fail) to the same
> thing (the versioned image to a non versioned one), and that's a force
> symlink so it will clobber the previously copied one if possible, or
> implicitly fall back if the symlink fails.
Your understanding is right.
> 
> So yes, that logic seems fine to me.
> 
> The reason I went through all that, is that for on-target upgrades.
> You won't have the do_install() to place the versioned image, have you
> confirmed that the versioned image file is packaged and will be
I have confirmed that both non versioned image and versioned image are 
packaged in rpm package.
[bcm_2xxx_rpi4]$ rpm -qpl 
kernel-image-image-5.4.46-yocto-standard-5.4.x+git0+f8c88c4331_87b52c3030-r0.bcm_2xxx_rpi4.rpm 

/boot
/boot/Image
/boot/Image-5.4.46-yocto-standard

Besides, I have tested "dnf install" on target, it did place files of 
package and then run postinstall script.
> present on an on-target install ? So the logic is the same with the
> symlink ? It should be, otherwise this would never work, but I just
> wanted to be sure.
> Yes, the logic is the same with the original symlink, hence I also think
there is no new bug introduced to on-target upgrades.


>> +set -e
>> +""" % (type, type))
>>
>>       image = d.getVar('INITRAMFS_IMAGE')
>>       # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
>> @@ -386,7 +394,7 @@ kernel_do_install() {
>>          for imageType in ${KERNEL_IMAGETYPES} ; do
>>                  install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType} ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
>>                  if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then
>> -                       ln -sf ${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
>> +                       install -m 0644 ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> 
> Looking at this, can the existing line:
> 
>   install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType}
> ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
> 
> be combined with the one you added ?
> 
> We are copying the non-versioned image into a versioned image file,
> and then in the new logic, coping that versioned image into a
> non-versioned one. Couldn't we just have one copy of the non-versioned
> image, into the non-versioned image in KERNEL_IMAGEDEST ?
I don't think it could be. Cause the existing line :
"if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then"
can't be ingnored.

Thanks for reviewing,

Yanfei

> 
> Cheers,
> 
> Bruce
> 
>>                  fi
>>          done
>>          install -m 0644 System.map ${D}/boot/System.map-${KERNEL_VERSION}
>> @@ -602,6 +610,7 @@ pkg_postinst_${KERNEL_PACKAGE_NAME}-base () {
>>          fi
>>   }
>>
>> +
>>   PACKAGESPLITFUNCS_prepend = "split_kernel_packages "
>>
>>   python split_kernel_packages () {
>> --
>> 2.18.2
>>
> 
> 

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

* Re: [OE-core] [PATCH 1/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
  2020-06-18  6:49     ` Xu, Yanfei
@ 2020-06-18 12:44       ` Bruce Ashfield
       [not found]       ` <1619A402D254EBD8.22873@lists.openembedded.org>
  1 sibling, 0 replies; 6+ messages in thread
From: Bruce Ashfield @ 2020-06-18 12:44 UTC (permalink / raw)
  To: Xu, Yanfei; +Cc: Patches and discussions about the oe-core layer, He Zhe

On Thu, Jun 18, 2020 at 2:49 AM Xu, Yanfei <yanfei.xu@windriver.com> wrote:
>
>
>
> On 6/17/20 9:48 PM, Bruce Ashfield wrote:
> > On Wed, Jun 17, 2020 at 2:20 AM <yanfei.xu@windriver.com> wrote:
> >>
> >> From: Yanfei Xu <yanfei.xu@windriver.com>
> >>
> >> Some filesystems don't support symlink, then you will get failed when
> >> you install or update the kernel rpm package. Now we use a copy for
> >> these filesystems instead of symlink.
> >>
> >> Suggested-by: Bruce Ashfield <bruce.ashfield@gmail.com>
> >> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com>
> >> ---
> >>   meta/classes/kernel.bbclass | 11 ++++++++++-
> >>   1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> >> index 41101a64a0..e9b5a84de7 100644
> >> --- a/meta/classes/kernel.bbclass
> >> +++ b/meta/classes/kernel.bbclass
> >> @@ -94,6 +94,14 @@ python __anonymous () {
> >>           d.appendVar('RDEPENDS_%s-image' % kname, ' %s-image-%s' % (kname, typelower))
> >>           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_ontarget_%s-image-%s' % (kname,typelower), """
> >> +set +e
> >> +ln -sf %s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
> >> +if [ $? -ne 0 ]; then
> >> +    echo "Filesystem on ${KERNEL_IMAGEDEST}/ doesn't support symlink, then use a copy."
> >
> > This could say "... doesn't support symlinks, falling back to copied
> > image (%s)" (and fill in the image file we are actually using).
> >
> This discription is more clear. I will rewrite the message.
> >> +fi
> >
> > I expected to see the fallback copy here in the postinstall, but this
> > looks ok to me .. I'm going to just type in my process of reading the
> At the beginning, I was going to only place versioned-image in rpm
> package, then make symlink or copy in postinstall on target. But it will
> cause rpm package don't contain no-versioned image. However, ARM device
> may use the no-versioned image for booting in boot partation. So I
> didn't implement it like that.
> > patch so you can correct me if I'm wrong.
> >
> > The logic is split between the two places. Below during the build
> > (do_install), and here in the postinstall.
> >
> > In do_install(), we no longer symlink, but instead copy the versioned
> > image into the non versioned 'image' file. And then in the
> > postinstall, we try and make a symlink (which may fail) to the same
> > thing (the versioned image to a non versioned one), and that's a force
> > symlink so it will clobber the previously copied one if possible, or
> > implicitly fall back if the symlink fails.
> Your understanding is right.
> >
> > So yes, that logic seems fine to me.
> >
> > The reason I went through all that, is that for on-target upgrades.
> > You won't have the do_install() to place the versioned image, have you
> > confirmed that the versioned image file is packaged and will be
> I have confirmed that both non versioned image and versioned image are
> packaged in rpm package.
> [bcm_2xxx_rpi4]$ rpm -qpl
> kernel-image-image-5.4.46-yocto-standard-5.4.x+git0+f8c88c4331_87b52c3030-r0.bcm_2xxx_rpi4.rpm
>
> /boot
> /boot/Image
> /boot/Image-5.4.46-yocto-standard
>
> Besides, I have tested "dnf install" on target, it did place files of
> package and then run postinstall script.
> > present on an on-target install ? So the logic is the same with the
> > symlink ? It should be, otherwise this would never work, but I just
> > wanted to be sure.
> > Yes, the logic is the same with the original symlink, hence I also think
> there is no new bug introduced to on-target upgrades.
>
>
> >> +set -e
> >> +""" % (type, type))
> >>
> >>       image = d.getVar('INITRAMFS_IMAGE')
> >>       # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
> >> @@ -386,7 +394,7 @@ kernel_do_install() {
> >>          for imageType in ${KERNEL_IMAGETYPES} ; do
> >>                  install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType} ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
> >>                  if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then
> >> -                       ln -sf ${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> >> +                       install -m 0644 ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> >
> > Looking at this, can the existing line:
> >
> >   install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType}
> > ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
> >
> > be combined with the one you added ?
> >
> > We are copying the non-versioned image into a versioned image file,
> > and then in the new logic, coping that versioned image into a
> > non-versioned one. Couldn't we just have one copy of the non-versioned
> > image, into the non-versioned image in KERNEL_IMAGEDEST ?
> I don't think it could be. Cause the existing line :
> "if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then"
> can't be ingnored.
>

Ah yes, I suppose so. I never change KERNEL_PACKAGE_NAME, so I haven't
run into that use case.

The change looks fine to me.

Reviewed-by: Bruce Ashfield <bruce.ashfield@gmail.com>

> Thanks for reviewing,
>
> Yanfei
>
> >
> > Cheers,
> >
> > Bruce
> >
> >>                  fi
> >>          done
> >>          install -m 0644 System.map ${D}/boot/System.map-${KERNEL_VERSION}
> >> @@ -602,6 +610,7 @@ pkg_postinst_${KERNEL_PACKAGE_NAME}-base () {
> >>          fi
> >>   }
> >>
> >> +
> >>   PACKAGESPLITFUNCS_prepend = "split_kernel_packages "
> >>
> >>   python split_kernel_packages () {
> >> --
> >> 2.18.2
> >>
> >
> >



-- 
- 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] 6+ messages in thread

* Re: [OE-core] [PATCH 1/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink
       [not found]       ` <1619A402D254EBD8.22873@lists.openembedded.org>
@ 2020-06-18 12:46         ` Bruce Ashfield
  0 siblings, 0 replies; 6+ messages in thread
From: Bruce Ashfield @ 2020-06-18 12:46 UTC (permalink / raw)
  To: Bruce Ashfield
  Cc: Xu, Yanfei, Patches and discussions about the oe-core layer, He Zhe

On Thu, Jun 18, 2020 at 8:44 AM Bruce Ashfield via
lists.openembedded.org
<bruce.ashfield=gmail.com@lists.openembedded.org> wrote:
>
> On Thu, Jun 18, 2020 at 2:49 AM Xu, Yanfei <yanfei.xu@windriver.com> wrote:
> >
> >
> >
> > On 6/17/20 9:48 PM, Bruce Ashfield wrote:
> > > On Wed, Jun 17, 2020 at 2:20 AM <yanfei.xu@windriver.com> wrote:
> > >>
> > >> From: Yanfei Xu <yanfei.xu@windriver.com>
> > >>
> > >> Some filesystems don't support symlink, then you will get failed when
> > >> you install or update the kernel rpm package. Now we use a copy for
> > >> these filesystems instead of symlink.
> > >>
> > >> Suggested-by: Bruce Ashfield <bruce.ashfield@gmail.com>
> > >> Signed-off-by: Yanfei Xu <yanfei.xu@windriver.com>
> > >> ---
> > >>   meta/classes/kernel.bbclass | 11 ++++++++++-
> > >>   1 file changed, 10 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> > >> index 41101a64a0..e9b5a84de7 100644
> > >> --- a/meta/classes/kernel.bbclass
> > >> +++ b/meta/classes/kernel.bbclass
> > >> @@ -94,6 +94,14 @@ python __anonymous () {
> > >>           d.appendVar('RDEPENDS_%s-image' % kname, ' %s-image-%s' % (kname, typelower))
> > >>           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_ontarget_%s-image-%s' % (kname,typelower), """
> > >> +set +e
> > >> +ln -sf %s-${KERNEL_VERSION} ${KERNEL_IMAGEDEST}/%s > /dev/null 2>&1
> > >> +if [ $? -ne 0 ]; then
> > >> +    echo "Filesystem on ${KERNEL_IMAGEDEST}/ doesn't support symlink, then use a copy."
> > >
> > > This could say "... doesn't support symlinks, falling back to copied
> > > image (%s)" (and fill in the image file we are actually using).
> > >
> > This discription is more clear. I will rewrite the message.
> > >> +fi
> > >
> > > I expected to see the fallback copy here in the postinstall, but this
> > > looks ok to me .. I'm going to just type in my process of reading the
> > At the beginning, I was going to only place versioned-image in rpm
> > package, then make symlink or copy in postinstall on target. But it will
> > cause rpm package don't contain no-versioned image. However, ARM device
> > may use the no-versioned image for booting in boot partation. So I
> > didn't implement it like that.
> > > patch so you can correct me if I'm wrong.
> > >
> > > The logic is split between the two places. Below during the build
> > > (do_install), and here in the postinstall.
> > >
> > > In do_install(), we no longer symlink, but instead copy the versioned
> > > image into the non versioned 'image' file. And then in the
> > > postinstall, we try and make a symlink (which may fail) to the same
> > > thing (the versioned image to a non versioned one), and that's a force
> > > symlink so it will clobber the previously copied one if possible, or
> > > implicitly fall back if the symlink fails.
> > Your understanding is right.
> > >
> > > So yes, that logic seems fine to me.
> > >
> > > The reason I went through all that, is that for on-target upgrades.
> > > You won't have the do_install() to place the versioned image, have you
> > > confirmed that the versioned image file is packaged and will be
> > I have confirmed that both non versioned image and versioned image are
> > packaged in rpm package.
> > [bcm_2xxx_rpi4]$ rpm -qpl
> > kernel-image-image-5.4.46-yocto-standard-5.4.x+git0+f8c88c4331_87b52c3030-r0.bcm_2xxx_rpi4.rpm
> >
> > /boot
> > /boot/Image
> > /boot/Image-5.4.46-yocto-standard
> >
> > Besides, I have tested "dnf install" on target, it did place files of
> > package and then run postinstall script.
> > > present on an on-target install ? So the logic is the same with the
> > > symlink ? It should be, otherwise this would never work, but I just
> > > wanted to be sure.
> > > Yes, the logic is the same with the original symlink, hence I also think
> > there is no new bug introduced to on-target upgrades.
> >
> >
> > >> +set -e
> > >> +""" % (type, type))
> > >>
> > >>       image = d.getVar('INITRAMFS_IMAGE')
> > >>       # If the INTIRAMFS_IMAGE is set but the INITRAMFS_IMAGE_BUNDLE is set to 0,
> > >> @@ -386,7 +394,7 @@ kernel_do_install() {
> > >>          for imageType in ${KERNEL_IMAGETYPES} ; do
> > >>                  install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType} ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
> > >>                  if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then
> > >> -                       ln -sf ${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> > >> +                       install -m 0644 ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION} ${D}/${KERNEL_IMAGEDEST}/${imageType}
> > >
> > > Looking at this, can the existing line:
> > >
> > >   install -m 0644 ${KERNEL_OUTPUT_DIR}/${imageType}
> > > ${D}/${KERNEL_IMAGEDEST}/${imageType}-${KERNEL_VERSION}
> > >
> > > be combined with the one you added ?
> > >
> > > We are copying the non-versioned image into a versioned image file,
> > > and then in the new logic, coping that versioned image into a
> > > non-versioned one. Couldn't we just have one copy of the non-versioned
> > > image, into the non-versioned image in KERNEL_IMAGEDEST ?
> > I don't think it could be. Cause the existing line :
> > "if [ "${KERNEL_PACKAGE_NAME}" = "kernel" ]; then"
> > can't be ingnored.
> >
>
> Ah yes, I suppose so. I never change KERNEL_PACKAGE_NAME, so I haven't
> run into that use case.
>
> The change looks fine to me.
>
> Reviewed-by: Bruce Ashfield <bruce.ashfield@gmail.com>

Cancel this. I see the v3 will be incoming shortly, I'll re-review then.

Bruce

>
> > Thanks for reviewing,
> >
> > Yanfei
> >
> > >
> > > Cheers,
> > >
> > > Bruce
> > >
> > >>                  fi
> > >>          done
> > >>          install -m 0644 System.map ${D}/boot/System.map-${KERNEL_VERSION}
> > >> @@ -602,6 +610,7 @@ pkg_postinst_${KERNEL_PACKAGE_NAME}-base () {
> > >>          fi
> > >>   }
> > >>
> > >> +
> > >>   PACKAGESPLITFUNCS_prepend = "split_kernel_packages "
> > >>
> > >>   python split_kernel_packages () {
> > >> --
> > >> 2.18.2
> > >>
> > >
> > >
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
> 



-- 
- 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] 6+ messages in thread

end of thread, other threads:[~2020-06-18 12:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17  6:20 [OE-core] [PATCH 0/1] classes/kernel: Use a copy for kernel*.rpm when fs don't support symlink Xu, Yanfei
2020-06-17  6:20 ` [OE-core] [PATCH 1/1] " Xu, Yanfei
2020-06-17 13:48   ` Bruce Ashfield
2020-06-18  6:49     ` Xu, Yanfei
2020-06-18 12:44       ` Bruce Ashfield
     [not found]       ` <1619A402D254EBD8.22873@lists.openembedded.org>
2020-06-18 12:46         ` Bruce Ashfield

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.