Patchwork kernel.bbclass: handles symbolic KERNEL_OUTPUT in bundle_initramfs task

login
register
mail settings
Submitter Ming Liu
Date Nov. 12, 2013, 8:17 a.m.
Message ID <1384244230-30541-1-git-send-email-ming.liu@windriver.com>
Download mbox | patch
Permalink /patch/61471/
State New
Headers show

Comments

Ming Liu - Nov. 12, 2013, 8:17 a.m.
In many cases, KERNEL_OUTPUT is a symbolic link to the real bootable image,
but in bundle_initramfs task, it's not considered so that the KERNEL_OUTPUT
is being renamed/restored as a regular file, this leads it finally point to
a incorrect target, or even worse, break the bundle_initramfs task for
KERNEL_OUTPUT is not going to be regenerated in case it is a symbolic link
to vmlinux.

Also, we need move bundle_initramfs after kernel_link_vmlinux.

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
 meta/classes/kernel.bbclass | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)
Bruce Ashfield - Nov. 12, 2013, 3:07 p.m.
On Tue, Nov 12, 2013 at 3:17 AM, Ming Liu <ming.liu@windriver.com> wrote:
> In many cases, KERNEL_OUTPUT is a symbolic link to the real bootable image,
> but in bundle_initramfs task, it's not considered so that the KERNEL_OUTPUT
> is being renamed/restored as a regular file, this leads it finally point to
> a incorrect target, or even worse, break the bundle_initramfs task for
> KERNEL_OUTPUT is not going to be regenerated in case it is a symbolic link
> to vmlinux.
>
> Also, we need move bundle_initramfs after kernel_link_vmlinux.
>
> Signed-off-by: Ming Liu <ming.liu@windriver.com>
> ---
>  meta/classes/kernel.bbclass | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index a4db9f5..2ec09ae 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -123,13 +123,29 @@ do_bundle_initramfs () {
>         if [ ! -z "${INITRAMFS_IMAGE}" -a x"${INITRAMFS_IMAGE_BUNDLE}" = x1 ]; then
>                 echo "Creating a kernel image with a bundled initramfs..."
>                 copy_initramfs
> -               if [ -e ${KERNEL_OUTPUT} ] ; then
> +               # Backuping kernel image relies on its type(regular file or symbolic link)
> +               linkpath=""
> +               realpath=""
> +               if [ -h ${KERNEL_OUTPUT} ] ; then
> +                       linkpath=`readlink -n ${KERNEL_OUTPUT}`
> +                       realpath=`readlink -fn ${KERNEL_OUTPUT}`
> +                       mv -f $realpath $realpath.bak
> +               elif [ -f ${KERNEL_OUTPUT} ]; then
>                         mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.bak
>                 fi
>                 use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio
>                 kernel_do_compile
> -               mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
> -               mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
> +               # Restoring kernel image
> +               if [ ! -z $realpath ]; then

Why not  if [ -n "$realpath" ] ? I always find that reads easier.

> +                       mv -f $realpath $realpath.initramfs
> +                       mv -f $realpath.bak $realpath
> +                       cd ${B}/$(dirname ${KERNEL_OUTPUT})
> +                       ln -sf $linkpath
> +                       ln -sf $linkpath.initramfs

Won't the links just follow .. they are symbolic after all. Since things are
being put back in place. Or are the names changing underneath ?

> +               else
> +                       mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
> +                       mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
> +               fi
>                 # Update install area
>                 echo "There is kernel image bundled with initramfs: ${B}/${KERNEL_OUTPUT}.initramfs"
>                 install -m 0644 ${B}/${KERNEL_OUTPUT}.initramfs ${D}/boot/${KERNEL_IMAGETYPE}-initramfs-${MACHINE}.bin
> @@ -152,7 +168,7 @@ python do_devshell_prepend () {
>      os.environ["LDFLAGS"] = ''
>  }
>
> -addtask bundle_initramfs after do_compile before do_build
> +addtask bundle_initramfs after do_kernel_link_vmlinux before do_build

This isn't explained in the commit log.

Bruce

>
>  kernel_do_compile() {
>         unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
> --
> 1.8.4.1
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Ming Liu - Nov. 13, 2013, 2:03 a.m.
On 11/12/2013 11:07 PM, Bruce Ashfield wrote:
> On Tue, Nov 12, 2013 at 3:17 AM, Ming Liu <ming.liu@windriver.com> wrote:
>> In many cases, KERNEL_OUTPUT is a symbolic link to the real bootable image,
>> but in bundle_initramfs task, it's not considered so that the KERNEL_OUTPUT
>> is being renamed/restored as a regular file, this leads it finally point to
>> a incorrect target, or even worse, break the bundle_initramfs task for
>> KERNEL_OUTPUT is not going to be regenerated in case it is a symbolic link
>> to vmlinux.
>>
>> Also, we need move bundle_initramfs after kernel_link_vmlinux.
>>
>> Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> ---
>>   meta/classes/kernel.bbclass | 24 ++++++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>> index a4db9f5..2ec09ae 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -123,13 +123,29 @@ do_bundle_initramfs () {
>>          if [ ! -z "${INITRAMFS_IMAGE}" -a x"${INITRAMFS_IMAGE_BUNDLE}" = x1 ]; then
>>                  echo "Creating a kernel image with a bundled initramfs..."
>>                  copy_initramfs
>> -               if [ -e ${KERNEL_OUTPUT} ] ; then
>> +               # Backuping kernel image relies on its type(regular file or symbolic link)
>> +               linkpath=""
>> +               realpath=""
>> +               if [ -h ${KERNEL_OUTPUT} ] ; then
>> +                       linkpath=`readlink -n ${KERNEL_OUTPUT}`
>> +                       realpath=`readlink -fn ${KERNEL_OUTPUT}`
>> +                       mv -f $realpath $realpath.bak
>> +               elif [ -f ${KERNEL_OUTPUT} ]; then
>>                          mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.bak
>>                  fi
>>                  use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio
>>                  kernel_do_compile
>> -               mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
>> -               mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
>> +               # Restoring kernel image
>> +               if [ ! -z $realpath ]; then
> Why not  if [ -n "$realpath" ] ? I always find that reads easier.
OK, that would be better.
>
>> +                       mv -f $realpath $realpath.initramfs
>> +                       mv -f $realpath.bak $realpath
>> +                       cd ${B}/$(dirname ${KERNEL_OUTPUT})
>> +                       ln -sf $linkpath
>> +                       ln -sf $linkpath.initramfs
> Won't the links just follow .. they are symbolic after all. Since things are
> being put back in place. Or are the names changing underneath ?
Indeed, 'ln -sf $linkpath' is unnecessary, I can remove it, but for the 
initramfs link, we must create it here for it doesn't exist before this.
>
>> +               else
>> +                       mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
>> +                       mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
>> +               fi
>>                  # Update install area
>>                  echo "There is kernel image bundled with initramfs: ${B}/${KERNEL_OUTPUT}.initramfs"
>>                  install -m 0644 ${B}/${KERNEL_OUTPUT}.initramfs ${D}/boot/${KERNEL_IMAGETYPE}-initramfs-${MACHINE}.bin
>> @@ -152,7 +168,7 @@ python do_devshell_prepend () {
>>       os.environ["LDFLAGS"] = ''
>>   }
>>
>> -addtask bundle_initramfs after do_compile before do_build
>> +addtask bundle_initramfs after do_kernel_link_vmlinux before do_build
> This isn't explained in the commit log.
So, I will put it into another commit, I will send the V1 soon including 
all the changes discussed above.

//Ming Liu

>
> Bruce
>
>>   kernel_do_compile() {
>>          unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
>> --
>> 1.8.4.1
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>
Otavio Salvador - Nov. 13, 2013, 2:18 a.m.
On Wed, Nov 13, 2013 at 12:03 AM, Ming Liu <ming.liu@windriver.com> wrote:
...
> So, I will put it into another commit, I will send the V1 soon including all
> the changes discussed above.

v2 ;-)
Ming Liu - Nov. 13, 2013, 2:21 a.m.
On 11/13/2013 10:18 AM, Otavio Salvador wrote:
> On Wed, Nov 13, 2013 at 12:03 AM, Ming Liu <ming.liu@windriver.com> wrote:
> ...
>> So, I will put it into another commit, I will send the V1 soon including all
>> the changes discussed above.
> v2 ;-)
Got it. Thanks! :)

//Ming Liu

>

Patch

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index a4db9f5..2ec09ae 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -123,13 +123,29 @@  do_bundle_initramfs () {
 	if [ ! -z "${INITRAMFS_IMAGE}" -a x"${INITRAMFS_IMAGE_BUNDLE}" = x1 ]; then
 		echo "Creating a kernel image with a bundled initramfs..."
 		copy_initramfs
-		if [ -e ${KERNEL_OUTPUT} ] ; then
+		# Backuping kernel image relies on its type(regular file or symbolic link)
+		linkpath=""
+		realpath=""
+		if [ -h ${KERNEL_OUTPUT} ] ; then
+			linkpath=`readlink -n ${KERNEL_OUTPUT}`
+			realpath=`readlink -fn ${KERNEL_OUTPUT}`
+			mv -f $realpath $realpath.bak
+		elif [ -f ${KERNEL_OUTPUT} ]; then
 			mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.bak
 		fi
 		use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE}-${MACHINE}.cpio
 		kernel_do_compile
-		mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
-		mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
+		# Restoring kernel image
+		if [ ! -z $realpath ]; then
+			mv -f $realpath $realpath.initramfs
+			mv -f $realpath.bak $realpath
+			cd ${B}/$(dirname ${KERNEL_OUTPUT})
+			ln -sf $linkpath
+			ln -sf $linkpath.initramfs
+		else
+			mv -f ${KERNEL_OUTPUT} ${KERNEL_OUTPUT}.initramfs
+			mv -f ${KERNEL_OUTPUT}.bak ${KERNEL_OUTPUT}
+		fi
 		# Update install area
 		echo "There is kernel image bundled with initramfs: ${B}/${KERNEL_OUTPUT}.initramfs"
 		install -m 0644 ${B}/${KERNEL_OUTPUT}.initramfs ${D}/boot/${KERNEL_IMAGETYPE}-initramfs-${MACHINE}.bin
@@ -152,7 +168,7 @@  python do_devshell_prepend () {
     os.environ["LDFLAGS"] = ''
 }
 
-addtask bundle_initramfs after do_compile before do_build
+addtask bundle_initramfs after do_kernel_link_vmlinux before do_build
 
 kernel_do_compile() {
 	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE