Re: [PATCH 2/2] integrate ubifs image type

[Henning Schild <henning.schild@siemens.com>]

Am Mon, 28 Jan 2019 13:28:21 +0100
schrieb "[ext] claudius.heine.ext@siemens.com"
<claudius.heine.ext@siemens.com>:

> From: Claudius Heine <ch@denx.de>
> 
> Signed-off-by: Claudius Heine <ch@denx.de>
> ---
>  doc/user_manual.md                            |  1 +
>  meta-isar/conf/local.conf.sample              |  1 +
>  .../multiconfig/qemuamd64-buster-ubifs.conf   | 16 +++++++
>  meta/classes/ubifs-img.bbclass                | 42
> +++++++++++++++++++ scripts/ci_build.sh                           |
> 1 + 5 files changed, 61 insertions(+)
>  create mode 100644
> meta-isar/conf/multiconfig/qemuamd64-buster-ubifs.conf create mode
> 100644 meta/classes/ubifs-img.bbclass
> 
> diff --git a/doc/user_manual.md b/doc/user_manual.md
> index c4fe42a..c9924ad 100644
> --- a/doc/user_manual.md
> +++ b/doc/user_manual.md
> @@ -476,6 +476,7 @@ Isar contains additional image type classes that
> can be used as reference:
>   - `ext4-img`
>   - `rpi-sdimg`
>   - `targz-img`
> + - `ubifs-img`
>   - `wic-img`
>  
>  ---
> diff --git a/meta-isar/conf/local.conf.sample
> b/meta-isar/conf/local.conf.sample index a671b20..9ea366c 100644
> --- a/meta-isar/conf/local.conf.sample
> +++ b/meta-isar/conf/local.conf.sample
> @@ -53,6 +53,7 @@ BBMULTICONFIG = " \
>      hikey-stretch \
>      qemuamd64-buster \
>      qemuamd64-buster-tgz \
> +    qemuamd64-buster-ubifs \
>      rpi-jessie \
>  "
>  
> diff --git a/meta-isar/conf/multiconfig/qemuamd64-buster-ubifs.conf
> b/meta-isar/conf/multiconfig/qemuamd64-buster-ubifs.conf new file
> mode 100644 index 0000000..7c638b9
> --- /dev/null
> +++ b/meta-isar/conf/multiconfig/qemuamd64-buster-ubifs.conf
> @@ -0,0 +1,16 @@
> +# This software is a part of ISAR.
> +# Copyright (c) Siemens AG, 2018
> +#
> +# SPDX-License-Identifier: MIT
> +
> +MACHINE ?= "qemuamd64"
> +
> +DISTRO ?= "debian-buster"
> +DISTRO_ARCH ?= "amd64"
> +
> +KERNEL_NAME ?= "amd64"
> +
> +MKUBIFS_ARGS ?= "-m 0x1000 -e 0x3e000 -c 1500"
> +IMAGE_TYPE ?= "ubifs-img"
> +
> +IMAGE_INSTALL += "sshd-regen-keys"
> diff --git a/meta/classes/ubifs-img.bbclass
> b/meta/classes/ubifs-img.bbclass new file mode 100644
> index 0000000..f5e17d3
> --- /dev/null
> +++ b/meta/classes/ubifs-img.bbclass
> @@ -0,0 +1,42 @@
> +# This software is a part of ISAR.
> +# Copyright (c) Siemens AG, 2018
> +
> +python() {
> +    if not d.getVar("MKUBIFS_ARGS"):
> +        bb.fatal("MKUBIFS_ARGS must be set")
> +}
> +
> +inherit image

I dislike all this variable setting and additional mounting.

> +UBIFS_IMAGE_FILE ?= "${IMAGE_FULLNAME}.ubifs.img"
> +
> +IMAGER_INSTALL += "mtd-utils"
> +
> +PP = "/home/builder/${PN}"

That is already defined in another file in Isar. We should rather find
a common location and refactor.

> +PP_DEPLOY = "${PP}/deploy"
> +PP_ROOTFS = "${PP}/rootfs"

Similar comments might apply here.

> +BUILDROOT = "${BUILDCHROOT_DIR}${PP}"
> +BUILDROOT_DEPLOY = "${BUILDCHROOT_DIR}${PP_DEPLOY}"
> +BUILDROOT_ROOTFS = "${BUILDCHROOT_DIR}${PP_ROOTFS}"

Same here, and "buildroot" to me is something else ;).

> +do_ubifs_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
> +
> +# Generate ubifs filesystem image
> +do_ubifs_image() {
> +    rm -f ${DEPLOY_DIR_IMAGE}/${UBIFS_IMAGE_FILE}
> +
> +    buildchroot_do_mounts
> +
> +    sudo flock ${MOUNT_LOCKFILE} -c ' \
> +        mkdir -p ${BUILDROOT_DEPLOY} ${BUILDROOT_ROOTFS}
> +        mount --bind ${DEPLOY_DIR_IMAGE} ${BUILDROOT_DEPLOY}
> +        mount --bind ${IMAGE_ROOTFS} ${BUILDROOT_ROOTFS}
> +    '

I think this mounting should also be required by a proper ext4 class.
On the other hand the wic one moves the image into the final deploy
folder.
Not sure what is best but maybe a good idea to go for "always move" or
"always mount".

Henning

> +    # Create ubifs image using buildchroot tools
> +    sudo chroot ${BUILDCHROOT_DIR} /usr/sbin/mkfs.ubifs
> ${MKUBIFS_ARGS} \
> +                -r "${PP_ROOTFS}" "${PP_DEPLOY}/${UBIFS_IMAGE_FILE}"
> +}
> +
> +addtask ubifs_image before do_build after do_copy_boot_files
> do_install_imager_deps diff --git a/scripts/ci_build.sh
> b/scripts/ci_build.sh index f3523e8..dcde0b4 100755
> --- a/scripts/ci_build.sh
> +++ b/scripts/ci_build.sh
> @@ -115,6 +115,7 @@ else
>          multiconfig:qemuamd64-stretch:isar-image-base \
>          multiconfig:qemuamd64-buster:isar-image-base \
>          multiconfig:qemuamd64-buster-tgz:isar-image-base \
> +        multiconfig:qemuamd64-buster-ubifs:isar-image-base \
>          multiconfig:rpi-jessie:isar-image-base
>      # qemu-user-static of <= buster too old to build that
>      #multiconfig:qemuarm64-buster:isar-image-base