Re: [PATCH] isar: Clean mount point on bitbake exit

[Alexander Smirnov <asmirnov@ilbers.de>]

On 02/09/2018 03:41 PM, Jan Kiszka wrote:
> On 2018-02-09 13:40, Henning Schild wrote:
>> Am Fri, 9 Feb 2018 13:35:15 +0100
>> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>>
>>> On 2018-02-09 13:33, [ext] Henning Schild wrote:
>>>> Hi,
>>>>
>>>> this patch is causing problems when building in a docker container,
>>>> because sysfs can only be mounted ro. (Subject: current next bash in
>>>> buildchroot problem)
>>>> Now we could discuss whether we should relax the security of our
>>>> containers even more, or whether Isar should care about that
>>>> use-case.
>>>>
>>>> But this patch actually does several things at a time, it changes >>>> the way we mount and adds three new mounts. I would suggest to

Actually not. It adds the only one new mount for sysfs. /proc was 
mounted inside do_build, /dev was mounted inside configscript.sh, so 
this is a kind of consolidation of these calls in one place.

I have no case for sysfs, so probably we could drop it for now. Please 
let me know ASAP because I'm going to release v0.4.

>>>> split it up so we can discuss the issues with dev and sys while
>>>> already merging the rest.

There is no official Docker support in Isar, so until there will be a 
document which specifies the container configuration, it really would be 
inefficient to block contributions. We can't support everything everywhere.

