[PATCH 0/2] Integrate UBIFS support (+doc fixes)

[PATCH 0/2] Integrate UBIFS support (+doc fixes)

[claudius.heine.ext]

From: Claudius Heine <ch@denx.de>

Hi,

The second patch add support for UBIFS images to isar as well a testcase
for it for qemuamd64. I am aware that this will result in a image, that
is very unlikely to be usable, since qemuamd64 does not support mtd
block device emulation. But for just testing those image types, that
should be ok, IMO.

I will now think about ubi images and maybe how to integrate boot with
qemu-arm (sabrelite).

regards,
Claudius

Claudius Heine (2):
  doc/user: added targz-img to image type list
  integrate ubifs image type

 doc/user_manual.md                            |  4 +-
 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, 63 insertions(+), 1 deletion(-)
 create mode 100644 meta-isar/conf/multiconfig/qemuamd64-buster-ubifs.conf
 create mode 100644 meta/classes/ubifs-img.bbclass

-- 
2.20.1

[PATCH 1/2] doc/user: added targz-img to image type list

[claudius.heine.ext]

From: Claudius Heine <ch@denx.de>

Signed-off-by: Claudius Heine <ch@denx.de>
---
 doc/user_manual.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/user_manual.md b/doc/user_manual.md
index eebcaa9..c4fe42a 100644
--- a/doc/user_manual.md
+++ b/doc/user_manual.md
@@ -471,10 +471,11 @@ IMAGE_TYPE = "my-image"
 
 ### Reference Classes
 
-Isar contains three image type classes that can be used as reference:
+Isar contains additional image type classes that can be used as reference:
 
  - `ext4-img`
  - `rpi-sdimg`
+ - `targz-img`
  - `wic-img`
 
 ---
-- 
2.20.1

[PATCH 2/2] integrate ubifs image type

[claudius.heine.ext]

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
+
+UBIFS_IMAGE_FILE ?= "${IMAGE_FULLNAME}.ubifs.img"
+
+IMAGER_INSTALL += "mtd-utils"
+
+PP = "/home/builder/${PN}"
+PP_DEPLOY = "${PP}/deploy"
+PP_ROOTFS = "${PP}/rootfs"
+
+BUILDROOT = "${BUILDCHROOT_DIR}${PP}"
+BUILDROOT_DEPLOY = "${BUILDCHROOT_DIR}${PP_DEPLOY}"
+BUILDROOT_ROOTFS = "${BUILDCHROOT_DIR}${PP_ROOTFS}"
+
+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}
+    '
+
+    # 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
-- 
2.20.1

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

[Jan Kiszka]

On 28.01.19 13:28, [ext] claudius.heine.ext@siemens.com wrote:
> 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
> +
> +UBIFS_IMAGE_FILE ?= "${IMAGE_FULLNAME}.ubifs.img"
> +
> +IMAGER_INSTALL += "mtd-utils"
> +
> +PP = "/home/builder/${PN}"
> +PP_DEPLOY = "${PP}/deploy"
> +PP_ROOTFS = "${PP}/rootfs"
> +
> +BUILDROOT = "${BUILDCHROOT_DIR}${PP}"
> +BUILDROOT_DEPLOY = "${BUILDCHROOT_DIR}${PP_DEPLOY}"
> +BUILDROOT_ROOTFS = "${BUILDCHROOT_DIR}${PP_ROOTFS}"
> +
> +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}
> +    '
> +
> +    # 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

For me as UBIFS newbie: I assume that this image class is already usable, e.g. 
as input for an out-of-isar ubi image generation, right? So, for someone 
familiar with ubi, it would be obvious what to do with this output artifact, and 
it does not need any form of documentation hint?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

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

[Claudius Heine]

Hi Jan,

