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

[Claudius Heine <claudius.heine.ext@siemens.com>]

Hi Henning,

On 28/01/2019 14.03, Henning Schild wrote:
> 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.

Do you think that refactoring should be part of this patchset?

> 
>> +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 ;).

That is part of my plot to slowly merge bitbake, isar and buildroot ;)

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

Hmm.. In OE this task would not to touch the deploy directory and *copy* 
all artifacts in a dedicated task AFAIK. But OE has a completely 
different image creation pipeline and infrastructure than isar, so who 
knows how things should be done here.

I am open to discussion about implementing some rules and documentation 
about how things should be done in isar.

Claudius

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de