[PATCH openbmc v6 00/18] Update flash update to be useable at runtime.

[PATCH openbmc v6 00/18] Update flash update to be useable at runtime.

[OpenBMC Patches]

The underlying storage for a file system can not be updated when it is in use.  The initial solution was to update the flash as the system was being shutdown, with output to the console.

Updates were made to the init script that allows the flash to not be used for a given boot but the update script was not enhanced to make this user friendly, and no documentation was provided to prepare the system for this mode.

This series partially addresses #293 by enhancing the update script to
1. provide more fine grained checks on update conflicts allowing use at runtime
2. provide additional checks that would cause failure to flash
3. provide user help and consistent tagging of error output
4. provide an error return code if the checks fail

In addition the series provides
* A fix to properly assemble the flash image, filling with `0xff` characters as intended.
* A cleanup to the build recipe and naming.
* A diagnostic message to shutdown if the update did not remove a file indicating it was successful.
* A method to build `init-options-base` into the initramfs providing options to `init` while allowing additional sources of options like the kernel command line. 
* A self containted image `initramfs-netboot.cpio.u-boot` for loading over the network.

These last two are intended to ease creating images for alternate network based deployment and testing, easing the use of the features and options already in the `init` script.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/openbmc/openbmc/297)
<!-- Reviewable:end -->


https://github.com/openbmc/openbmc/pull/297

Brad Bishop (3):
  Add pflash parallel build fix patch
  Remove unnecessary skeleton dependencies
  Removed unused host-ipmid-bt application

Milton D. Miller II (9):
  image_types_uboot: Don't truncate when assembling flash
  initfs: Fix recipe, remove unnecessary class
  initfs: shutdown: comment on update success
  initfs: update: Don't exec sh or sulogin on error just exit 1
  initfs: update: Do not cause an error if no files were saved
  initfs: update: add option to perform checks but skip actual update
  initfs: update: check individual mtd partitions for mounts
  initfs: update: add check for image size vs mtd size
  initfs: init: add file for base options

Milton Miller (6):
  initfs: update: Consistently add ERROR: and print to stderr
  initfs: update: add --help message
  image-types_uboot: Package a self contained netboot image
  initfs: update: handle no images to update
  initfs: update: Sanitze whitelist directory entries
  initfs: whitelist: Sanitize obmc events whitelist entry

[PATCH openbmc v6 01/18] Add pflash parallel build fix patch

[OpenBMC Patches]

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

This resolves https://github.com/openbmc/openbmc/issues/325

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
 .../common/recipes-bsp/pflash/pflash.bb            |  1 +
 ...-Fix-ARM-build-failure-with-parallel-make.patch | 28 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-make.patch

diff --git a/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash.bb b/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash.bb
index 496e900..ec43b4f 100644
--- a/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash.bb
+++ b/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash.bb
@@ -11,6 +11,7 @@ SRC_URI += "file://0003-external-Fix-pflash-install-target.patch"
 SRC_URI += "file://0004-external-Remove-m64-from-shared-CFLAGS-on-ARM.patch"
 SRC_URI += "file://0005-external-Create-shared-rules.mk.patch"
 SRC_URI += "file://0006-external-Add-dynamically-linked-pflash.patch"
+SRC_URI += "file://0007-external-Fix-ARM-build-failure-with-parallel-make.patch"
 
 LIC_FILES_CHKSUM = "file://${S}/LICENCE;md5=3b83ef96387f14655fc854ddc3c6bd57"
 