On 28/01/2019 13.36, Jan Kiszka wrote:
> On 28.01.19 13:28, [ext] claudius.heine.ext@siemens.com wrote:
>> 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
>> +
>> +UBIFS_IMAGE_FILE ?= "${IMAGE_FULLNAME}.ubifs.img"
>> +
>> +IMAGER_INSTALL += "mtd-utils"
>> +
>> +PP = "/home/builder/${PN}"
>> +PP_DEPLOY = "${PP}/deploy"
>> +PP_ROOTFS = "${PP}/rootfs"
>> +
>> +BUILDROOT = "${BUILDCHROOT_DIR}${PP}"
>> +BUILDROOT_DEPLOY = "${BUILDCHROOT_DIR}${PP_DEPLOY}"
>> +BUILDROOT_ROOTFS = "${BUILDCHROOT_DIR}${PP_ROOTFS}"
>> +
>> +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}
>> +    '
>> +
>> +    # 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
> 
> For me as UBIFS newbie: I assume that this image class is already 
> usable, e.g. as input for an out-of-isar ubi image generation, right? 
> So, for someone familiar with ubi, it would be obvious what to do with 
> this output artifact, and it does not need any form of documentation hint?

I would suppose so.

UBI is like a partition table or LVM and UBIFS image like an ext4 image.

Claudius

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

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

[Henning Schild]

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

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

[Claudius Heine]

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

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

[Henning Schild]

Am Mon, 28 Jan 2019 14:40:36 +0100
schrieb 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?

I am not sure, just wanted to write it down. On the one hand it would
be nice to solve all drive-by issues as we go. On the other hand that
will slow down development and impose on contributors.
So feel free to ignore my comments, the inconsistencies are not your
fault and asking you to fix them would be too much.

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

Good first step. We actually have buildroot isar recipes in
jailhouse-images, so this name can potentially get confusing for real.

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

We currently generate and deploy in one task. The fun thing is that we
need multiple commands and can probably end up in weird states if we
get interrupted or fail.

Changing that general pattern is a discussion of its own, since it
would change the API.

Staying in the current API i think the mount and generate directly into
deploy ... no more chmod/chown mv ... whatever. Would be the cleanest
way. We already have a reliable umount mechanism to clean up if things
go wrong.

If you agree: Please include that deploydir/rootfs mount into the common
mounts and update your imager. I will do the same for wic and maybe
ext4.

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

I assume that consistent references are equally efficient as
documentation, maybe more.

Henning

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

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

[Claudius Heine]

Hi Henning,

On 28/01/2019 18.22, Henning Schild wrote:
> Am Mon, 28 Jan 2019 14:40:36 +0100
> schrieb 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?
> 
> I am not sure, just wanted to write it down. On the one hand it would
> be nice to solve all drive-by issues as we go. On the other hand that
> will slow down development and impose on contributors.
> So feel free to ignore my comments, the inconsistencies are not your
> fault and asking you to fix them would be too much.

Maybe we should open issues for all those inconsistencies and 
annoyances, so that they can be fixed or at least be documented.

At some point making a complete code review (not just patches) on isar 
would be great, just to find and fix those.

>>>> +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 ;)
> 
> Good first step. We actually have buildroot isar recipes in
> jailhouse-images, so this name can potentially get confusing for real.

I took this variable name from:

https://github.com/ilbers/isar/blob/22f2387b3791a1c6c88b8c4424285d7788c3743c/meta/classes/dpkg-base.bbclass#L49

So fixing this should be done as well then?

> 
>>>    
>>>> +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.
> 
> We currently generate and deploy in one task. The fun thing is that we
> need multiple commands and can probably end up in weird states if we
> get interrupted or fail.
> 
> Changing that general pattern is a discussion of its own, since it
> would change the API.
> 
> Staying in the current API i think the mount and generate directly into
> deploy ... no more chmod/chown mv ... whatever. Would be the cleanest
> way. We already have a reliable umount mechanism to clean up if things
> go wrong.
> 
> If you agree: Please include that deploydir/rootfs mount into the common
> mounts and update your imager. I will do the same for wic and maybe
> ext4.

Ok. Will do that.

> 
>> I am open to discussion about implementing some rules and
>> documentation about how things should be done in isar.
> 
> I assume that consistent references are equally efficient as
> documentation, maybe more.

Right!

Cheers,
Claudius

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

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

[Claudius Heine]

