[PATCH v3 0/6] openbmc: Use JFFS2 for RW overlay filesystem

[PATCH v3 0/6] openbmc: Use JFFS2 for RW overlay filesystem

[Andrew Jeffery]

The changes now include support for migrating between arbitrary^ filesystem
types (tested back and forth between ext4 and jffs2). The final patch in the
series switches the RWFS image to JFFS2. Lastly, a couple of clean-ups are
thrown in.

NOTE: This series requires the (pending) kernel fix for the spi-nor driver, and
is why I haven't done this as a pull-request. The fix has been applied to
openbmc/linux but is not yet integrated into openbmc/openbmc. If you'd like to
test this series I've added an extra patch which hacks the fix in. You can
fetch it all from github:

    git fetch git@github.com:amboar/openbmc.git jffs2-mtd6

Upgrading the BMC should be done in two stages after building the necessary
images:

1. Build image-kernel, image-initramfs and image-rwfs
2. Apply image-kernel and image-initramfs, and after the reboot,
3. Apply image-rwfs

To apply an image file, copy the it into /run/initramfs on the BMC, then reboot
(the BMC).

^ Migration is limited to filesystems detected by blkid(1), and JFFS2. JFFS2 is
  assumed if the filesystem is not identified by blkid(1).

Since v2:

  * Minor whitespace changes for consistency

  Address further suggestions from Milton:

  * Support migration with RW FS type detection in obmc-{init,update}.sh
  * Use bitbake OVERRIDES support for mkfs options
  * Pre-fill images with 0xFF to set NOR to erased state

Since v1:

  Address comments from Milton:

  * Drop mtd-utils-jffs2 for busybox-provided functionality
  * Drop re-arrangement of variables in obmc-{init,update}.sh
  * Simplify generation of image-rwfs
  * Move hard-coded ext4 mkfs options to (commented) options variable
  * Split out mount message fix

Andrew Jeffery (6):
  oe_mkimage: Use consistent indentation
  obmc-init.sh: Fix FS type in mount failure message
  obmc-{init,update}.sh: Cope with alternate RW FS types
  openbmc: Generate images that erase empty NOR sections
  openbmc: Lift mkfs opts to variables and use OVERRIDES capability
  openbmc: Switch RW FS from ext4 to jffs2

 meta-phosphor/classes/image-overlay.bbclass        |  2 +-
 .../obmc-phosphor-image_types_uboot.bbclass        | 29 +++++++++++++++++-----
 .../obmc-phosphor-initfs/files/obmc-init.sh        | 17 +++++++++++--
 .../obmc-phosphor-initfs/files/obmc-update.sh      | 18 +++++++++++---
 4 files changed, 54 insertions(+), 12 deletions(-)

-- 
2.5.0

[PATCH v3 1/6] oe_mkimage: Use consistent indentation

[Andrew Jeffery]

It was inconsistent with the rest of the file, which seems to use 7
spaces. In particular, vim-sleuth was detecting the indentation style
from the first indent(s) in the file, which were in the oe_mkimage
function.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
index c390c36..bf987bb 100644
--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
@@ -4,9 +4,9 @@ inherit image_types_uboot
 # the image load address and entry point. Override it here.
 
 oe_mkimage () {
-    mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_BASENAME} \
-        -a ${INITRD_IMAGE_LOADADDRESS} -e ${INITRD_IMAGE_ENTRYPOINT} \
-        -d ${DEPLOY_DIR_IMAGE}/$1 ${DEPLOY_DIR_IMAGE}/$1.u-boot
+       mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_BASENAME} \
+              -a ${INITRD_IMAGE_LOADADDRESS} -e ${INITRD_IMAGE_ENTRYPOINT} \
+              -d ${DEPLOY_DIR_IMAGE}/$1 ${DEPLOY_DIR_IMAGE}/$1.u-boot
 }
 
 INITRD_IMAGE_ENTRYPOINT ?= "0x40800000"
-- 
2.5.0

[PATCH v3 2/6] obmc-init.sh: Fix FS type in mount failure message

[Andrew Jeffery]

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 .../common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
index 6750de3..9674765 100644
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
@@ -114,7 +114,7 @@ then
 	msg="$(cat)" << HERE
 
 Mounting read-write $rwdev filesystem failed.  Please fix and run