diff --git a/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-make.patch b/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-make.patch
new file mode 100644
index 0000000..a9d219c
--- /dev/null
+++ b/meta-openbmc-machines/meta-openpower/common/recipes-bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-make.patch
@@ -0,0 +1,28 @@
+From 55749b091414e0cf2691bd8f72c4cb48235b51c3 Mon Sep 17 00:00:00 2001
+From: Brad Bishop <bradleyb@fuzziesquirrel.com>
+Date: Mon, 20 Jun 2016 20:31:07 -0400
+Subject: [PATCH] external: Fix ARM build failure with parallel make
+
+Arch headers need to be linked in before compiling.
+
+Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
+---
+ external/common/rules.mk | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/external/common/rules.mk b/external/common/rules.mk
+index 5558cd3..bb12fd5 100644
+--- a/external/common/rules.mk
++++ b/external/common/rules.mk
+@@ -67,7 +67,7 @@ arch_clean:
+ 
+ $(ARCH_SRC): | common
+ 
+-$(ARCH_OBJS): common-%.o: common/%.c
++$(ARCH_OBJS): common-%.o: common/%.c $(ARCH_LINKS)
+ 	$(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@
+ 
+ common-arch_flash.o: $(ARCH_OBJS)
+-- 
+1.8.3.1
+
-- 
2.9.0

[PATCH openbmc v6 02/18] Remove unnecessary skeleton dependencies

[OpenBMC Patches]

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

The network and settings daemons had unnecessary startup dependencies
on the skeleton service.

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
 meta-phosphor/common/recipes-phosphor/network/network/network.service   | 2 --
 .../common/recipes-phosphor/settings/settings/settings.service          | 2 --
 2 files changed, 4 deletions(-)

diff --git a/meta-phosphor/common/recipes-phosphor/network/network/network.service b/meta-phosphor/common/recipes-phosphor/network/network/network.service
index 85a44bb..6d0aad7 100644
--- a/meta-phosphor/common/recipes-phosphor/network/network/network.service
+++ b/meta-phosphor/common/recipes-phosphor/network/network/network.service
@@ -1,7 +1,5 @@
 [Unit]
 Description=Network DBUS object
-Requires=skeleton.service
-After=skeleton.service
 
 [Service]
 ExecStart=/usr/sbin/netman.py
diff --git a/meta-phosphor/common/recipes-phosphor/settings/settings/settings.service b/meta-phosphor/common/recipes-phosphor/settings/settings/settings.service
index 24c4a5a..deed98f 100755
--- a/meta-phosphor/common/recipes-phosphor/settings/settings/settings.service
+++ b/meta-phosphor/common/recipes-phosphor/settings/settings/settings.service
@@ -1,7 +1,5 @@
 [Unit]
 Description=Settings DBUS Object
-Requires=skeleton.service
-After=skeleton.service
 
 [Service]
 ExecStart=/usr/sbin/settings_manager.py
-- 
2.9.0

[PATCH openbmc v6 03/18] Removed unused host-ipmid-bt application

[OpenBMC Patches]

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

This was deprecated in favor of btbridged.

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
 .../common/recipes-phosphor/host-ipmid/host-ipmi-bt.bb  | 17 -----------------
 .../host-ipmid/host-ipmi-bt/host-ipmi-bt.service        |  9 ---------
 2 files changed, 26 deletions(-)
 delete mode 100644 meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt.bb
 delete mode 100644 meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt/host-ipmi-bt.service

diff --git a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt.bb b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt.bb
deleted file mode 100644
index 830f754..0000000
--- a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt.bb
+++ /dev/null
@@ -1,17 +0,0 @@
-SUMMARY = "Phosphor OpenBMC BT to DBUS"
-DESCRIPTION = "Phosphor OpenBMC BT to DBUS."
-PR = "r1"
-
-RDEPENDS_${PN} += "python-subprocess"
-
-SYSTEMD_SERVICE_${PN} = "host-ipmi-bt.service"
-
-inherit obmc-phosphor-pydbus-service
-inherit obmc-phosphor-host-ipmi-hw
-
-S = "${WORKDIR}/git"
-SRC_URI += "git://github.com/openbmc/skeleton.git;subpath=bin;destsuffix=git"
-SRCREV="${AUTOREV}"
-
-SCRIPT_NAME = "ipmid.py"
-INSTALL_NAME = "host-ipmi-bt"
diff --git a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt/host-ipmi-bt.service b/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt/host-ipmi-bt.service
deleted file mode 100644
index 3e40b46..0000000
--- a/meta-phosphor/common/recipes-phosphor/host-ipmid/host-ipmi-bt/host-ipmi-bt.service
+++ /dev/null
@@ -1,9 +0,0 @@
-[Unit]
-Description=Phosphor OpenBMC BT to DBUS 
-
-[Service]
-Restart=always
-ExecStart=/usr/sbin/host-ipmi-bt
-
-[Install]
-WantedBy=multi-user.target
-- 
2.9.0

[PATCH openbmc v6 04/18] image_types_uboot: Don't truncate when assembling flash

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

The generate_flash_image step was creating a file then using dd to
fill in the pieces, but missed adding the conv=notrunc flag, so each
step was shortening the file to its output.

Add the forgotten conversion flag to each dd command.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
index adb86e0..164d1ed 100644
--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
@@ -76,11 +76,11 @@ do_generate_flash() {
        dst="${ddir}/${FLASH_IMAGE_NAME}"
        rm -rf $dst
        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}
-       dd if=${ddir}/${rootfs} of=${dst} bs=1k seek=${FLASH_ROFS_OFFSET}
-       dd if=${ddir}/${rwfs} of=${dst} bs=1k seek=${FLASH_RWFS_OFFSET}
+       dd if=${ddir}/${uboot} of=${dst} conv=notrunc bs=1k seek=${FLASH_UBOOT_OFFSET}
+       dd if=${ddir}/${kernel} of=${dst} bs=1k conv=notrunc seek=${FLASH_KERNEL_OFFSET}
+       dd if=${ddir}/${uinitrd} of=${dst} bs=1k conv=notrunc seek=${FLASH_INITRD_OFFSET}
+       dd if=${ddir}/${rootfs} of=${dst} bs=1k conv=notrunc seek=${FLASH_ROFS_OFFSET}
+       dd if=${ddir}/${rwfs} of=${dst} bs=1k conv=notrunc seek=${FLASH_RWFS_OFFSET}
        dstlink="${ddir}/${FLASH_IMAGE_LINK}"
        rm -rf $dstlink
        ln -sf ${FLASH_IMAGE_NAME} $dstlink
-- 
2.9.0

[PATCH openbmc v6 05/18] initfs: Fix recipe, remove unnecessary class

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

Because the recipe was named with trailing part init instead
of initfs, the package didn't match the package name requiring
PROVIDES, RPROVIDES and other bitbake workarounds.  Fix the recipe
name to match the package removing the overly verbose packaging.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 meta-phosphor/classes/obmc-phosphor-initfs.bbclass                    | 4 ----
 .../{obmc-phosphor-init.bb => obmc-phosphor-initfs.bb}                | 1 -
 2 files changed, 5 deletions(-)
 delete mode 100644 meta-phosphor/classes/obmc-phosphor-initfs.bbclass
 rename meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/{obmc-phosphor-init.bb => obmc-phosphor-initfs.bb} (96%)

diff --git a/meta-phosphor/classes/obmc-phosphor-initfs.bbclass b/meta-phosphor/classes/obmc-phosphor-initfs.bbclass
deleted file mode 100644
index de7923e..0000000
--- a/meta-phosphor/classes/obmc-phosphor-initfs.bbclass
+++ /dev/null
@@ -1,4 +0,0 @@
-# Common code for recipes that implement Phosphor OpenBMC filesystem
-
-RPROVIDES_${PN} += "obmc-phosphor-initfs"
-PROVIDES += "obmc-phosphor-initfs"
diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
similarity index 96%
rename from meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb
rename to meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
index 3fa88c9..f81e21d 100644
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
@@ -3,7 +3,6 @@ DESCRIPTION = "Phosphor OpenBMC filesytem mount reference implementation."
 PR = "r1"
 
 inherit obmc-phosphor-license
-inherit obmc-phosphor-initfs
 
 S = "${WORKDIR}"
 SRC_URI += "file://obmc-init.sh"
-- 
2.9.0

[PATCH openbmc v6 06/18] initfs: shutdown: comment on update success

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

Print an error from the shutdown script when the update fails.

Update is changing to return codes when its preconditions are not
met, so be more verbose about its success to provide a means to
debug flash updates not occurring.

We could add a sulogin or sushell but its not clear what would be
desired here.  If so the message should include "rmdir /oldroot
to get a PID 1 shell" like /takever works in init.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh  | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh
index 8d5d0c9..8d5c672 100644
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh
@@ -44,6 +44,14 @@ then
 	if test -x $update
 	then
 		$update --clean-saved-files
+		remaining=$(ls $image*)
+		if test -n "$remaining"
+		then
+			echo 1>&2 "Flash update failed to flash these images:"
+			echo 1>&2 "$remaining"
+		else
+			echo "Flash update completed."
+		fi
 	else
 		echo 1>&2 "Flash update requested but $update program missing!"
 	fi
-- 
2.9.0

[PATCH openbmc v6 07/18] initfs: update: Don't exec sh or sulogin on error just exit 1

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

When update was written it was exec'd from the shutdown script
and hence took over pid 1.  Since exiting in that environment was
a panic situation, the script instead started a rescue shell with
its output presumably on the console.

The calling convention was updated to be a simple invocation in
commit dbacf104885c ("obmc-initfs: run update as a sub-script")
but the error handling was not updated.  That error handling is
now becoming a hinderance to use from additional environments so
change it.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh      | 23 ++++++----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

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 aa8fd89..f8e551c 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
@@ -2,9 +2,6 @@
 
 echo update: "$@"
 
-export PS1=update-sh#\ 
-# exec /bin/sh
-
 cd /
 if ! test -r /proc/mounts || ! test -f /proc/mounts
 then
@@ -21,12 +18,12 @@ then
 	mkdir -p /dev
 	mount -t devtmpfs dev dev
 fi
-while grep mtd /proc/mounts
-do
+
+if grep mtd /proc/mounts
+then
 	echo 1>&2 "Error: A mtd device is mounted."
-	sulogin
-	# exec /bin/sh
-done
+	exit 1
+fi
 
 findmtd() {
 	m=$(grep -xl "$1" /sys/class/mtd/*/name)
@@ -130,7 +127,7 @@ do
 	if test -z "$m"
 	then
 		echo 1>&2  "Unable to find mtd partiton for ${f##*/}."
-		exec /bin/sh
+		exit 1
 	fi
 done
 
@@ -173,11 +170,3 @@ then
 fi
 
 exit
-
-# NOT REACHED without edit
-# NOT REACHED without edit
-
-echo "Flash completed.  Inspect, cleanup and reboot -f to continue."
-
-export PS1=update-sh#\ 
-exec /bin/sh
-- 
2.9.0

[PATCH openbmc v6 08/18] initfs: update: Do not cause an error if no files were saved

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

If there were no files in the read-write overlay file system that
were in the persistent file list, then the save directory will
not be created and therefore not exist.  Skip attempting copying
the non-existent directory in that case to avoid the error message.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 f8e551c..f0ca989 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
@@ -144,7 +144,7 @@ do
 	flashcp -v $f /dev/$m && rm $f
 done
 
-if test "x$toram" = xy
+if test -d $save -a "x$toram" = xy
 then
 	mkdir -p $upper
 	cp -rp $save/. $upper/
-- 
2.9.0

[PATCH openbmc v6 09/18] initfs: update: Consistently add ERROR: and print to stderr

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Make sure all error messages start with the tag ERROR.  Also redirect
output to stderr (once case had the file descriptors backwards).

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh    | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

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 f0ca989..4249161 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
@@ -21,7 +21,7 @@ fi
 
 if grep mtd /proc/mounts
 then
-	echo 1>&2 "Error: A mtd device is mounted."
+	echo 1>&2 "ERROR: A mtd device is mounted."
 	exit 1
 fi
 
@@ -65,6 +65,8 @@ toram=
 whitelist=/run/initramfs/whitelist
 image=/run/initramfs/image-
 
+E="ERROR:"
+
 while test "$1" != "${1#-}"
 do
 	case "$1" in
@@ -90,7 +92,7 @@ do
 		toram=y
 		shift ;;
 	*)
-		echo 2>&1 "Unknown option $1"
+		echo 1>&2 "$E Unknown option $1"
 		exit 1 ;;
 	esac
 done
@@ -126,7 +128,7 @@ do
 	m=$(findmtd ${f#$image})
 	if test -z "$m"
 	then
-		echo 1>&2  "Unable to find mtd partiton for ${f##*/}."
+		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
 		exit 1
 	fi
 done
-- 
2.9.0

[PATCH openbmc v6 10/18] initfs: update: add option to perform checks but skip actual update

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

Provide a way to check if update is likely to succeed without
attempting actual updates.

This will check the image names correspond to mtd partition names,
and will be enhanced with additional checking for file size and
individual partition mounts.

This is not called --dry-run because it will still do file save
and restore if those are enabled.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh      | 31 +++++++++++++---------
 1 file changed, 19 insertions(+), 12 deletions(-)

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 4249161..4442e5b 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
@@ -57,6 +57,7 @@ upper=$rwdir/cow
 save=/run/save/${upper##*/}
 
 mounted=
+doflash=y
 doclean=
 dosave=y
 dorestore=y
@@ -88,6 +89,9 @@ do
 	--restore-files)
 		dorestore=y
 		shift ;;
+	--no-flash)
+		doflash=
+		shift ;;
 	--copy-files)
 		toram=y
 		shift ;;
@@ -133,18 +137,21 @@ do
 	fi
 done
 
-for f in $image*
-do
-	if test ! -s $f
-	then
-		echo "Skipping empty update of ${f#$image}."
-		rm $f
-		continue
-	fi
-	m=$(findmtd ${f#$image})
-	echo "Updating ${f#$image}..."
-	flashcp -v $f /dev/$m && rm $f
-done
+if test -n "$doflash"
+then
+	for f in $image*
+	do
+		if test ! -s $f
+		then
+			echo "Skipping empty update of ${f#$image}."
+			rm $f
+			continue
+		fi
+		m=$(findmtd ${f#$image})
+		echo "Updating ${f#$image}..."
+		flashcp -v $f /dev/$m && rm $f
+	done
+fi
 
 if test -d $save -a "x$toram" = xy
 then
-- 
2.9.0

[PATCH openbmc v6 11/18] initfs: update: check individual mtd partitions for mounts

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

Instead of checking for any mtd device, only check mtd devices
that are to be updated.  Include child mtd devices when looking
for mounted filesystems.

If this check is suppressed flash updates are suppressed.
Suppressing the check may be useful to verify a update could
succeed at shutdown.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh      | 47 +++++++++++++++++++---
 1 file changed, 41 insertions(+), 6 deletions(-)

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 4442e5b..22c9dcc 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
@@ -19,11 +19,34 @@ then
 	mount -t devtmpfs dev dev
 fi
 
-if grep mtd /proc/mounts
-then
-	echo 1>&2 "ERROR: A mtd device is mounted."
-	exit 1
-fi
+# mtd number N with mtd name Name can be mounted via mtdN, or mtd:Name
+# (with a mtd aware fs) or by /dev/mtdblockN (with a mtd or block fs).
+mtdismounted() {
+	m=${1##mtd}
+	if grep -s "mtdblock$m " /proc/mounts || grep -s "mtd$m " /proc/mounts
+	then
+		return 0
+	fi
+	n=$(cat /sys/class/mtd/mtd$m/name)
+	if test -n "$n" && grep -s "mtd:$n " /proc/mounts
+	then
+		return 0
+	fi
+	return 1
+}
+
+# Detect child partitions when the whole flash is to be updated.
+# Ignore mtdNro and mtdblockN names in the class subsystem directory.
+childmtds() {
+	for m in /sys/class/mtd/$1/mtd*
+	do
+		m=${m##*/}
+		if test "${m%ro}" = "${m#mtdblock}"
+		then
+			echo $m
+		fi
+	done
+}
 
 findmtd() {
 	m=$(grep -xl "$1" /sys/class/mtd/*/name)
@@ -62,10 +85,10 @@ doclean=
 dosave=y
 dorestore=y
 toram=
+checkmount=y
 
 whitelist=/run/initramfs/whitelist
 image=/run/initramfs/image-
-
 E="ERROR:"
 
 while test "$1" != "${1#-}"
@@ -92,6 +115,10 @@ do
 	--no-flash)
 		doflash=
 		shift ;;
+	--ignore-mount)
+		checkmount=
+		doflash=
+		shift ;;
 	--copy-files)
 		toram=y
 		shift ;;
@@ -135,6 +162,14 @@ do
 		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
 		exit 1
 	fi
+	for s in $m $(childmtds $m)
+	do
+		if test -n "$checkmount" && mtdismounted $s
+		then
+			echo 1>&2 "$E Device $s is mounted, ${f##*/} is busy."
+			exit 1
+		fi
+	done
 done
 
 if test -n "$doflash"
-- 
2.9.0

[PATCH openbmc v6 12/18] initfs: update: add check for image size vs mtd size

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

The flashcp command will check the file size vs the partition size,
so add a check when looking at images.

Use stat -L -c "%s" to get the file size, and compare it to the mtd
size from the sysfs size attribute.

If this check is suppressed the enforcement is left to flashcp.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh           | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

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 22c9dcc..06e0e74 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
@@ -48,6 +48,14 @@ childmtds() {
 	done
 }
 
+toobig() {
+	if test $(stat -L -c "%s" "$1") -gt $(cat /sys/class/mtd/"$2"/size)
+	then
+		return 0
+	fi
+	return 1
+}
+
 findmtd() {
 	m=$(grep -xl "$1" /sys/class/mtd/*/name)
 	m=${m%/name}
@@ -85,6 +93,7 @@ doclean=
 dosave=y
 dorestore=y
 toram=
+checksize=y
 checkmount=y
 
 whitelist=/run/initramfs/whitelist
@@ -115,6 +124,9 @@ do
 	--no-flash)
 		doflash=
 		shift ;;
+	--ignore-size)
+		checksize=
+		shift ;;
 	--ignore-mount)
 		checkmount=
 		doflash=
@@ -162,6 +174,11 @@ do
 		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
 		exit 1
 	fi
+	if test -n "$checksize" && toobig "$f" "$m"
+	then
+		echo 1>&2 "$E Image ${f##*/} too big for $m."
+		exit 1
+	fi
 	for s in $m $(childmtds $m)
 	do
 		if test -n "$checkmount" && mtdismounted $s
-- 
2.9.0

[PATCH openbmc v6 13/18] initfs: update: add --help message

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Now that update is intended for use at runtime and not just by
the init and shutdown scripts, add a short help message listing
the options so people don't have to understand how to read shell.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh        | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

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 06e0e74..1dbf65f 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
@@ -103,6 +103,24 @@ E="ERROR:"
 while test "$1" != "${1#-}"
 do
 	case "$1" in
+	--help)
+		cat <<HERE
+Usage: $0 [options] -- Write images in /run/initramfs to flash (/dev/mtd*)
+    --help                    Show this message
+    --no-flash	              Don't attempt to write images to flash
+    --ignore-size             Don't compare image size to mtd device size
+    --ignore-mount            Don't check if destination is mounted
+    --save-files              Copy whitelisted files to save directory in RAM
+    --no-save-files           Don't copy whitelisted files to save directory
+    --copy-files              Copy files from save directory to rwfs mountpoint
+    --restore-files           Restore files from save directory to rwfs layer
+    --no-restore-files        Don't restore saved files from ram to rwfs layer
+    --clean-saved-files       Delete saved whitelisted files from RAM
+    --no-clean-saved-files    Retain saved whitelisted files in RAM
+HERE
+
+	    exit 0 ;;
+
 	--no-clean-saved-files)
 		doclean=
 		shift ;;
@@ -135,7 +153,7 @@ do
 		toram=y
 		shift ;;
 	*)
-		echo 1>&2 "$E Unknown option $1"
+		echo 1>&2 "$E Unknown option $1.  Try $0 --help."
 		exit 1 ;;
 	esac
 done
-- 
2.9.0

[PATCH openbmc v6 14/18] initfs: init: add file for base options

[OpenBMC Patches]

From: "Milton D. Miller II" <miltonm@us.ibm.com>

Allow an initrd image to include options for init while still allowing
additional options to come from the kernel command line and/or u-boot
environment variables.

This will allow building a netboot image that can specify to run
from RAM while allowing the command line to initiate debug or skip
copying files from the read-write overlay file system.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh   | 10 +++++++++-
 1 file changed, 9 insertions(+), 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 c37b3d5..70af93d 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
@@ -184,6 +184,7 @@ fsckbase=/sbin/fsck.
 fsck=$fsckbase$rwfst
 fsckopts=-a
 optfile=/run/initramfs/init-options
+optbase=/run/initramfs/init-options-base
 urlfile=/run/initramfs/init-download-url
 update=/run/initramfs/update
 
@@ -192,9 +193,16 @@ then
 	cp /${optfile##*/} $optfile
 fi
 
+if test -e /${optbase##*/}
+then
+	cp /${optbase##*/} $optbase
+else
+	touch $optbase
+fi
+
 if test ! -f $optfile
 then
-	cat /proc/cmdline > $optfile
+	cat /proc/cmdline $optbase > $optfile
 	get_fw_env_var openbmcinit >> $optfile
 	get_fw_env_var openbmconce >> $optfile
 fi
-- 
2.9.0

[PATCH openbmc v6 15/18] image-types_uboot: Package a self contained netboot image

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Create a cpio containing the base read-only image, and create a
single combined initramfs image consisting of the image-rofs
and the existing initramfs.

There has been a desire to have a self contained netboot image present
for developer testing and experimentation.   The init script already
supports having the rofs layer sourced from a file packaged into the
initramfs, and will use it if found.

While this image will not fit in the current flash layout in
the initrd space, it will load over the network.  This method
leaves the squashfs compressed in RAM, decompreses is invoked as
needed at runtime, trading boot time and memory space for runtime
decompress on demand.  This will also give similar file access
overheads and performance to reading the data from flash.

Because the rofs is already compressed with xz compression,
don't try to compress this cpio (the cpio packaging adds about 40
bytes of ascii before and after, plus some alignment padding).
Because the kernel requires uncompressed cpios to appear on a
4-byte boundary with 0 byte fill between archives, place this
cpio first in the combined image.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
index 164d1ed..1c0d9b0 100644
--- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
@@ -51,6 +51,8 @@ do_generate_flash() {
        uinitrd="${initrd}.u-boot"
        rootfs="${IMAGE_LINK_NAME}.${IMAGE_BASETYPE}"
        rwfs="rwfs.${OVERLAY_BASETYPE}"
+       rofsimg=rofs.${IMAGE_BASETYPE}.cpio
+       netimg=initramfs-netboot.cpio
 
        if [ ! -f $ddir/$kernel ]; then
               bbfatal "Kernel file ${ddir}/${kernel} does not exist"
@@ -94,4 +96,12 @@ do_generate_flash() {
 
        tar -h -cvf ${ddir}/${MACHINE}-${DATETIME}.all.tar -C ${ddir} image-bmc
        tar -h -cvf ${ddir}/${MACHINE}-${DATETIME}.tar -C ${ddir} image-u-boot image-kernel image-initramfs image-rofs image-rwfs
+
+       # Package the root image (rofs layer) with the initramfs for net booting.
+       # Uses the symlink above to get the desired name in the cpio
+       ( cd $ddir && echo image-rofs | cpio -oHnewc -L > ${rofsimg} )
+       # Prepend the rofs cpio -- being uncompressed it must be 4-byte aligned
+       cat ${ddir}/${rofsimg} ${ddir}/${initrd} > ${ddir}/${netimg}
+       oe_mkimage  "${netimg}" "${INITRD_CTYPE}"
+
 }
-- 
2.9.0

[PATCH openbmc v6 16/18] initfs: update: handle no images to update

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Notify the user if image wildcard expansion fails instead of printing
message about failing to find partition to flash.

The update script errors with the message that it can't figure out what
partition to flash for /run/initramfs/image-* if there are no images
pending.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh               | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

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 1dbf65f..7120a18 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
@@ -99,6 +99,7 @@ checkmount=y
 whitelist=/run/initramfs/whitelist
 image=/run/initramfs/image-
 E="ERROR:"
+imglist=
 
 while test "$1" != "${1#-}"
 do
@@ -184,7 +185,15 @@ then
 	fi
 fi
 
-for f in $image*
+imglist=$(echo $image*)
+if test "$imglist" = "$image*" -a ! -e "$imglist"
+then
+	# shell didn't expand the wildcard, so no files exist
+	echo "No images found to update."
+	imglist=
+fi
+
+for f in $imglist
 do
 	m=$(findmtd ${f#$image})
 	if test -z "$m"
@@ -209,7 +218,7 @@ done
 
 if test -n "$doflash"
 then
-	for f in $image*
+	for f in $imglist
 	do
 		if test ! -s $f
 		then
-- 
2.9.0

[PATCH openbmc v6 17/18] initfs: update: Sanitze whitelist directory entries

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Repeatedly strip trailing / and /. from whitelist entries and
fail if an entry includes /../ or ends with /.. or doesn't start
with a /.  Also use the entries quoted to avoid any glob.

It was noticed the save code was saving directories that ended in /
into a subdirectory of the last component name.  This was traced
the the code creating the directory just stripping the last /
and then copying to the directory.

Choose to sanitize the entry where possible for ease of use verses
a small performance penality.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../obmc-phosphor-initfs/files/obmc-update.sh             | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

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 7120a18..bfacd00 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
@@ -170,13 +170,24 @@ then
 
 	while read f
 	do
-		if ! test -e $upper/$f
+		# Entries shall start with /, no trailing /.. or embedded /../
+		if test "/${f#/}" != "$f" -o "${f%/..}" != "${f#*/../}"
+		then
+			echo 1>&2 "WARNING: Skipping bad whitelist entry $f."
+			continue
+		fi
+		if ! test -e "$upper/$f"
 		then
 			continue
 		fi
 		d="$save/$f"
+		while test "${d%/}" != "${d%/.}"
+		do
+			d="${d%/.}"
+			d="${d%/}"
+		done
 		mkdir -p "${d%/*}"
-		cp -rp $upper/$f "${d%/*}/"
+		cp -rp "$upper/$f" "${d%/*}/"
 	done < $whitelist
 
 	if test -n "$mounted"
-- 
2.9.0

[PATCH openbmc v6 18/18] initfs: whitelist: Sanitize obmc events whitelist entry

[OpenBMC Patches]

From: Milton Miller <miltonm@us.ibm.com>

Remove the trailing / so that the update script doesn't have to loop.

It was determined the trailing slash confused the code that makes
the save subdirectory.  The update script was fixed but this removes
the extra work needed.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
---
 .../common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
index 603cec7..c35a552 100644
--- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
+++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
@@ -6,4 +6,4 @@
 /etc/group
 /etc/shadow
 /etc/gshadow
-/var/lib/obmc/events/
+/var/lib/obmc/events
-- 
2.9.0

Re: [PATCH openbmc v6 00/18] Update flash update to be useable at runtime.

[Andrew Jeffery]

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

Hi Milton,

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> The underlying storage for a file system can not be updated when it is in use.  The initial solution was to update the flash as the system was being shutdown, with output to the console.
> 
> Updates were made to the init script that allows the flash to not be used for a given boot but the update script was not enhanced to make this user friendly, and no documentation was provided to prepare the system for this mode.
> 
> This series partially addresses #293 by enhancing the update script to
> 1. provide more fine grained checks on update conflicts allowing use at runtime
> 2. provide additional checks that would cause failure to flash
> 3. provide user help and consistent tagging of error output
> 4. provide an error return code if the checks fail
> 
> In addition the series provides
> * A fix to properly assemble the flash image, filling with `0xff` characters as intended.
> * A cleanup to the build recipe and naming.
> * A diagnostic message to shutdown if the update did not remove a file indicating it was successful.
> * A method to build `init-options-base` into the initramfs providing options to `init` while allowing additional sources of options like the kernel command line. 
> * A self containted image `initramfs-netboot.cpio.u-boot` for loading over the network.
> 
> These last two are intended to ease creating images for alternate network based deployment and testing, easing the use of the features and options already in the `init` script.
> 
> 
> ---
> This change is [https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/openbmc/openbmc/297)
> 
> 
> 
> https://github.com/openbmc/openbmc/pull/297
> 
> Brad Bishop (3):
>   Add pflash parallel build fix patch
>   Remove unnecessary skeleton dependencies
>   Removed unused host-ipmid-bt application
> 
> Milton D. Miller II (9):
...
> Milton Miller (6):

Nit pick: Probably worth reconfiguring git to use a consistent user.name.

Cheers,

Andrew

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

Re: [PATCH openbmc v6 01/18] Add pflash parallel build fix patch

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> 
> This resolves https://github.com/openbmc/openbmc/issues/325
> 
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>

Brad's sent this to the skiboot list, maybe we should just hold off
until it's submitted there and drop this patch from the series?

Andrew

> ---
>  .../common/recipes-bsp/pflash/pflash.bb            |  1 +
>  ...-Fix-ARM-build-failure-with-parallel-make.patch | 28
> ++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 meta-openbmc-machines/meta-
> openpower/common/recipes-bsp/pflash/pflash/0007-external-Fix-ARM-
> build-failure-with-parallel-make.patch
> 
> diff --git a/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash.bb b/meta-openbmc-machines/meta-
> openpower/common/recipes-bsp/pflash/pflash.bb
> index 496e900..ec43b4f 100644
> --- a/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash.bb
> +++ b/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash.bb
> @@ -11,6 +11,7 @@ SRC_URI += "file://0003-external-Fix-pflash-install
> -target.patch"
>  SRC_URI += "file://0004-external-Remove-m64-from-shared-CFLAGS-on-
> ARM.patch"
>  SRC_URI += "file://0005-external-Create-shared-rules.mk.patch"
>  SRC_URI += "file://0006-external-Add-dynamically-linked-
> pflash.patch"
> +SRC_URI += "file://0007-external-Fix-ARM-build-failure-with-
> parallel-make.patch"
>  
>  LIC_FILES_CHKSUM =
> "file://${S}/LICENCE;md5=3b83ef96387f14655fc854ddc3c6bd57"
>  
> diff --git a/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-
> make.patch b/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-
> make.patch
> new file mode 100644
> index 0000000..a9d219c
> --- /dev/null
> +++ b/meta-openbmc-machines/meta-openpower/common/recipes-
> bsp/pflash/pflash/0007-external-Fix-ARM-build-failure-with-parallel-
> make.patch
> @@ -0,0 +1,28 @@
> +From 55749b091414e0cf2691bd8f72c4cb48235b51c3 Mon Sep 17 00:00:00
> 2001
> +From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> +Date: Mon, 20 Jun 2016 20:31:07 -0400
> +Subject: [PATCH] external: Fix ARM build failure with parallel make
> +
> +Arch headers need to be linked in before compiling.
> +
> +Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
> +---
> + external/common/rules.mk | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/external/common/rules.mk b/external/common/rules.mk
> +index 5558cd3..bb12fd5 100644
> +--- a/external/common/rules.mk
> ++++ b/external/common/rules.mk
> +@@ -67,7 +67,7 @@ arch_clean:
> + 
> + $(ARCH_SRC): | common
> + 
> +-$(ARCH_OBJS): common-%.o: common/%.c
> ++$(ARCH_OBJS): common-%.o: common/%.c $(ARCH_LINKS)
> + 	$(Q_CC)$(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@
> + 
> + common-arch_flash.o: $(ARCH_OBJS)
> +-- 
> +1.8.3.1
> +

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

Re: [PATCH openbmc v6 04/18] image_types_uboot: Don't truncate when assembling flash

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> The generate_flash_image step was creating a file then using dd to
> fill in the pieces, but missed adding the conv=notrunc flag, so each
> step was shortening the file to its output.
> 
> Add the forgotten conversion flag to each dd command.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> index adb86e0..164d1ed 100644
> --- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> +++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> @@ -76,11 +76,11 @@ do_generate_flash() {
>         dst="${ddir}/${FLASH_IMAGE_NAME}"
>         rm -rf $dst
>         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}
> -       dd if=${ddir}/${rootfs} of=${dst} bs=1k seek=${FLASH_ROFS_OFFSET}
> -       dd if=${ddir}/${rwfs} of=${dst} bs=1k seek=${FLASH_RWFS_OFFSET}
> +       dd if=${ddir}/${uboot} of=${dst} conv=notrunc bs=1k seek=${FLASH_UBOOT_OFFSET}

Would be good to keep the option ordering consistent here (conv=notrunc
is after bs=1k on the remaining dd invocations). That's a minor issue
though and this is a good catch, so:

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> +       dd if=${ddir}/${kernel} of=${dst} bs=1k conv=notrunc seek=${FLASH_KERNEL_OFFSET}
> +       dd if=${ddir}/${uinitrd} of=${dst} bs=1k conv=notrunc seek=${FLASH_INITRD_OFFSET}
> +       dd if=${ddir}/${rootfs} of=${dst} bs=1k conv=notrunc seek=${FLASH_ROFS_OFFSET}
> +       dd if=${ddir}/${rwfs} of=${dst} bs=1k conv=notrunc seek=${FLASH_RWFS_OFFSET}
>         dstlink="${ddir}/${FLASH_IMAGE_LINK}"
>         rm -rf $dstlink
>         ln -sf ${FLASH_IMAGE_NAME} $dstlink

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

Re: [PATCH openbmc v6 05/18] initfs: Fix recipe, remove unnecessary class

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> Because the recipe was named with trailing part init instead
> of initfs, the package didn't match the package name requiring
> PROVIDES, RPROVIDES and other bitbake workarounds.  Fix the recipe
> name to match the package removing the overly verbose packaging.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  meta-phosphor/classes/obmc-phosphor-initfs.bbclass                    | 4 ----
>  .../{obmc-phosphor-init.bb => obmc-phosphor-initfs.bb}                | 1 -
>  2 files changed, 5 deletions(-)
>  delete mode 100644 meta-phosphor/classes/obmc-phosphor-initfs.bbclass
>  rename meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/{obmc-phosphor-init.bb => obmc-phosphor-initfs.bb} (96%)
> 
> diff --git a/meta-phosphor/classes/obmc-phosphor-initfs.bbclass b/meta-phosphor/classes/obmc-phosphor-initfs.bbclass
> deleted file mode 100644
> index de7923e..0000000
> --- a/meta-phosphor/classes/obmc-phosphor-initfs.bbclass
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -# Common code for recipes that implement Phosphor OpenBMC filesystem
> -
> -RPROVIDES_${PN} += "obmc-phosphor-initfs"
> -PROVIDES += "obmc-phosphor-initfs"
> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
> similarity index 96%
> rename from meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb
> rename to meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
> index 3fa88c9..f81e21d 100644
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-init.bb
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/obmc-phosphor-initfs.bb
> @@ -3,7 +3,6 @@ DESCRIPTION = "Phosphor OpenBMC filesytem mount reference implementation."
>  PR = "r1"
>  
>  inherit obmc-phosphor-license
> -inherit obmc-phosphor-initfs
>  
>  S = "${WORKDIR}"
>  SRC_URI += "file://obmc-init.sh"

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH openbmc v6 07/18] initfs: update: Don't exec sh or sulogin on error just exit 1

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> When update was written it was exec'd from the shutdown script
> and hence took over pid 1.  Since exiting in that environment was
> a panic situation, the script instead started a rescue shell with
> its output presumably on the console.
> 
> The calling convention was updated to be a simple invocation in
> commit dbacf104885c ("obmc-initfs: run update as a sub-script")
> but the error handling was not updated.  That error handling is
> now becoming a hinderance

hindrance

>  to use from additional environments so
> change it.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh      | 23 ++++++----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> 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 aa8fd89..f8e551c 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
> @@ -2,9 +2,6 @@
>  
>  echo update: "$@"
>  
> -export PS1=update-sh#\ 
> -# exec /bin/sh
> -
>  cd /
>  if ! test -r /proc/mounts || ! test -f /proc/mounts
>  then
> @@ -21,12 +18,12 @@ then
>  	mkdir -p /dev
>  	mount -t devtmpfs dev dev
>  fi
> -while grep mtd /proc/mounts
> -do
> +
> +if grep mtd /proc/mounts
> +then
>  	echo 1>&2 "Error: A mtd device is mounted."
> -	sulogin
> -	# exec /bin/sh
> -done
> +	exit 1
> +fi
>  
>  findmtd() {
>  	m=$(grep -xl "$1" /sys/class/mtd/*/name)
> @@ -130,7 +127,7 @@ do
>  	if test -z "$m"
>  	then
>  		echo 1>&2  "Unable to find mtd partiton for ${f##*/}."
> -		exec /bin/sh
> +		exit 1

Is there any value differentiating between the failure cases? I.e.
could we `exit 2` here instead?

It is just a thought and I am not overly fussed by it.


>  	fi
>  done
>  
> @@ -173,11 +170,3 @@ then
>  fi
>  
>  exit
> -
> -# NOT REACHED without edit
> -# NOT REACHED without edit
> -
> -echo "Flash completed.  Inspect, cleanup and reboot -f to continue."
> -
> -export PS1=update-sh#\ 
> -exec /bin/sh
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH openbmc v6 08/18] initfs: update: Do not cause an error if no files were saved

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> If there were no files in the read-write overlay file system that
> were in the persistent file list, then the save directory will
> not be created and therefore not exist.  Skip attempting copying
> the non-existent directory in that case to avoid the error message.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 f8e551c..f0ca989 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
> @@ -144,7 +144,7 @@ do
>  	flashcp -v $f /dev/$m && rm $f
>  done
>  
> -if test "x$toram" = xy
> +if test -d $save -a "x$toram" = xy
>  then
>  	mkdir -p $upper
>  	cp -rp $save/. $upper/

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH openbmc v6 09/18] initfs: update: Consistently add ERROR: and print to stderr

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm@us.ibm.com>
> 
> Make sure all error messages start with the tag ERROR.  Also redirect
> output to stderr (once case had the file descriptors backwards).

Maybe we should lift the calls out to a shell function, something like:

print_err() {
	echo 1>&2 ERROR: $@
}

> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh    | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> 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 f0ca989..4249161 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
> @@ -21,7 +21,7 @@ fi
>  
>  if grep mtd /proc/mounts
>  then
> -	echo 1>&2 "Error: A mtd device is mounted."
> +	echo 1>&2 "ERROR: A mtd device is mounted."
>  	exit 1
>  fi
>  
> @@ -65,6 +65,8 @@ toram=
>  whitelist=/run/initramfs/whitelist
>  image=/run/initramfs/image-
>  
> +E="ERROR:"

I'd prefer the function approach, but if we're going to do this
instead, lets at least do it at the top of the script so we can use the
same approach in the hunk above.

> +
>  while test "$1" != "${1#-}"
>  do
>  	case "$1" in
> @@ -90,7 +92,7 @@ do
>  		toram=y
>  		shift ;;
>  	*)
> -		echo 2>&1 "Unknown option $1"
> +		echo 1>&2 "$E Unknown option $1"
>  		exit 1 ;;
>  	esac
>  done
> @@ -126,7 +128,7 @@ do
>  	m=$(findmtd ${f#$image})
>  	if test -z "$m"
>  	then
> -		echo 1>&2  "Unable to find mtd partiton for ${f##*/}."
> +		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
>  		exit 1
>  	fi
>  done

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

Re: [PATCH openbmc v6 10/18] initfs: update: add option to perform checks but skip actual update

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> Provide a way to check if update is likely to succeed without
> attempting actual updates.
> 
> This will check the image names correspond to mtd partition names,
> and will be enhanced with additional checking for file size and
> individual partition mounts.
> 
> This is not called --dry-run because it will still do file save
> and restore if those are enabled.

Is save/restore useful if nothing's going to change - as in, why not
make it a true dry-run? Or are you concerned about adding more logic to
prevent the save/restore from happening?

Cheers,

Andrew

> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh      | 31 +++++++++++++---------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> 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 4249161..4442e5b 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
> @@ -57,6 +57,7 @@ upper=$rwdir/cow
>  save=/run/save/${upper##*/}
>  
>  mounted=
> +doflash=y
>  doclean=
>  dosave=y
>  dorestore=y
> @@ -88,6 +89,9 @@ do
>  	--restore-files)
>  		dorestore=y
>  		shift ;;
> +	--no-flash)
> +		doflash=
> +		shift ;;
>  	--copy-files)
>  		toram=y
>  		shift ;;
> @@ -133,18 +137,21 @@ do
>  	fi
>  done
>  
> -for f in $image*
> -do
> -	if test ! -s $f
> -	then
> -		echo "Skipping empty update of ${f#$image}."
> -		rm $f
> -		continue
> -	fi
> -	m=$(findmtd ${f#$image})
> -	echo "Updating ${f#$image}..."
> -	flashcp -v $f /dev/$m && rm $f
> -done
> +if test -n "$doflash"
> +then
> +	for f in $image*
> +	do
> +		if test ! -s $f
> +		then
> +			echo "Skipping empty update of ${f#$image}."
> +			rm $f
> +			continue
> +		fi
> +		m=$(findmtd ${f#$image})
> +		echo "Updating ${f#$image}..."
> +		flashcp -v $f /dev/$m && rm $f
> +	done
> +fi
>  
>  if test -d $save -a "x$toram" = xy
>  then

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

Re: [PATCH openbmc v6 11/18] initfs: update: check individual mtd partitions for mounts

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> Instead of checking for any mtd device, only check mtd devices
> that are to be updated.

What do you mean by "instead of"? We're not removing any code here, and
from a quick look we loop over the update images in existing checks.

>   Include child mtd devices when looking
> for mounted filesystems.
> 
> If this check is suppressed flash updates are suppressed.
> Suppressing the check may be useful to verify a update could
> succeed at shutdown.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh      | 47 +++++++++++++++++++---
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> 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 4442e5b..22c9dcc 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
> @@ -19,11 +19,34 @@ then
>  	mount -t devtmpfs dev dev
>  fi
>  
> -if grep mtd /proc/mounts
> -then
> -	echo 1>&2 "ERROR: A mtd device is mounted."
> -	exit 1
> -fi
> +# mtd number N with mtd name Name can be mounted via mtdN, or mtd:Name
> +# (with a mtd aware fs) or by /dev/mtdblockN (with a mtd or block fs).
> +mtdismounted() {

I keep reading this as mt-dismounted rather than mtd-is-mounted. Can we
add some underscores?

> +	m=${1##mtd}
> +	if grep -s "mtdblock$m " /proc/mounts || grep -s "mtd$m " /proc/mounts
> +	then
> +		return 0
> +	fi
> +	n=$(cat /sys/class/mtd/mtd$m/name)
> +	if test -n "$n" && grep -s "mtd:$n " /proc/mounts
> +	then
> +		return 0
> +	fi
> +	return 1
> +}
> +
> +# Detect child partitions when the whole flash is to be updated.
> +# Ignore mtdNro and mtdblockN names in the class subsystem directory.
> +childmtds() {
> +	for m in /sys/class/mtd/$1/mtd*
> +	do
> +		m=${m##*/}
> +		if test "${m%ro}" = "${m#mtdblock}"
> +		then
> +			echo $m
> +		fi
> +	done
> +}
>  
>  findmtd() {
>  	m=$(grep -xl "$1" /sys/class/mtd/*/name)
> @@ -62,10 +85,10 @@ doclean=
>  dosave=y
>  dorestore=y
>  toram=
> +checkmount=y
>  
>  whitelist=/run/initramfs/whitelist
>  image=/run/initramfs/image-
> -
>  E="ERROR:"
>  
>  while test "$1" != "${1#-}"
> @@ -92,6 +115,10 @@ do
>  	--no-flash)
>  		doflash=
>  		shift ;;
> +	--ignore-mount)
> +		checkmount=
> +		doflash=
> +		shift ;;
>  	--copy-files)
>  		toram=y
>  		shift ;;
> @@ -135,6 +162,14 @@ do
>  		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
>  		exit 1
>  	fi
> +	for s in $m $(childmtds $m)
> +	do
> +		if test -n "$checkmount" && mtdismounted $s

$checkmount is invariant with respect to $s, so is it worth hoisting
the check out of the loop? Though it would mean another indent...

Cheers,

Andrew

> +		then
> +			echo 1>&2 "$E Device $s is mounted, ${f##*/} is busy."
> +			exit 1
> +		fi
> +	done
>  done
>  
>  if test -n "$doflash"

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

Re: [PATCH openbmc v6 12/18] initfs: update: add check for image size vs mtd size

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> 
> The flashcp command will check the file size vs the partition size,
> so add a check when looking at images.
> 
> Use stat -L -c "%s" to get the file size, and compare it to the mtd
> size from the sysfs size attribute.
> 
> If this check is suppressed the enforcement is left to flashcp.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh           | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> 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 22c9dcc..06e0e74 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
> @@ -48,6 +48,14 @@ childmtds() {
>  	done
>  }
>  
> +toobig() {
> +	if test $(stat -L -c "%s" "$1") -gt $(cat /sys/class/mtd/"$2"/size)
> +	then
> +		return 0
> +	fi
> +	return 1
> +}
> +
>  findmtd() {
>  	m=$(grep -xl "$1" /sys/class/mtd/*/name)
>  	m=${m%/name}
> @@ -85,6 +93,7 @@ doclean=
>  dosave=y
>  dorestore=y
>  toram=
> +checksize=y
>  checkmount=y
>  
>  whitelist=/run/initramfs/whitelist
> @@ -115,6 +124,9 @@ do
>  	--no-flash)
>  		doflash=
>  		shift ;;
> +	--ignore-size)
> +		checksize=
> +		shift ;;
>  	--ignore-mount)
>  		checkmount=
>  		doflash=
> @@ -162,6 +174,11 @@ do
>  		echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
>  		exit 1
>  	fi
> +	if test -n "$checksize" && toobig "$f" "$m"
> +	then
> +		echo 1>&2 "$E Image ${f##*/} too big for $m."
> +		exit 1
> +	fi
>  	for s in $m $(childmtds $m)
>  	do
>  		if test -n "$checkmount" && mtdismounted $s

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH openbmc v6 13/18] initfs: update: add --help message

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm@us.ibm.com>
> 
> Now that update is intended for use at runtime and not just by
> the init and shutdown scripts, add a short help message listing
> the options so people don't have to understand how to read shell.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh        | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> 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 06e0e74..1dbf65f 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
> @@ -103,6 +103,24 @@ E="ERROR:"
>  while test "$1" != "${1#-}"
>  do
>  	case "$1" in
> +	--help)
> +		cat <
> +Usage: $0 [options] -- Write images in /run/initramfs to flash (/dev/mtd*)
> +    --help                    Show this message
> +    --no-flash	              Don't attempt to write images to flash

Looks like a tab snuck in and is messing up the alignment

> +    --ignore-size             Don't compare image size to mtd device size
> +    --ignore-mount            Don't check if destination is mounted

Maybe it is worth adding some words about --ignore-mount also setting
--no-flash?

> +    --save-files              Copy whitelisted files to save directory in RAM
> +    --no-save-files           Don't copy whitelisted files to save directory
> +    --copy-files              Copy files from save directory to rwfs mountpoint
> +    --restore-files           Restore files from save directory to rwfs layer
> +    --no-restore-files        Don't restore saved files from ram to rwfs layer
> +    --clean-saved-files       Delete saved whitelisted files from RAM
> +    --no-clean-saved-files    Retain saved whitelisted files in RAM

Hmm, I think we need more words about how copy and restore are
different - I've forgotten and the documentation isn't making it clear
to me.

Cheers,

Andrew

> +HERE

> +
> +	    exit 0 ;;
> +
>  	--no-clean-saved-files)
>  		doclean=
>  		shift ;;
> @@ -135,7 +153,7 @@ do
>  		toram=y
>  		shift ;;
>  	*)
> -		echo 1>&2 "$E Unknown option $1"
> +		echo 1>&2 "$E Unknown option $1.  Try $0 --help."
>  		exit 1 ;;
>  	esac
>  done

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

Re: [PATCH openbmc v6 16/18] initfs: update: handle no images to update

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm@us.ibm.com>
> 
> Notify the user if image wildcard expansion fails instead of printing
> message about failing to find partition to flash.
> 

This kind of problem is why I have reservations about our early/late
shell scripts. I wonder how many more of these issues are lurking...

> The update script errors with the message that it can't figure out what
> partition to flash for /run/initramfs/image-* if there are no images
> pending.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh               | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> 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 1dbf65f..7120a18 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
> @@ -99,6 +99,7 @@ checkmount=y
>  whitelist=/run/initramfs/whitelist
>  image=/run/initramfs/image-
>  E="ERROR:"
> +imglist=
>  
>  while test "$1" != "${1#-}"
>  do
> @@ -184,7 +185,15 @@ then
>  	fi
>  fi
>  
> -for f in $image*
> +imglist=$(echo $image*)

If there are no wildcard matches doesn't this expand to the string
"/run/initramfs/image-*"? Why are we using echo in a subshell?

> +if test "$imglist" = "$image*" -a ! -e "$imglist"

What's the idea behind the first condition ("imglist" = "$image*")?
Won't they always be the same?

> +then
> +	# shell didn't expand the wildcard, so no files exist
> +	echo "No images found to update."

Is this worthy of the ERROR: prefix?

> +	imglist=
> +fi
> +
> +for f in $imglist

If imglist is empty don't we run into the same issue with findmtd, just
that we have printed the warning above? Should we exit?

Cheers,

Andrew

>  do
>  	m=$(findmtd ${f#$image})
>  	if test -z "$m"
> @@ -209,7 +218,7 @@ done
>  
>  if test -n "$doflash"
>  then
> -	for f in $image*
> +	for f in $imglist
>  	do
>  		if test ! -s $f
>  		then

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

Re: [PATCH openbmc v6 17/18] initfs: update: Sanitze whitelist directory entries

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm@us.ibm.com>
> 
> Repeatedly strip trailing / and /. from whitelist entries and
> fail if an entry includes /../ or ends with /.. or doesn't start
> with a /.  Also use the entries quoted to avoid any glob.

Right. Ending this particular sentence with a '/' is a bit subtle. I
think you should quote your path bits: "/", "/.", "/../", "/.."; this
way the sentence would end '... or doesn't start with a "/".'.
> 
> It was noticed the save code was saving directories that ended in /
> into a subdirectory of the last component name.  This was traced
> the the code creating the directory just stripping the last /
> and then copying to the directory.
> 
> Choose to sanitize the entry where possible for ease of use verses
> a small performance penality.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh             | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> 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 7120a18..bfacd00 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
> @@ -170,13 +170,24 @@ then
>  
>  	while read f
>  	do
> -		if ! test -e $upper/$f
> +		# Entries shall start with /, no trailing /.. or embedded /../
> +		if test "/${f#/}" != "$f" -o "${f%/..}" != "${f#*/../}"

Can we do this with `readlink -f` or `realpath` instead? Do we have
them available in this environment? As in, test that the result before
and after realpath are the same. You can tell realpath to not expand
symlinks.

> +		then
> +			echo 1>&2 "WARNING: Skipping bad whitelist entry $f."
> +			continue
> +		fi
> +		if ! test -e "$upper/$f"
>  		then
>  			continue
>  		fi
>  		d="$save/$f"
> +		while test "${d%/}" != "${d%/.}"
> +		do
> +			d="${d%/.}"
> +			d="${d%/}"
> +		done
>  		mkdir -p "${d%/*}"
> -		cp -rp $upper/$f "${d%/*}/"
> +		cp -rp "$upper/$f" "${d%/*}/"

Again the while loop and other bits can hopefully be replaced by
`readlink -f` or `realpath`. The whole trailing-slash thing with `cp`
always throws me.

Andrew

>  	done < $whitelist
>  
>  	if test -n "$mounted"

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

Re: [PATCH openbmc v6 18/18] initfs: whitelist: Sanitize obmc events whitelist entry

[Andrew Jeffery]

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

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm@us.ibm.com>
> 
> Remove the trailing / so that the update script doesn't have to loop.
> 
> It was determined the trailing slash confused the code that makes
> the save subdirectory.  The update script was fixed but this removes
> the extra work needed.
> 
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> ---
>  .../common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist        | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
> index 603cec7..c35a552 100644
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/whitelist
> @@ -6,4 +6,4 @@
>  /etc/group
>  /etc/shadow
>  /etc/gshadow
> -/var/lib/obmc/events/
> +/var/lib/obmc/events

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

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

Re: [PATCH openbmc v6 11/18] initfs: update: check individual mtd partitions for mounts

[Milton Miller II]

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

Re: [PATCH openbmc v6 10/18] initfs: update: add option to perform checks but skip actual update

[Milton Miller II]

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

Re: [PATCH openbmc v6 16/18] initfs: update: handle no images to update

[Milton Miller II]

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

Re: [PATCH openbmc v6 17/18] initfs: update: Sanitze whitelist directory entries

[Milton Miller II]

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

Re: [PATCH openbmc v6 11/18] initfs: update: check individual mtd partitions for mounts

[Andrew Jeffery]

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

On Sun, 2016-06-26 at 00:12 +0000, Milton Miller II wrote:
> 
> On 06/23/2016 around 12:35AM in some timezone, Andrew Jeffery wrote:
> >On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> >> From: "Milton D. Miller II" <miltonm@us.ibm.com>
> >> 
> >> Instead of checking for any mtd device, only check mtd devices
> >> that are to be updated.
> >
> >What do you mean by "instead of"? We're not removing any code here,
> >and
> >from a quick look we loop over the update images in existing checks.
> >
> 
> The grep for mtd anywhere in proc at the top of the script (that you
> had me move ERROR up to handle) is replaced with ...
> 
> >>   Include child mtd devices when looking
> >> for mounted filesystems.
> >> 
> >> If this check is suppressed flash updates are suppressed.
> >> Suppressing the check may be useful to verify a update could
> >> succeed at shutdown.
> >> 
> >> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> >> ---
> >>  .../obmc-phosphor-initfs/files/obmc-update.sh      | 47
> >+++++++++++++++++++---
> >>  1 file changed, 41 insertions(+), 6 deletions(-)
> >> 
> >> diff --git
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> index 4442e5b..22c9dcc 100755
> >> ---
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> +++
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> @@ -19,11 +19,34 @@ then
> >>   mount -t devtmpfs dev dev
> >>  fi
> >>  
> >> -if grep mtd /proc/mounts
> >> -then
> >> - echo 1>&2 "ERROR: A mtd device is mounted."
> >> - exit 1
> >> -fi
> 
> Right here is the "instead of" code that is being removed.

Ah, thanks, that was an oversight.

> 
> 
> >> +# mtd number N with mtd name Name can be mounted via mtdN, or
> >mtd:Name
> >> +# (with a mtd aware fs) or by /dev/mtdblockN (with a mtd or block
> >fs).
> >> +mtdismounted() {
> >
> >I keep reading this as mt-dismounted rather than mtd-is-mounted. Can
> >we
> >add some underscores?
> 
> If I do that then I have to add them to all the functions and then 
> my lines go over 80 characters and then ...
> 
> I considered ismtdmounted but that gets back to which is true like
> checkmtdmounted would, so I decided to go with mountedmtd.
> 
> >>   shift ;;
> >> @@ -135,6 +162,14 @@ do
> >>   echo 1>&2 "$E Unable to find mtd partiton for ${f##*/}."
> >>   exit 1
> >>   fi
> >> + for s in $m $(childmtds $m)
> >> + do
> >> + if test -n "$checkmount" && mtdismounted $s
> >
> >$checkmount is invariant with respect to $s, so is it worth hoisting
> >the check out of the loop? Though it would mean another indent...
> >
> 
> Since this also suppresses the call to childmtds I decided to 
> implement this, changing the error to be Device $s for ${f} is in use.
> 
> 
> Unfortunately the file system can be unmounted to where it 
> no longer appears in /proc/mounts but is stlil mounted and in 
> use by the overlay.  I don't know how to fix this. 

Right, that's pretty ugly. Though, what context are we running in? I
thought we were executing in initrd, and if so, shouldn't it be
possible to avoid the overlay? That query is based on my hazy
recollection from the initial patches for the update support, so take
it with a grain of salt.

Cheers,

Andrew

>  Some long 
> ideas are add sysfs status to mtd and add a process holding 
> the directory open with the systemd "don't kill leave to shutdown 
> script" process name (if that can be done from shell).  Or copying 
> the .proc/mounts to /run when mounting overlay and checking 
> that too.   This is to avoid root writing a new image while the 
> file system writes to the flash based on the old image, and
> several of these options would not be detect mounting a device 
> node with a nonstandard name.
> 
> milton
> 

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

Re: [PATCH openbmc v6 16/18] initfs: update: handle no images to update

[Andrew Jeffery]

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

On Sun, 2016-06-26 at 00:12 +0000, Milton Miller II wrote:
> 
> On 06/23/2016 01:11AM Andrew Jeffery wrote:
> >
> >On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> >> From: Milton Miller <miltonm@us.ibm.com>
> >> 
> >> Notify the user if image wildcard expansion fails instead of
> >printing
> >> message about failing to find partition to flash.
> >> 
> >
> >This kind of problem is why I have reservations about our early/late
> >shell scripts. I wonder how many more of these issues are lurking...
> >
> >> The update script errors with the message that it can't figure out
> >what
> >> partition to flash for /run/initramfs/image-* if there are no
> >images
> >> pending.
> >> 
> >> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> >> ---
> >>  .../obmc-phosphor-initfs/files/obmc-update.sh               | 13
> >+++++++++++--
> >>  1 file changed, 11 insertions(+), 2 deletions(-)
> >> 
> >> diff --git
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> index 1dbf65f..7120a18 100755
> >> ---
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> +++
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> @@ -99,6 +99,7 @@ checkmount=y
> >>  whitelist=/run/initramfs/whitelist
> >>  image=/run/initramfs/image-
> >>  E="ERROR:"
> >> +imglist=
> >>  
> >>  while test "$1" != "${1#-}"
> >>  do
> >> @@ -184,7 +185,15 @@ then
> >>   fi
> >>  fi
> >>  
> >> -for f in $image*
> >> +imglist=$(echo $image*)
> >
> >If there are no wildcard matches doesn't this expand to the string
> >"/run/initramfs/image-*"? Why are we using echo in a subshell?
> >
> 
> Because that causes path expansion to be performed before
> assigning the output to the parameter.
> 
> >> +if test "$imglist" = "$image*" -a ! -e "$imglist"
> >
> >What's the idea behind the first condition ("imglist" = "$image*")?
> >Won't they always be the same?
> >
> 
> No, because parameter splitting and path expansion are
> suppressed by the double quotes here but path expansion
> occurred in the echo above.

Okay, I understand, but it still feels subtle. Can you add some
comments?

Cheers,

Andrew

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

Re: [PATCH openbmc v6 17/18] initfs: update: Sanitze whitelist directory entries

[Andrew Jeffery]

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

On Sun, 2016-06-26 at 00:12 +0000, Milton Miller II wrote:
> 
> On 06/23/2016 about 01:26AM in some time zone, Andrew Jeffery wrote:
> >On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> >> From: Milton Miller <miltonm@us.ibm.com>
> >> 
> >> Repeatedly strip trailing / and /. from whitelist entries and
> >> fail if an entry includes /../ or ends with /.. or doesn't start
> >> with a /.  Also use the entries quoted to avoid any glob.
> >
> >Right. Ending this particular sentence with a '/' is a bit subtle. I
> >think you should quote your path bits: "/", "/.", "/../", "/.."; this
> >way the sentence would end '... or doesn't start with a "/".'.
> 
> Ok done.
> 
> >> 
> >> It was noticed the save code was saving directories that ended in /
> >> into a subdirectory of the last component name.  This was traced
> >> the the code creating the directory just stripping the last /
> >> and then copying to the directory.
> >> 
> >> Choose to sanitize the entry where possible for ease of use verses
> >> a small performance penality.
> >> 
> >> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> >> ---
> >>  .../obmc-phosphor-initfs/files/obmc-update.sh             | 15
> >+++++++++++++--
> >>  1 file changed, 13 insertions(+), 2 deletions(-)
> >> 
> >> diff --git
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> index 7120a18..bfacd00 100755
> >> ---
> >a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> +++
> >b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/ob
> >mc-update.sh
> >> @@ -170,13 +170,24 @@ then
> >>  
> >>   while read f
> >>   do
> >> - if ! test -e $upper/$f
> >> + # Entries shall start with /, no trailing /.. or embedded /../
> >> + if test "/${f#/}" != "$f" -o "${f%/..}" != "${f#*/../}"
> >
> >Can we do this with `readlink -f` or `realpath` instead? Do we have
> >them available in this environment? As in, test that the result
> >before
> >and after realpath are the same. You can tell realpath to not expand
> >symlinks.
> >
> 
> ENODOC
> 
> There does not appear to be a man page available for realpath(1)
> in either Debian wheezy or RHEL 6.4.

Okay, as suggested in your link[1] it looks like debian ship their own
realpath and RHEL 6.4 is probably too old for GNU realpath. GNU
realpath has:

       -s, --strip, --no-symlinks
              don't expand symlinks

So:

    $ realpath -s /etc/../etc/alternatives/cc
    /etc/alternatives/cc
    $ ls -l !$
    ls -l /etc/../etc/alternatives/cc
    lrwxrwxrwx 1 root root 12 Dec  7  2015 /etc/../etc/alternatives/cc -> /usr/bin/gcc

>   The man page for readlink
> is talking about expanding symlinks with -f saying it follows symlinks
> recursively, no where does it talk about cleaning up the path, nor
> returning the path of something not a symlink.

Yes, readlink -f was probably a poor suggestion if we don't want to
resolve symlinks.

> 
> A qucik search turned up [1] which shows some of the history.  
> Also note it says busybox (which is the version we are using) 
> does not take options in its realpath command.

Right, this is the killer bit, that busybox realpath can't do -s.

> 
> Also this is already peaking under the covers at the read-write 
> COW directory which is a sparse copy and not looking though 
> the combined file system.  And this file subtree is not mounted 
> at / meaning any attempt to follow an absolute symlink will likely 
> find the wrong content and not necessarily fail.
> 
> So I am rejecting this suggestion.

Given the above, I'm okay with that.

> 
> [1] http://unix.stackexchange.com/questions/136494/whats-the-difference-between-realpath-and-readlink-f
> 
> >> + then
> >> + echo 1>&2 "WARNING: Skipping bad whitelist entry $f."
> >> + continue
> >> + fi
> >> + if ! test -e "$upper/$f"
> >>   then
> >>   continue
> >>   fi
> >>   d="$save/$f"
> >> + while test "${d%/}" != "${d%/.}"
> >> + do
> >> + d="${d%/.}"
> >> + d="${d%/}"
> >> + done
> >>   mkdir -p "${d%/*}"
> >> - cp -rp $upper/$f "${d%/*}/"
> >> + cp -rp "$upper/$f" "${d%/*}/"
> >
> >Again the while loop and other bits can hopefully be replaced by
> >`readlink -f` or `realpath`. The whole trailing-slash thing with `cp`
> >always throws me.
> 
> While the test here for $d is absolute I'd rather not do it.  And 
> right now this has at least two / (one in the string and one leading 
> the $f) meaning a simple equality check of $d and $(realpath $d) 
> will always fail.

If we know everything in the whitelist is absolute could we take a
different approach to the path construction?

>   I could remove the slash but that implies the 
> first check,

Isn't it instead that because of the first check we now know that the
whitelist path is absolute, so we can validly removing the leading '/'
to construct the necessary path?

>  and again we are copying from a sparse directory and 
> want to copy a symlink not resolve it. 

Sure; as above, realpath -s would have avoided the resolution but as
you point out we can't use it (busybox).

>   I'd love to use rsync here 
> but that is not currently available.

right.

> 
> If you object to the while loop I can reject the path if the conditions 
> on $f with similar substitutions are not equal (reordering the fix
> commit). I don't anticipate the loop executing more than once
> unless someone is testing corner cases and thought the convenience 
> of making the entry sane was better than the debug why the entry
> didn't work.
> 

What you've got sounds preferable. The path string hackery is
unfortunate but sounds unavoidable.

Andrew

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