On 29/01/2019 09.16, [ext] Claudius Heine wrote:
>>>>> +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.
>>
>> We currently generate and deploy in one task. The fun thing is that we
>> need multiple commands and can probably end up in weird states if we
>> get interrupted or fail.
>>
>> Changing that general pattern is a discussion of its own, since it
>> would change the API.
>>
>> Staying in the current API i think the mount and generate directly into
>> deploy ... no more chmod/chown mv ... whatever. Would be the cleanest
>> way. We already have a reliable umount mechanism to clean up if things
>> go wrong.
>>
>> If you agree: Please include that deploydir/rootfs mount into the common
>> mounts and update your imager. I will do the same for wic and maybe
>> ext4.
> 
> Ok. Will do that.

Hmmm... You mean mounts that are done by `buildchroot_do_mounts`? 
Extending that function would cause deploydir and rootfs to be mounted 
for every recipe. `rootfs` doesn't make sense for normal recipes and 
mounting `deploydir` into every recipes workspace sound bad as well...

Maybe we would need a image specific mount function and then switch 
every image type to this. But I guess that would be a feature on its own.

Claudius

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

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

[Henning Schild]

On Tue, 29 Jan 2019 09:16:02 +0100
Claudius Heine <claudius.heine.ext@siemens.com> wrote:

> Hi Henning,
> 
> On 28/01/2019 18.22, Henning Schild wrote:
> > Am Mon, 28 Jan 2019 14:40:36 +0100
> > schrieb 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?  
> > 
> > I am not sure, just wanted to write it down. On the one hand it
> > would be nice to solve all drive-by issues as we go. On the other
> > hand that will slow down development and impose on contributors.
> > So feel free to ignore my comments, the inconsistencies are not your
> > fault and asking you to fix them would be too much.  
> 
> Maybe we should open issues for all those inconsistencies and 
> annoyances, so that they can be fixed or at least be documented.

We do not use issues, so we would need another way to track not fully
resolved discussions from the list. But i agree, it would be nice to
keep track of things in a structured way to fix them eventually.
Maybe a TODO.md?

> At some point making a complete code review (not just patches) on
> isar would be great, just to find and fix those.

I thought the same, maybe an Isar hackathon.

> >>>> +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 ;)  
> > 
> > Good first step. We actually have buildroot isar recipes in
> > jailhouse-images, so this name can potentially get confusing for
> > real.  
> 
> I took this variable name from:
> 
> https://github.com/ilbers/isar/blob/22f2387b3791a1c6c88b8c4424285d7788c3743c/meta/classes/dpkg-base.bbclass#L49
> 
> So fixing this should be done as well then?

I guess in that case the name is OK.

Henning

> >   
> >>>      
> >>>> +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.  
> > 
> > We currently generate and deploy in one task. The fun thing is that
> > we need multiple commands and can probably end up in weird states
> > if we get interrupted or fail.
> > 
> > Changing that general pattern is a discussion of its own, since it
> > would change the API.
> > 
> > Staying in the current API i think the mount and generate directly
> > into deploy ... no more chmod/chown mv ... whatever. Would be the
> > cleanest way. We already have a reliable umount mechanism to clean
> > up if things go wrong.
> > 
> > If you agree: Please include that deploydir/rootfs mount into the
> > common mounts and update your imager. I will do the same for wic
> > and maybe ext4.  
> 
> Ok. Will do that.
> 
> >   
> >> I am open to discussion about implementing some rules and
> >> documentation about how things should be done in isar.  
> > 
> > I assume that consistent references are equally efficient as
> > documentation, maybe more.  
> 
> Right!
> 
> Cheers,
> Claudius
> 
> > 
> > Henning
> >   
> >> 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  
> >>>      
> >>  
> >   
> 

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

[Henning Schild]

On Tue, 29 Jan 2019 09:24:38 +0100
Claudius Heine <claudius.heine.ext@siemens.com> wrote:

> On 29/01/2019 09.16, [ext] Claudius Heine wrote:
> >>>>> +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.  
> >>
> >> We currently generate and deploy in one task. The fun thing is
> >> that we need multiple commands and can probably end up in weird
> >> states if we get interrupted or fail.
> >>
> >> Changing that general pattern is a discussion of its own, since it
> >> would change the API.
> >>
> >> Staying in the current API i think the mount and generate directly
> >> into deploy ... no more chmod/chown mv ... whatever. Would be the
> >> cleanest way. We already have a reliable umount mechanism to clean
> >> up if things go wrong.
> >>
> >> If you agree: Please include that deploydir/rootfs mount into the
> >> common mounts and update your imager. I will do the same for wic
> >> and maybe ext4.  
> > 
> > Ok. Will do that.  
> 
> Hmmm... You mean mounts that are done by `buildchroot_do_mounts`? 
> Extending that function would cause deploydir and rootfs to be
> mounted for every recipe. `rootfs` doesn't make sense for normal
> recipes and mounting `deploydir` into every recipes workspace sound
> bad as well...

