[PATCH] Fix "tar: .: file changed as we read it" with cpio+tar builds enabled

[PATCH] Fix "tar: .: file changed as we read it" with cpio+tar builds enabled

[Jonas Eriksson]

Hi,

This patch solves a problem we have been seeing when building both a .tar(.gz)
and .cpio(.gz) image, where the cpio job creates a file in the rootfs
directory. This event may happen while tar is running, which makes tar exit
indicating an error and thus aborting the build.

The only potential problem with the patch that I can think of is whether or not
cpio in the build host support -A and -F. I've checked the GNU cpio git repo,
and the arguments have been there since they started using git in 2004, which
means that this should not be a problem.

Regards,
/Jonas

Jonas Eriksson (1):
  cpio rootfs build: Avoid modifying rootfs dir

 meta/classes/image_types.bbclass | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
1.9.0

[PATCH] cpio rootfs build: Avoid modifying rootfs dir

[Jonas Eriksson]

The Linux kernel requires that initrd images contain a /init file for
the image to be used as an initrd, even if it is empty. Adding it into
the rootfs directory creates a race, that can upset tar when building
both a .tar and .cpio image file ("tar: .: file changed as we read it").
Additionally, whether or not the tar file will contain the /init file is
also up to the race condition.

To avoid this problem, move the /init addition out from the rootfs
directory, and thus only include it in the .cpio image.

Signed-off-by: Jonas Eriksson <jonas.eriksson@enea.com>
Signed-off-by: Josep Puigdemont <josep.puigdemont@enea.com>
Cc: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 meta/classes/image_types.bbclass | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 602c1f0..8001025 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -67,10 +67,12 @@ IMAGE_CMD_squashfs-xz = "mksquashfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_
 IMAGE_CMD_tar = "tar -cvf ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.tar -C ${IMAGE_ROOTFS} ."
 
 IMAGE_CMD_cpio () {
-	if [ ! -L ${IMAGE_ROOTFS}/init ]; then
-		touch ${IMAGE_ROOTFS}/init
-	fi
 	(cd ${IMAGE_ROOTFS} && find . | cpio -o -H newc >${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cpio)
+	if [ ! -e ${IMAGE_ROOTFS}/init ]; then
+		mkdir -p ${WORKDIR}/cpio_append
+		touch ${WORKDIR}/cpio_append/init
+		(cd  ${WORKDIR}/cpio_append && echo ./init | cpio -oA -H newc -F ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cpio)
+	fi
 }
 
 ELF_KERNEL ?= "${STAGING_DIR_HOST}/usr/src/kernel/${KERNEL_IMAGETYPE}"
-- 
1.9.0

Re: [PATCH] cpio rootfs build: Avoid modifying rootfs dir

[Andrea Adami]

On Mon, Mar 24, 2014 at 4:56 PM, Jonas Eriksson <jonas.eriksson@enea.com> wrote:
> The Linux kernel requires that initrd images contain a /init file for
> the image to be used as an initrd, even if it is empty. Adding it into
> the rootfs directory creates a race, that can upset tar when building
> both a .tar and .cpio image file ("tar: .: file changed as we read it").
> Additionally, whether or not the tar file will contain the /init file is
> also up to the race condition.
>
> To avoid this problem, move the /init addition out from the rootfs
> directory, and thus only include it in the .cpio image.
>
> Signed-off-by: Jonas Eriksson <jonas.eriksson@enea.com>
> Signed-off-by: Josep Puigdemont <josep.puigdemont@enea.com>
> Cc: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>  meta/classes/image_types.bbclass | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> index 602c1f0..8001025 100644
> --- a/meta/classes/image_types.bbclass
> +++ b/meta/classes/image_types.bbclass
> @@ -67,10 +67,12 @@ IMAGE_CMD_squashfs-xz = "mksquashfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_
>  IMAGE_CMD_tar = "tar -cvf ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.tar -C ${IMAGE_ROOTFS} ."
>
>  IMAGE_CMD_cpio () {
> -       if [ ! -L ${IMAGE_ROOTFS}/init ]; then
> -               touch ${IMAGE_ROOTFS}/init
> -       fi
>         (cd ${IMAGE_ROOTFS} && find . | cpio -o -H newc >${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cpio)
> +       if [ ! -e ${IMAGE_ROOTFS}/init ]; then
> +               mkdir -p ${WORKDIR}/cpio_append
> +               touch ${WORKDIR}/cpio_append/init
> +               (cd  ${WORKDIR}/cpio_append && echo ./init | cpio -oA -H newc -F ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cpio)
> +       fi
>  }
>
>  ELF_KERNEL ?= "${STAGING_DIR_HOST}/usr/src/kernel/${KERNEL_IMAGETYPE}"
> --
> 1.9.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

I admit I did not encounter this race, still I tested the patch
because it caught my attention.

There are some rare cases where init is a symlink pointing to a custom
executable so the previous test was specifically done in order to
avoid to touch a symlink.
I was initially afraid of the change but actually the test is done
after the creation of the cpio so this patch doesn't seem to break our
special case.

Acked-by: Andrea Adami <andrea.adami@gmail.com>

Re: [PATCH] cpio rootfs build: Avoid modifying rootfs dir

[Jonas Eriksson]

Hi Andrea,

On Wed, Mar 26, 2014 at 00:04:55 +0100 Andrea Adami wrote:
> I admit I did not encounter this race, still I tested the patch
> because it caught my attention.
> 
> There are some rare cases where init is a symlink pointing to a custom
> executable so the previous test was specifically done in order to
> avoid to touch a symlink.

Yes, that makes sense. It's however a bit problematic to alter
the rootfs directory after the do_rootfs has finished executing.
This is why the ./init append was moved to work with a separate
directory.

To clarify around the symlink situation: The patch moves from a
-L to a -e to make sure that we don't append the ./init to the
cpio if there is any kind of ./init in the rootfs directory, be
it a file or a symlink.

> I was initially afraid of the change but actually the test is done
> after the creation of the cpio so this patch doesn't seem to break our
> special case.
> 
> Acked-by: Andrea Adami <andrea.adami@gmail.com>

Thanks for the feedback and Ack!

/Jonas