-	mount $rwdev $rwdir -t $rwfs -o $rwopts
+	mount $rwdev $rwdir -t $rwfst -o $rwopts
 to to continue, or do change nothing to run from RAM for this boot.
 HERE
 	debug_takeover "$msg"
-- 
2.5.0

[PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Andrew Jeffery]

Busybox's blkid is a little hamstrung, requiring some processing of the
output to emulate what can be achieved with a couple of options with
blkid from util-linux.

Similar to findmtd(), the code for probe_fs_type() and blkid_fs_type()
is duplicated between obmc-{init,update}.sh. Some consideration should
be given to splitting out common functionality into well-defined
sourcable scripts.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 .../obmc-phosphor-initfs/files/obmc-init.sh            | 15 ++++++++++++++-
 .../obmc-phosphor-initfs/files/obmc-update.sh          | 18 +++++++++++++++---
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
index 9674765..1d268bb 100644
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
@@ -30,6 +30,19 @@ findmtd() {
 	echo $m
 }
 
+blkid_fs_type() {
+	# Emulate util-linux's `blkid -s TYPE -o value $1`
+	blkid $1 \
+		| tr ' ' '\n' \
+		| awk -F= '/TYPE/ { print $2 }' \
+		| tr -d '"'
+}
+
+probe_fs_type() {
+	fst=$(blkid_fs_type $1)
+	echo ${fst:=jffs2}
+}
+
 debug_takeover() {
 	echo "$@"
 	test -n "$@" && echo Enter password to try to manually fix.
@@ -72,7 +85,7 @@ rodev=/dev/mtdblock${rofs#mtd}
 rwdev=/dev/mtdblock${rwfs#mtd}
 
 rofst=squashfs
-rwfst=ext4
+rwfst=$(probe_fs_type $rwdev)
 roopts=ro
 rwopts=rw
 
diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
index face06d..cec4062 100755
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
@@ -35,10 +35,22 @@ findmtd() {
 	echo $m
 }
 
+blkid_fs_type() {
+	# Emulate util-linux's `blkid -s TYPE -o value $1`
+	blkid $1 \
+		| tr ' ' '\n' \
+		| awk -F= '/TYPE/ { print $2 }' \
+		| tr -d '"'
+}
+
+probe_fs_type() {
+	fst=$(blkid_fs_type $1)
+	echo ${fst:=jffs2}
+}
+
 rwfs=$(findmtd rwfs)
 
 rwdev=/dev/mtdblock${rwfs#mtd}
-rwfst=ext4
 rwopts=rw
 rorwopts=ro${rwopts#rw}
 
@@ -50,7 +62,7 @@ if test -n "$rwfs" && test -s whitelist
 then
 
 	mkdir -p $rwdir
-	mount $rwdev $rwdir -t $rwfst -o $rorwopts
+	mount $rwdev $rwdir -t $(probe_fs_type $rwdev) -o $rorwopts
 
 	while read f
 	do
@@ -87,7 +99,7 @@ done
 
 if test -d $save
 then
-	mount $rwdev $rwdir -t $rwfst -o $rwopts
+	mount $rwdev $rwdir -t $(probe_fs_type $rwdev) -o $rwopts
 	cp -rp $save/. $upper/
 	umount $rwdir
 fi
-- 
2.5.0

[PATCH v3 4/6] openbmc: Generate images that erase empty NOR sections

[Andrew Jeffery]

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
index bf987bb..96193a3 100644
--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
@@ -27,6 +27,13 @@ RWFS_SIZE ?= "4096"
 
 # IMAGE_POSTPROCESS_COMMAND += "do_generate_flash"
 
+mk_nor_image() {
+       image_dst="$1"
+       image_size_kb=$2
+       dd if=/dev/zero bs=1k count=${image_size_kb} \
+              | tr '\000' '\377' > ${image_dst}
+}
+
 do_generate_flash() {
        INITRD_CTYPE=${INITRAMFS_CTYPE}
        ddir="${DEPLOY_DIR_IMAGE}"
@@ -51,12 +58,12 @@ do_generate_flash() {
        fi
 
        oe_mkimage  "${initrd}" "${INITRD_CTYPE}" || bbfatal "oe_mkimage initrd"
-       dd if=/dev/zero of=${ddir}/${rwfs} bs=1k count=${RWFS_SIZE}
+       mk_nor_image ${ddir}/${rwfs} ${RWFS_SIZE}
        mkfs.${OVERLAY_BASETYPE} -b 4096 -F -O^huge_file ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
 
        dst="${ddir}/${FLASH_IMAGE_NAME}"
        rm -rf $dst
-       dd if=/dev/zero of=${dst} bs=1k count=${FLASH_SIZE}
+       mk_nor_image ${dst} ${FLASH_SIZE}
        dd if=${ddir}/${uboot} of=${dst} bs=1k seek=${FLASH_UBOOT_OFFSET}
        dd if=${ddir}/${kernel} of=${dst} bs=1k seek=${FLASH_KERNEL_OFFSET}
        dd if=${ddir}/${uinitrd} of=${dst} bs=1k seek=${FLASH_INITRD_OFFSET}
-- 
2.5.0

[PATCH v3 5/6] openbmc: Lift mkfs opts to variables and use OVERRIDES capability

[Andrew Jeffery]

Moves the ext4-specific mkfs options out to configuration variables.
OVERLAY_MKFS_OPTS can be overridden using the usual bitbake OVERRIDES
mechanism, where the override postfix is 'rwfs-${OVERLAY_BASETYPE}',
e.g. rwfs-ext4.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 .../classes/obmc-phosphor-image_types_uboot.bbclass          | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
index 96193a3..f75cdc1 100644
--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
@@ -23,6 +23,12 @@ FLASH_ROFS_OFFSET ?= "4864"
 FLASH_RWFS_OFFSET ?= "28672"
 RWFS_SIZE ?= "4096"
 
+# Avoid setting 'ext4' or 'jffs2' in OVERRIDES as this breaks the build. Prefix
+# with 'rwfs-' to avoid option collisions.
+DISTROOVERRIDES .= ":rwfs-${OVERLAY_BASETYPE}"
+
+OVERLAY_MKFS_OPTS_rwfs-ext4 = "-b 4096 -F -O^huge_file"
+
 # $(( ${FLASH_SIZE} - ${FLASH_RWFS_OFFSET} ))
 
 # IMAGE_POSTPROCESS_COMMAND += "do_generate_flash"
@@ -58,8 +64,12 @@ do_generate_flash() {
        fi
 
        oe_mkimage  "${initrd}" "${INITRD_CTYPE}" || bbfatal "oe_mkimage initrd"
+
        mk_nor_image ${ddir}/${rwfs} ${RWFS_SIZE}
-       mkfs.${OVERLAY_BASETYPE} -b 4096 -F -O^huge_file ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
+       if [ "${OVERLAY_BASETYPE}" != jffs2 ]; then
+              mkfs.${OVERLAY_BASETYPE} ${OVERLAY_MKFS_OPTS} ${ddir}/${rwfs} || \
+                     bbfatal "mkfs rwfs"
+       fi
 
        dst="${ddir}/${FLASH_IMAGE_NAME}"
        rm -rf $dst
-- 
2.5.0

[PATCH v3 6/6] openbmc: Switch RW FS from ext4 to jffs2

[Andrew Jeffery]

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 meta-phosphor/classes/image-overlay.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-phosphor/classes/image-overlay.bbclass b/meta-phosphor/classes/image-overlay.bbclass
index 1c6137d..8997aae 100644
--- a/meta-phosphor/classes/image-overlay.bbclass
+++ b/meta-phosphor/classes/image-overlay.bbclass
@@ -3,7 +3,7 @@ INITRD_IMAGE ?= "core-image-minimal-initramfs"
 INITRD ?= "${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}-${MACHINE}.cpio.${INITRD_CTYPE}${uboot}"
 
 IMAGE_BASETYPE ?= "squashfs-xz"
-OVERLAY_BASETYPE ?= "ext4"
+OVERLAY_BASETYPE ?= "jffs2"
 
 IMAGE_TYPES_${PN} += "${IMAGE_BASETYPE}"
 
-- 
2.5.0

Re: [PATCH v3 1/6] oe_mkimage: Use consistent indentation

[Joel Stanley]

On Wed, Feb 24, 2016 at 1:14 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> It was inconsistent with the rest of the file, which seems to use 7
> spaces. In particular, vim-sleuth was detecting the indentation style
> from the first indent(s) in the file, which were in the oe_mkimage
> function.

7 spaces seems kind of unusual.

Upstream bbclass files are a mixture; I'd go with 8 spaces as I
suspect that was the intent in this file.

>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> index c390c36..bf987bb 100644
> --- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> +++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> @@ -4,9 +4,9 @@ inherit image_types_uboot
>  # the image load address and entry point. Override it here.
>
>  oe_mkimage () {
> -    mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_BASENAME} \
> -        -a ${INITRD_IMAGE_LOADADDRESS} -e ${INITRD_IMAGE_ENTRYPOINT} \
> -        -d ${DEPLOY_DIR_IMAGE}/$1 ${DEPLOY_DIR_IMAGE}/$1.u-boot
> +       mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_BASENAME} \
> +              -a ${INITRD_IMAGE_LOADADDRESS} -e ${INITRD_IMAGE_ENTRYPOINT} \
> +              -d ${DEPLOY_DIR_IMAGE}/$1 ${DEPLOY_DIR_IMAGE}/$1.u-boot
>  }
>
>  INITRD_IMAGE_ENTRYPOINT ?= "0x40800000"
> --
> 2.5.0
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

Re: [PATCH v3 2/6] obmc-init.sh: Fix FS type in mount failure message

[Joel Stanley]

On Wed, Feb 24, 2016 at 1:14 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  .../common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh     | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> index 6750de3..9674765 100644
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> @@ -114,7 +114,7 @@ then
>         msg="$(cat)" << HERE
>
>  Mounting read-write $rwdev filesystem failed.  Please fix and run
> -       mount $rwdev $rwdir -t $rwfs -o $rwopts
> +       mount $rwdev $rwdir -t $rwfst -o $rwopts
>  to to continue, or do change nothing to run from RAM for this boot.
>  HERE
>         debug_takeover "$msg"
> --
> 2.5.0
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

Re: [PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Joel Stanley]

On Wed, Feb 24, 2016 at 1:15 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> Busybox's blkid is a little hamstrung, requiring some processing of the
> output to emulate what can be achieved with a couple of options with
> blkid from util-linux.
>
> Similar to findmtd(), the code for probe_fs_type() and blkid_fs_type()
> is duplicated between obmc-{init,update}.sh. Some consideration should
> be given to splitting out common functionality into well-defined
> sourcable scripts.

Why not do that? :)

>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  .../obmc-phosphor-initfs/files/obmc-init.sh            | 15 ++++++++++++++-
>  .../obmc-phosphor-initfs/files/obmc-update.sh          | 18 +++++++++++++++---
>  2 files changed, 29 insertions(+), 4 deletions(-)

> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> @@ -35,10 +35,22 @@ findmtd() {
>         echo $m
>  }
>
> +blkid_fs_type() {
> +       # Emulate util-linux's `blkid -s TYPE -o value $1`

Put an example of the input and what you expect as the output in the comment.

> +       blkid $1 \
> +               | tr ' ' '\n' \
> +               | awk -F= '/TYPE/ { print $2 }' \
> +               | tr -d '"'
> +}
> +
> +probe_fs_type() {
> +       fst=$(blkid_fs_type $1)
> +       echo ${fst:=jffs2}
> +}
> +
>  rwfs=$(findmtd rwfs)
>
>  rwdev=/dev/mtdblock${rwfs#mtd}
> -rwfst=ext4

Would it make sense to do the same as the init case, where you set
rwfst to $(probe_fs_type $rwdev)?

>  rwopts=rw
>  rorwopts=ro${rwopts#rw}
>

Re: [PATCH v3 4/6] openbmc: Generate images that erase empty NOR sections

[Joel Stanley]

On Wed, Feb 24, 2016 at 1:15 AM, Andrew Jeffery <andrew@aj.id.au> wrote:

Why do we need to do this?

> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> index bf987bb..96193a3 100644
> --- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> +++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> @@ -27,6 +27,13 @@ RWFS_SIZE ?= "4096"
>
>  # IMAGE_POSTPROCESS_COMMAND += "do_generate_flash"
>
> +mk_nor_image() {
> +       image_dst="$1"
> +       image_size_kb=$2
> +       dd if=/dev/zero bs=1k count=${image_size_kb} \
> +              | tr '\000' '\377' > ${image_dst}
> +}
> +
>  do_generate_flash() {
>         INITRD_CTYPE=${INITRAMFS_CTYPE}
>         ddir="${DEPLOY_DIR_IMAGE}"
> @@ -51,12 +58,12 @@ do_generate_flash() {
>         fi
>
>         oe_mkimage  "${initrd}" "${INITRD_CTYPE}" || bbfatal "oe_mkimage initrd"
> -       dd if=/dev/zero of=${ddir}/${rwfs} bs=1k count=${RWFS_SIZE}
> +       mk_nor_image ${ddir}/${rwfs} ${RWFS_SIZE}
>         mkfs.${OVERLAY_BASETYPE} -b 4096 -F -O^huge_file ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
>
>         dst="${ddir}/${FLASH_IMAGE_NAME}"
>         rm -rf $dst
> -       dd if=/dev/zero of=${dst} bs=1k count=${FLASH_SIZE}
> +       mk_nor_image ${dst} ${FLASH_SIZE}
>         dd if=${ddir}/${uboot} of=${dst} bs=1k seek=${FLASH_UBOOT_OFFSET}
>         dd if=${ddir}/${kernel} of=${dst} bs=1k seek=${FLASH_KERNEL_OFFSET}
>         dd if=${ddir}/${uinitrd} of=${dst} bs=1k seek=${FLASH_INITRD_OFFSET}
> --
> 2.5.0
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

Re: [PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Milton Miller II]

[-- Attachment #1: Type: text/html, Size: 3745 bytes --]

Re: [PATCH v3 1/6] oe_mkimage: Use consistent indentation

[Milton Miller II]

[-- Attachment #1: Type: text/html, Size: 1473 bytes --]

Re: [PATCH v3 4/6] openbmc: Generate images that erase empty NOR sections

[Andrew Jeffery]

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

On Wed, 2016-02-24 at 09:36 +1030, Joel Stanley wrote:
> On Wed, Feb 24, 2016 at 1:15 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> Why do we need to do this?

Primarily for the following patch where we conditionally call
mkfs.${OVERLAY_BASETYPE} when creating the RW FS. jffs2 can mount raw
erased partitions, so we ensure flash-palmetto and image-rwfs are in a
state where this can be achieved (after the image has been written to
the NOR). Erasing the partition then writing zeros isn't helpful.

To be fair we don't have to do this for the ${FLASH_SIZE} file, it's
not necessary, but Milton requested it and I don't really have a strong
opinion on the issue.

Andrew

> 
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> >  meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> > index bf987bb..96193a3 100644
> > --- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> > +++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> > @@ -27,6 +27,13 @@ RWFS_SIZE ?= "4096"
> > 
> >  # IMAGE_POSTPROCESS_COMMAND += "do_generate_flash"
> > 
> > +mk_nor_image() {
> > +       image_dst="$1"
> > +       image_size_kb=$2
> > +       dd if=/dev/zero bs=1k count=${image_size_kb} \
> > +              | tr '\000' '\377' > ${image_dst}
> > +}
> > +
> >  do_generate_flash() {
> >         INITRD_CTYPE=${INITRAMFS_CTYPE}
> >         ddir="${DEPLOY_DIR_IMAGE}"
> > @@ -51,12 +58,12 @@ do_generate_flash() {
> >         fi
> > 
> >         oe_mkimage  "${initrd}" "${INITRD_CTYPE}" || bbfatal "oe_mkimage initrd"
> > -       dd if=/dev/zero of=${ddir}/${rwfs} bs=1k count=${RWFS_SIZE}
> > +       mk_nor_image ${ddir}/${rwfs} ${RWFS_SIZE}
> >         mkfs.${OVERLAY_BASETYPE} -b 4096 -F -O^huge_file ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
> > 
> >         dst="${ddir}/${FLASH_IMAGE_NAME}"
> >         rm -rf $dst
> > -       dd if=/dev/zero of=${dst} bs=1k count=${FLASH_SIZE}
> > +       mk_nor_image ${dst} ${FLASH_SIZE}
> >         dd if=${ddir}/${uboot} of=${dst} bs=1k seek=${FLASH_UBOOT_OFFSET}
> >         dd if=${ddir}/${kernel} of=${dst} bs=1k seek=${FLASH_KERNEL_OFFSET}
> >         dd if=${ddir}/${uinitrd} of=${dst} bs=1k seek=${FLASH_INITRD_OFFSET}
> > --
> > 2.5.0
> > 
> > _______________________________________________
> > openbmc mailing list
> > openbmc@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/openbmc

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 5/6] openbmc: Lift mkfs opts to variables and use OVERRIDES capability

[Milton Miller II]

[-- Attachment #1: Type: text/html, Size: 1860 bytes --]

Re: [PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Andrew Jeffery]

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

On Tue, 2016-02-23 at 23:36 +0000, Milton Miller II wrote:
> (me found a new mailer option)
> 
> Around 02/23/2016 05:02PM in some time zone, Joel Stanley wrote:
> >
> >On Wed, Feb 24, 2016 at 1:15 AM, Andrew Jeffery <andrew@aj.id.au>
> >wrote:
> >> Busybox's blkid is a little hamstrung, requiring some processing of
> >>the
> >> output to emulate what can be achieved with a couple of options
> >>with
> >> blkid from util-linux.
> >>
> >> Similar to findmtd(), the code for probe_fs_type() and
> >>blkid_fs_type()
> >> is duplicated between obmc-{init,update}.sh. Some consideration
> >>should
> >> be given to splitting out common functionality into well-defined
> >> sourcable scripts.
> >
> >Why not do that? :)

Because I was trying to avoid bikeshedding where and how, at the
expense of introducing some minor problems :)

> 
> Well, for one , it reduces the reliability of the script and increases 
> the time by requiring opening and finding additional files.  But we 
> could put all these variables in that file with it.
> 
> I vote for later.

Soo: what am I extracting, where am I putting it, and are we going to
bother splitting what we extract into functional pieces? Or just lump
unrelated functionality into a single file?

> 
> >
> >Put an example of the input and what you expect as the output in the
> >comment.
> >
> 
> One can run the command if they need to see it, and its common to
> ubuntu and busybox.
> 
> >> + blkid $1 \
> >> + | tr ' ' '\n' \
> >> + | awk -F= '/TYPE/ { print $2 }' \
> >> + | tr -d '"'
> >> +}
> >> +
> 
> But more awk less pipes!
> 
> -       blkid $1 \
> -               | tr ' ' '\n' \
> -               | awk -F= '/TYPE/ { print $2 }' \
> -               | tr -d '"'
> +       blkid $1 | awk ' { if (match($0, / TYPE="[^"]*"/)) {
> +                               print substr ($0, RSTART+7, RLENGTH-8);
> +                               exit 0; }
> +                       }'

IMO this alternative is less readable: I find it relatively harder to
parse the regex (with character classes and repetition operators) and
then perform substring indexing, than to follow the pipeline of
substitutions and a straight text match. Horses for courses? Less pipes
means less subprocesses but do we really care here?

> 
> busybox awk does not have the gawk match third arg; the exit 0 
> says first arg only which should be redundant, and the extra new line 
> makes it a bit more readable at the expense of being the 4th line.
> alternatively we could look at each arg in a for loop but I decided it
> just added more code than matching the leading space.
> 
> This also gets rid of those trailing backslashes (otherwise accomplished 
> by putting the pipe at the end of the line).

Fair point about the EOL pipes. The backslashes are ugly.

> 
> (I'll submit this as a patch on top; you can fold or rebase).
> 
> >> +probe_fs_type() {>> + fst=$(blkid_fs_type $1)
> >> + echo ${fst:=jffs2}
> >> +}
> >> +
> >> rwfs=$(findmtd rwfs)
> >>
> >> rwdev=/dev/mtdblock${rwfs#mtd}
> >> -rwfst=ext4
> >
> >Would it make sense to do the same as the init case, where you set
> >rwfst to $(probe_fs_type $rwdev)?
> 
> Not as useful because the type needs be able to change 
> between before and after ... so we need 2 options and one can 
> not be probed until after flashing.

You beat me to it :)


> 
> milton
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 5/6] openbmc: Lift mkfs opts to variables and use OVERRIDES capability

[Andrew Jeffery]

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

On Wed, 2016-02-24 at 00:09 +0000, Milton Miller II wrote:
> 
> 
> Andrew Jeffery <andrew@aj.id.au> wrote:
> >Subject: [PATCH v3 5/6] openbmc: Lift mkfs opts to variables and use
> >OVERRIDES capability
> >
> >Moves the ext4-specific mkfs options out to configuration variables.
> >OVERLAY_MKFS_OPTS can be overridden using the usual bitbake OVERRIDES
> >mechanism, where the override postfix is 'rwfs-${OVERLAY_BASETYPE}',
> >e.g. rwfs-ext4.
> >
> >Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >---
> > .../classes/obmc-phosphor-image_types_uboot.bbclass | 12
> >+++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> >diff --git
> >a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> >b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> >index 96193a3..f75cdc1 100644
> >--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> >+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> >@@ -23,6 +23,12 @@ FLASH_ROFS_OFFSET ?= "4864"
> > FLASH_RWFS_OFFSET ?= "28672"
> > RWFS_SIZE ?= "4096"
> > 
> >+# Avoid setting 'ext4' or 'jffs2' in OVERRIDES as this breaks the
> >build. Prefix
> >+# with 'rwfs-' to avoid option collisions.
> >+DISTROOVERRIDES .= ":rwfs-${OVERLAY_BASETYPE}"
> 
> or more simply comment the reason it breaks:
> 
> # OVERRIDES of just the file system type are reserved for 
> # the primary image type.  Prefix with rwfs- for the overlay.

Gah, got a link to the documentation? I concluded this through blood,
sweat and bitbake, but didn't have anything formal to clear it up.

Andrew

> 
> otherwise,
> Acked-by: Milton Miller <miltonm@us.ibm.com>
> 
> milton
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 3/6] obmc-{init, update}.sh: Cope with alternate RW FS types

[Milton Miller II]

[-- Attachment #1: Type: text/html, Size: 2017 bytes --]

Re: [PATCH v3 1/6] oe_mkimage: Use consistent indentation

[Andrew Jeffery]

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

On Tue, 2016-02-23 at 23:54 +0000, Milton Miller II wrote:
> 
> About 02/23/2016 04:55PM in some time zone, Joel Stanley wrote:
> >On Wed, Feb 24, 2016 at 1:14 AM, Andrew Jeffery <andrew@aj.id.au>
> >wrote:
> >> It was inconsistent with the rest of the file, which seems to use 7
> >> spaces. In particular, vim-sleuth was detecting the indentation
> >style
> >> from the first indent(s) in the file, which were in the oe_mkimage
> >> function.
> >
> >7 spaces seems kind of unusual.
> >
> >Upstream bbclass files are a mixture; I'd go with 8 spaces as I
> >suspect that was the intent in this file.
> >
> 
> Looks like yocto-poky/meta/classes/image_types_uboot.bbclass
> upon which this is based used 4 spaces for the function that
> was copied.

So according to the openembedded wiki[1]:


* Use spaces for indentation as developers tends to use different
amount of spaces per one tab.
* Shell functions in OE-Core usually use tabs for indentation

but

* other layers usually use consistent indentation with 4 spaces (in
shell functions, python functions and for indentation of multi-line
variables)

Maybe we just be consistently inconsistent as well? :P

[1] http://www.openembedded.org/wiki/Styleguide

Anyway, this was another instance where I was trying to avoid the
bikeshedding; I wanted my editor to work with me not against me and so
just made it consistent with the rest of the file.

Happy to bikeshed out an acceptable coding style. Can we make the change to whatever we come up with a separate patch?

> 
> Going to 8 spaces puts one \ over 80 columns in do_generate_flash
> but its after a || so it can just be dropped and the shell will parse the 
> next line implicitly.
> 
> I don't care if we leave it at 7 for now or go to 8.

I don't think I've ever seen 7 spaces used for indentation anywhere
else. My preferences are either 4 or 8 spaces. Probably 4 given the
styleguide quote above, given we're layers, not OE-core.

Andrew

> 
> 
> >>
> >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >> ---
> >> meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 6
> >+++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> 
> milton
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]