You are right, those should better not be mounted for every recipe.

> Maybe we would need a image specific mount function and then switch 
> every image type to this. But I guess that would be a feature on its
> own.

That might actually call for a changed task-chain for image-recipes.
Maybe do_deploy_deb should actually be do_deploy, with a deb and a
deploydir implementation.
Forget me not liking the extra mount and let us remember this one as an
item on the non-existing TODO list.

Henning

> Claudius
> 

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

[Claudius Heine]

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

Quoting Henning Schild (2019-01-29 10:10:50)
...
> > >> Do you think that refactoring should be part of this patchset?  
> > > 
> > > I am not sure, just wanted to write it down. On the one hand it
> > > would be nice to solve all drive-by issues as we go. On the other
> > > hand that will slow down development and impose on contributors.
> > > So feel free to ignore my comments, the inconsistencies are not your
> > > fault and asking you to fix them would be too much.  
> > 
> > Maybe we should open issues for all those inconsistencies and 
> > annoyances, so that they can be fixed or at least be documented.
> 
> We do not use issues, so we would need another way to track not fully
> resolved discussions from the list. But i agree, it would be nice to
> keep track of things in a structured way to fix them eventually.
> Maybe a TODO.md?

I am not really a fan of TODO lists that are checked into the VCS.

I would find it very strange to send patches to the TODO list... I don't
even know what the process there would be? Should I first send a patch
to the TODO list and then have discussions about it and afterwarts start
working on patches in isar?

I rather have some uncoupled list of TODO items somewhere without the
patch/review overhead.

> > At some point making a complete code review (not just patches) on
> > isar would be great, just to find and fix those.
> 
> I thought the same, maybe an Isar hackathon.

Maybe... I am not a hackathon expert. I don't know if quality codes
comes out of that...

Claudius

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

           PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
                             Keyserver: hkp://pool.sks-keyservers.net

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEb/LlnwDGvCgx2GTBEXPLGZgIsVMFAlxQHEEACgkQEXPLGZgI
sVMU7Q//V9y0LiS+yeGgeOXctxeJOzZMzyDc4AuNAnV2aRoFVsnb4LTylS3OLpof
zqQwGe4NszJpI/pLvyyH2vagrozC14AMWXEEAqsgUsN3XbLP92cljrvT8DI/YSzS
Vvk22CQK0eVkb3+JpftGHSwq4bIFKONzSOkKOzdh3HIXKTCbn47PeTvUX0qzGRGB
/rf5BiHIENPYntYPgNCFp9JIFcoskN63ANUgmcyDEaKaOEAugMBcwvqG9PMAoQN3
iza2M9cAOKavNMNReNtM7BCAe5umaytGY2FCLl+UMhRH96pivHNOeSQvUccBKWxV
2QDEM/d4MOdy1V2uZR+OAhvP7gFQyb1OGUwlkCg6GTWTe97AY+hpOo/BiiXLr3fu
F33EXDU+EI095F1/z6fZyqVPiwvZ9mvatRWA1PhipnoEEsuJX9DHheG8faawjk5J
e1hFoC/Jft00iNLDJ6aVfFz+hPeIR3s+A0i4X+ZkCFHLubu0IUx4p5mQbMlpdD8E
DaKPNts3i1NUXuR5cGOJOfUwnhegugRL4hvkVfQdAR3ColSLsHFNf83rVIggBa6m
wME68g+/70z60ZVJoRMsgrZkeO43yHOTJibF59eeczpMWlWGkBvcjLf+ydDiB75k
+aw/aExE/abmIPWMnIne33kMcy/9ibLkpzjllY1K+oxifvip6FA=
=52NA
-----END PGP SIGNATURE-----