[PATCH openbmc 1/7] obmc-initfs: minor updates

[Andrew Jeffery]

On Sat, 2016-02-06 at 18:00 -0600, OpenBMC Patches wrote:
> From: Milton Miller <miltonm at us.ibm.com>
> 
> In shutdown, Like init and update, cd to / to be clear the
> expected environment.  Although shorter names are not used, it
> prevents problems with unmounting filesystems, even if this is
> the normal state for a call of this script by systemd.  Also
> make a few paths absolute and don't follow symlinks in ln.
> 
> In init, check the new init is an executable file with non-zero
> size in addition to the shell being executable with its shared
> libraries.

>From a process point of view, these paragraphs appear to be separate
concerns and so my preference (in the future) is they be separate
patches. That way we avoid confusion around subtle interactions between
the changes. If the changes need to happen together then that should be
stated, along with an explanation as to why.

> 
> Signed-off-by: Milton Miller <miltonm at us.ibm.com>
> ---
>  .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh       |
> 4 ++--
>  .../recipes-phosphor/obmc-phosphor-initfs/files/obmc-shutdown.sh   |
> 7 ++++---
>  2 files changed, 6 insertions(+), 5 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 0dc4c35..f0d8522 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
> @@ -69,9 +69,9 @@ mkdir -p $work
>  
>  mount -t overlay -o lowerdir=$rodir,upperdir=$upper,workdir=$work
> cow /root
>  
> -if ! chroot /root /bin/sh -c exit
> +if ! chroot /root /bin/sh -c "test -x /sbin/init -a -s /sbin/init"
>  then
> -	echo 'chroot test failed; invoking emergency shell.'
> +	echo "Change Root test failed!  Invoking emergency shell."

Maybe it would be nice to tell the user exactly what test failed? Given
it's an emergency shell I think it would be nice to have as much info
as possible about what went wrong. Maybe something like "Will not
chroot to /root: /sbin/init is not functional in /root. Invoking
emergency shell".

Even here 'not functional' could be seen as vague, though that's around
where my line in the sand exists. Thoughts?

>  	PS1=rescue#\  sulogin
>  fi
>  
> 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 cc076fd..c550e06 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
> @@ -5,10 +5,11 @@ echo shutdown: "$@"
>  export PS1=shutdown-sh#\ 
>  # exec bin/sh
>  
> +cd /
>  if [ ! -e /proc/mounts ]
>  then
>  	mkdir -p /proc
> -	mount  proc proc -tproc
> +	mount  proc /proc -tproc
>  	umount_proc=1
>  else
>  	umount_proc=
> @@ -27,10 +28,10 @@ set +x
>  if test -s /run/fw_env -a -c /run/mtd:u-boot-env -a ! -e /image-u
> -boot-env &&
>  	! cmp /run/mtd:u-boot-env /run/fw_env
>  then
> -	ln -s /run/fw_env /image-u-boot-env
> +	ln -sn /run/fw_env /image-u-boot-env
>  fi
>  
> -if test -x /update && ls image-* > /dev/null 2>&1
> +if test -x /update && ls /image-* > /dev/null 2>&1
>  then
>  	exec /update ${1+"$@"}
>  fi