>>>
>>> I think (didn't check if there was an update of next this morning) it
>>> works for me - in Docker. How are you starting the container?
>>
>> docker run -e USER_ID=$(id -u) --rm -t -i --cap-add=SYS_ADMIN
>> --cap-add=MKNOD --device $(/sbin/losetup -f) -e ... proxy stuff ...
>>

Do you have instructions how to build Isar in container, so at least I 
could be able to reproduce the issue?

Alex


> Try adding --privileged - that's needed for binfmt anyway.
> 
> Jan
> 
>> inside my sysfs is ro, a bind-mount of sysfs is ro and a "mount -t
>> sysfs ..." will be ro. Maybe i could add a "-o rw" to the mount but for
>> now i just reverted the two patches that deal with mounting.
>>
>> Might also be a difference in our host systems.
>>
>> Henning
>>
>>> Jan
>>>
>>>>
>>>> Henning
>>>>
>>>> Am Tue, 6 Feb 2018 22:55:16 +0300
>>>> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
>>>>    
>>>>> 8<--
>>>>>
>>>>> That's it! Branch 'asmirnov/devel', please test and enjoy :-)
>>>>>
>>>>> 8<--
>>>>>
>>>>> Now each multiconfig has registered handler for BuildCompleted
>>>>> event (see class 'isar-event.bbclass'). Moreover, the
>>>>> '/proc/mounts' file contains all the active mounts. In addition,
>>>>> from event handler we could derive all the variables like
>>>>> ${TMPDIR}, ${DISTRO} etc. So it's possible to find all the active
>>>>> mounts for current multiconfig and clean them.
>>>>>
>>>>> NOTE: if build is interrupted by double ^C, some mount points could
>>>>> stay uncleaned. This is caused by remaining processes started by
>>>>> bitbake, for example:
>>>>>   - 'chroot build.sh ...'
>>>>>   - 'multistrap ...'
>>>>>
>>>>> So please be careful when interrupting build.
>>>>>
>>>>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>>>>> ---
>>>>>   meta-isar/recipes-core/images/isar-image-base.bb   | 11 ++++------
>>>>>   meta/classes/dpkg-base.bbclass                     | 12
>>>>> ++++------- meta/classes/isar-events.bbclass                   | 15
>>>>> +++++++++++--- meta/recipes-devtools/buildchroot/buildchroot.bb   |
>>>>> 24
>>>>> +++++++++------------- .../buildchroot/files/configscript.sh
>>>>> |  4 ---- .../buildchroot/files/download_dev-random          | 13
>>>>> ------------ 6 files changed, 30 insertions(+), 49 deletions(-)
>>>>> delete mode 100644
>>>>> meta/recipes-devtools/buildchroot/files/download_dev-random
>>>>>
>>>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb
>>>>> b/meta-isar/recipes-core/images/isar-image-base.bb index
>>>>> e359ac3..8ddbabb 100644 ---
>>>>> a/meta-isar/recipes-core/images/isar-image-base.bb +++
>>>>> b/meta-isar/recipes-core/images/isar-image-base.bb @@ -55,14 +55,10
>>>>> @@ do_rootfs() { -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>>>>>              "${WORKDIR}/multistrap.conf.in" >
>>>>> "${WORKDIR}/multistrap.conf"
>>>>> +    # Do not use bitbake flag [dirs] here because this folder
>>>>> should have
>>>>> +    # specific ownership.
>>>>>       [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m
>>>>> 555 ${IMAGE_ROOTFS}/proc sudo mount -t proc none
>>>>> ${IMAGE_ROOTFS}/proc
>>>>> -    _do_rootfs_cleanup() {
>>>>> -        ret=$?
>>>>> -        sudo umount ${IMAGE_ROOTFS}/proc 2>/dev/null || true
>>>>> -        (exit $ret) || bb_exit_handler
>>>>> -    }
>>>>> -    trap '_do_rootfs_cleanup' EXIT
>>>>>   
>>>>>       # Create root filesystem. We must use sudo -E here to preserve
>>>>> the environment # because of proxy settings
>>>>> @@ -72,5 +68,6 @@ do_rootfs() {
>>>>>       sudo chroot ${IMAGE_ROOTFS} /${DISTRO_CONFIG_SCRIPT}
>>>>> ${MACHINE_SERIAL} ${BAUDRATE_TTY} \ ${ROOTFS_DEV}
>>>>>       sudo rm "${IMAGE_ROOTFS}/${DISTRO_CONFIG_SCRIPT}"
>>>>> -    _do_rootfs_cleanup
>>>>> +
>>>>> +    sudo umount ${IMAGE_ROOTFS}/proc 2>/dev/null || true
>>>>>   }
>>>>> diff --git a/meta/classes/dpkg-base.bbclass
>>>>> b/meta/classes/dpkg-base.bbclass index 5d5a924..a34c21f 100644
>>>>> --- a/meta/classes/dpkg-base.bbclass
>>>>> +++ b/meta/classes/dpkg-base.bbclass
>>>>> @@ -20,15 +20,11 @@ dpkg_runbuild() {
>>>>>   do_build() {
>>>>>       mkdir -p ${BUILDROOT}
>>>>>       sudo mount --bind ${WORKDIR} ${BUILDROOT}
>>>>> -    _do_build_cleanup() {
>>>>> -        ret=$?
>>>>> -        sudo umount ${BUILDROOT} 2>/dev/null || true
>>>>> -        sudo rmdir ${BUILDROOT} 2>/dev/null || true
>>>>> -        (exit $ret) || bb_exit_handler
>>>>> -    }
>>>>> -    trap '_do_build_cleanup' EXIT
>>>>> +
>>>>>       dpkg_runbuild
>>>>> -    _do_build_cleanup
>>>>> +
>>>>> +    sudo umount ${BUILDROOT} 2>/dev/null || true
>>>>> +    sudo rmdir ${BUILDROOT} 2>/dev/null || true
>>>>>   }
>>>>>   
>>>>>   # Install package to Isar-apt
>>>>> diff --git a/meta/classes/isar-events.bbclass
>>>>> b/meta/classes/isar-events.bbclass index 55fc106..ae0f791 100644
>>>>> --- a/meta/classes/isar-events.bbclass
>>>>> +++ b/meta/classes/isar-events.bbclass
>>>>> @@ -11,10 +11,19 @@ python isar_handler () {
>>>>>       devnull = open(os.devnull, 'w')
>>>>>   
>>>>>       if isinstance(e, bb.event.BuildCompleted):
>>>>> -        bchroot = d.getVar('BUILDCHROOT_DIR', True)
>>>>> +        tmpdir = d.getVar('TMPDIR', True)
>>>>> +        distro = d.getVar('DISTRO', True)
>>>>> +        arch = d.getVar('DISTRO_ARCH', True)
>>>>>   
>>>>> -        # Clean up buildchroot
>>>>> -        subprocess.call('/usr/bin/sudo /bin/umount ' + bchroot +
>>>>> '/isar-apt || /bin/true', stdout=devnull, stderr=devnull,
>>>>> shell=True)
>>>>> +        w = tmpdir + '/work/' + distro + '-' + arch
>>>>> +
>>>>> +        # '/proc/mounts' contains all the active mounts, so
>>>>> knowing 'w' we
>>>>> +        # could get the list of mounts for the specific
>>>>> multiconfig and
>>>>> +        # clean them.
>>>>> +        with open('/proc/mounts', 'rU') as f:
>>>>> +            for line in f:
>>>>> +                if w in line:
>>>>> +                    subprocess.call('sudo umount -f ' +
>>>>> line.split()[1], stdout=devnull, stderr=devnull, shell=True)
>>>>>       devnull.close()
>>>>>   }
>>>>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb
>>>>> b/meta/recipes-devtools/buildchroot/buildchroot.bb index
>>>>> 304c67e..df9df19 100644 ---
>>>>> a/meta/recipes-devtools/buildchroot/buildchroot.bb +++
>>>>> b/meta/recipes-devtools/buildchroot/buildchroot.bb @@ -12,7 +12,6
>>>>> @@ FILESPATH =.
>>>>> "${LAYERDIR_core}/recipes-devtools/buildchroot/files:" SRC_URI =
>>>>> "file://multistrap.conf.in \ file://configscript.sh \
>>>>> file://setup.sh \
>>>>> -           file://download_dev-random \
>>>>>              file://build.sh"
>>>>>   PV = "1.0"
>>>>>   
>>>>> @@ -32,8 +31,10 @@ BUILDCHROOT_PREINSTALL ?= "gcc \
>>>>>   WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>>>>   
>>>>>   do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>>>> -do_build[dirs] = "${WORKDIR}/hooks_multistrap \
>>>>> -                  ${BUILDCHROOT_DIR}/isar-apt"
>>>>> +do_build[dirs] = "${BUILDCHROOT_DIR}/isar-apt \
>>>>> +                  ${BUILDCHROOT_DIR}/dev \
>>>>> +                  ${BUILDCHROOT_DIR}/proc \
>>>>> +                  ${BUILDCHROOT_DIR}/sys"
>>>>>   do_build[depends] = "isar-apt:do_cache_config"
>>>>>   
>>>>>   do_build() {
>>>>> @@ -41,7 +42,6 @@ do_build() {
>>>>>   
>>>>>       chmod +x "${WORKDIR}/setup.sh"
>>>>>       chmod +x "${WORKDIR}/configscript.sh"
>>>>> -    install -m 755 "${WORKDIR}/download_dev-random"
>>>>> "${WORKDIR}/hooks_multistrap/"
>>>>>       # Multistrap accepts only relative path in configuration
>>>>> files, so get it: cd ${TOPDIR}
>>>>> @@ -60,15 +60,6 @@ do_build() {
>>>>>           -e
>>>>> 's|##DIR_HOOKS##|./'"$WORKDIR_REL"'/hooks_multistrap|g' \
>>>>> "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>>>>> -    [ ! -d ${BUILDCHROOT_DIR}/proc ] && install -d -m 555
>>>>> ${BUILDCHROOT_DIR}/proc
>>>>> -    sudo mount -t proc none ${BUILDCHROOT_DIR}/proc
>>>>> -    _do_build_cleanup() {
>>>>> -        ret=$?
>>>>> -        sudo umount ${BUILDCHROOT_DIR}/proc 2>/dev/null || true
>>>>> -        (exit $ret) || bb_exit_handler
>>>>> -    }
>>>>> -    trap '_do_build_cleanup' EXIT
>>>>> -
>>>>>       do_setup_mounts
>>>>>   
>>>>>       # Create root filesystem
>>>>> @@ -79,7 +70,6 @@ do_build() {
>>>>>   
>>>>>       # Configure root filesystem
>>>>>       sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
>>>>> -    _do_build_cleanup
>>>>>   
>>>>>       do_cleanup_mounts
>>>>>   }
>>>>> @@ -96,10 +86,16 @@ do_setup_mounts[stamp-extra-info] =
>>>>> "${DISTRO}-${DISTRO_ARCH}"
>>>>>   do_setup_mounts() {
>>>>>       sudo mount --bind ${DEPLOY_DIR_APT}/${DISTRO}
>>>>> ${BUILDCHROOT_DIR}/isar-apt
>>>>> +    sudo mount --bind /dev ${BUILDCHROOT_DIR}/dev
>>>>> +    sudo mount -t proc none ${BUILDCHROOT_DIR}/proc
>>>>> +    sudo mount -t sysfs none ${BUILDCHROOT_DIR}/sys
>>>>>   }
>>>>>   
>>>>>   addtask setup_mounts after do_build
>>>>>   
>>>>>   do_cleanup_mounts() {
>>>>>       sudo umount ${BUILDCHROOT_DIR}/isar-apt 2>/dev/null || true
>>>>> +    sudo umount ${BUILDCHROOT_DIR}/dev 2>/dev/null || true
>>>>> +    sudo umount ${BUILDCHROOT_DIR}/proc 2>/dev/null || true
>>>>> +    sudo umount ${BUILDCHROOT_DIR}/sys 2>/dev/null || true
>>>>>   }
>>>>> diff --git
>>>>> a/meta/recipes-devtools/buildchroot/files/configscript.sh
>>>>> b/meta/recipes-devtools/buildchroot/files/configscript.sh index
>>>>> 9813c9a..524e50c 100644 ---
>>>>> a/meta/recipes-devtools/buildchroot/files/configscript.sh +++
>>>>> b/meta/recipes-devtools/buildchroot/files/configscript.sh @@
>>>>> -39,10 +39,6 @@ export LC_ALL=C LANGUAGE=C LANG=C #run pre
>>>>> installation script /var/lib/dpkg/info/dash.preinst install
>>>>> -# apt-get http method, gpg require /dev/null
>>>>> -mount -t devtmpfs -o mode=0755,nosuid devtmpfs /dev
>>>>> -
>>>>>   #configuring packages
>>>>>   dpkg --configure -a
>>>>>   apt-get update
>>>>> -umount /dev
>>>>> diff --git
>>>>> a/meta/recipes-devtools/buildchroot/files/download_dev-random
>>>>> b/meta/recipes-devtools/buildchroot/files/download_dev-random
>>>>> deleted file mode 100644 index 5b5b96b..0000000 ---
>>>>> a/meta/recipes-devtools/buildchroot/files/download_dev-random
>>>>> +++ /dev/null @@ -1,13 +0,0 @@
>>>>> -#!/bin/sh
>>>>> -
>>>>> -set -e
>>>>> -
>>>>> -readonly ROOTFS="$1"
>>>>> -
>>>>> -mknod "${ROOTFS}/dev/random" c 1 8
>>>>> -chmod 640 "${ROOTFS}/dev/random"
>>>>> -chown 0:0 "${ROOTFS}/dev/random"
>>>>> -
>>>>> -mknod "${ROOTFS}/dev/urandom" c 1 9
>>>>> -chmod 640 "${ROOTFS}/dev/urandom"
>>>>> -chown 0:0 "${ROOTFS}/dev/urandom"
>>>>    
>>>
>>
> 

-- 
With best regards,
Alexander Smirnov

ilbers GmbH
Baierbrunner Str. 28c
D-81379 Munich
+49 (89) 122 67 24-0
http://ilbers.de/
Commercial register Munich, HRB 214197
General manager: Baurzhan Ismagulov