public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
From: "'Florian Bezdeka' via isar-users" <isar-users@googlegroups.com>
To: Anton Mikanovich <amikan@ilbers.de>, isar-users@googlegroups.com
Cc: Ilia Skochilov <iskochilov@ilbers.de>,
	Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [PATCH] meta: Drop lazy and recursive unmounts
Date: Wed, 28 Aug 2024 15:05:15 +0200	[thread overview]
Message-ID: <dccfd6b9e590e3429f74a990648e27c4d5e25ff8.camel@siemens.com> (raw)
In-Reply-To: <20240619104126.105252-1-amikan@ilbers.de>

Hi all,

I'm not 100% sure, but it seems I'm facing some problems with this
patch. I know about the situation that we sporadically saw some
mounting issues and that patch tries to expose such issues.

At least my issue seems to reproduce to 100%...

See below... 

On Wed, 2024-06-19 at 13:41 +0300, Anton Mikanovich wrote:
> From: Ilia Skochilov <iskochilov@ilbers.de>
> 
> Cleanup lazy and recursive unmounting because they just mask other
> issues caused by wrong mounting.
> Also remove umount || true usages for the same reason.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> Signed-off-by: Ilia Skochilov <iskochilov@ilbers.de>
> ---
>  meta/classes/deb-dl-dir.bbclass               |  4 ++--
>  meta/classes/image.bbclass                    | 14 +++++++++-----
>  meta/classes/isar-events.bbclass              |  2 +-
>  meta/classes/rootfs.bbclass                   | 10 +++++-----
>  meta/classes/sbuild.bbclass                   | 12 ++++++------
>  meta/classes/sdk.bbclass                      | 13 ++++++++++---
>  .../isar-bootstrap/isar-bootstrap.inc         | 19 ++++++++++---------
>  .../sdk-files/files/README.sdk                |  2 +-
>  scripts/mount_chroot.sh                       |  2 +-
>  9 files changed, 45 insertions(+), 33 deletions(-)
> 
> diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
> index d36b7190..8e0243fe 100644
> --- a/meta/classes/deb-dl-dir.bbclass
> +++ b/meta/classes/deb-dl-dir.bbclass
> @@ -27,7 +27,7 @@ debsrc_do_mounts() {
>      set -e
>      mkdir -p "${1}/deb-src"
>      mountpoint -q "${1}/deb-src" || \
> -    mount --bind "${DEBSRCDIR}" "${1}/deb-src"
> +    mount -o bind,private "${DEBSRCDIR}" "${1}/deb-src"
>  EOSUDO
>  }
>  
> @@ -36,7 +36,7 @@ debsrc_undo_mounts() {
>      set -e
>      mkdir -p "${1}/deb-src"
>      mountpoint -q "${1}/deb-src" && \
> -    umount -l "${1}/deb-src"
> +    umount "${1}/deb-src"
>      rm -rf "${1}/deb-src"
>  EOSUDO
>  }
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 4f774bbc..0a80273f 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -407,19 +407,23 @@ do_rootfs_finalize() {
>          fi
>  
>          mountpoint -q '${ROOTFSDIR}/isar-apt' && \
> -            umount -l ${ROOTFSDIR}/isar-apt && \
> +            umount '${ROOTFSDIR}/isar-apt' && \
>              rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
>  
>          mountpoint -q '${ROOTFSDIR}/base-apt' && \
> -            umount -l ${ROOTFSDIR}/base-apt && \
> +            umount '${ROOTFSDIR}/base-apt' && \
>              rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
>  
> +        mountpoint -q '${ROOTFSDIR}/dev/pts' && \
> +            umount '${ROOTFSDIR}/dev/pts'
> +        mountpoint -q '${ROOTFSDIR}/dev/shm' && \
> +            umount '${ROOTFSDIR}/dev/shm'
>          mountpoint -q '${ROOTFSDIR}/dev' && \
> -            umount -l ${ROOTFSDIR}/dev
> +            umount '${ROOTFSDIR}/dev'
>          mountpoint -q '${ROOTFSDIR}/proc' && \
> -            umount -l ${ROOTFSDIR}/proc
> +            umount '${ROOTFSDIR}/proc'
>          mountpoint -q '${ROOTFSDIR}/sys' && \
> -            umount -l ${ROOTFSDIR}/sys
> +            umount '${ROOTFSDIR}/sys'
>  
>          if [ -e "${ROOTFSDIR}/etc/apt/sources-list" ]; then
>              mv "${ROOTFSDIR}/etc/apt/sources-list" \
> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
> index a6ba0a9e..f5061a8b 100644
> --- a/meta/classes/isar-events.bbclass
> +++ b/meta/classes/isar-events.bbclass
> @@ -55,7 +55,7 @@ python build_completed() {
>              if basepath in line:
>                  bb.debug(1, '%s left mounted, unmounting...' % line.split()[1])
>                  subprocess.call(
> -                    ["sudo", "umount", "-l", line.split()[1]],
> +                    ["sudo", "umount", line.split()[1]],
>                      stdout=subprocess.DEVNULL,
>                      stderr=subprocess.DEVNULL,
>                  )
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index 498fbfd6..2e091e0c 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -37,11 +37,11 @@ rootfs_do_mounts() {
>          mountpoint -q '${ROOTFSDIR}/dev' || \
>              ( mount -o bind,private /dev '${ROOTFSDIR}/dev' &&
>                mount -t tmpfs none '${ROOTFSDIR}/dev/shm' &&
> -              mount --bind /dev/pts '${ROOTFSDIR}/dev/pts' )
> +              mount -o bind,private /dev/pts '${ROOTFSDIR}/dev/pts' )
>          mountpoint -q '${ROOTFSDIR}/proc' || \
>              mount -t proc none '${ROOTFSDIR}/proc'
>          mountpoint -q '${ROOTFSDIR}/sys' || \
> -            mount --rbind /sys '${ROOTFSDIR}/sys'
> +            mount -o bind,private /sys '${ROOTFSDIR}/sys'
>          mount --make-rslave '${ROOTFSDIR}/sys'
>  
>          # Mount isar-apt if the directory does not exist or if it is empty
> @@ -51,7 +51,7 @@ rootfs_do_mounts() {
>          then
>              mkdir -p '${ROOTFSDIR}/isar-apt'
>              mountpoint -q '${ROOTFSDIR}/isar-apt' || \
> -                mount --bind '${REPO_ISAR_DIR}/${DISTRO}' '${ROOTFSDIR}/isar-apt'
> +                mount -o bind,private '${REPO_ISAR_DIR}/${DISTRO}' '${ROOTFSDIR}/isar-apt'
>          fi
>  
>          # Mount base-apt if 'ISAR_USE_CACHED_BASE_REPO' is set
> @@ -59,7 +59,7 @@ rootfs_do_mounts() {
>          then
>              mkdir -p '${ROOTFSDIR}/base-apt'
>              mountpoint -q '${ROOTFSDIR}/base-apt' || \
> -                mount --bind '${REPO_BASE_DIR}' '${ROOTFSDIR}/base-apt'
> +                mount -o bind,private '${REPO_BASE_DIR}' '${ROOTFSDIR}/base-apt'
>          fi
>  
>  EOSUDO
> @@ -360,7 +360,7 @@ rootfs_install_sstate_prepare() {
>      # tar --one-file-system will cross bind-mounts to the same filesystem,
>      # so we use some mount magic to prevent that
>      mkdir -p ${WORKDIR}/mnt/rootfs
> -    sudo mount --bind ${WORKDIR}/rootfs ${WORKDIR}/mnt/rootfs -o ro
> +    sudo mount -o bind,private '${WORKDIR}/rootfs' '${WORKDIR}/mnt/rootfs' -o ro
>      lopts="--one-file-system --exclude=var/cache/apt/archives"
>      sudo tar -C ${WORKDIR}/mnt -cpSf rootfs.tar $lopts ${SSTATE_TAR_ATTR_FLAGS} rootfs
>      sudo umount ${WORKDIR}/mnt/rootfs
> diff --git a/meta/classes/sbuild.bbclass b/meta/classes/sbuild.bbclass
> index f1193c20..9c268281 100644
> --- a/meta/classes/sbuild.bbclass
> +++ b/meta/classes/sbuild.bbclass
> @@ -40,14 +40,14 @@ EOF
>          cp -rf "${SCHROOT_CONF}/sbuild" "${SBUILD_CONF_DIR}"
>          sbuild_fstab="${SBUILD_CONF_DIR}/fstab"
>  
> -        fstab_baseapt="${REPO_BASE_DIR} /base-apt none rw,bind 0 0"
> +        fstab_baseapt="${REPO_BASE_DIR} /base-apt none rw,bind,private 0 0"
>          grep -qxF "${fstab_baseapt}" ${sbuild_fstab} || echo "${fstab_baseapt}" >> ${sbuild_fstab}
>  
> -        fstab_pkgdir="${WORKDIR} /home/builder/${PN} none rw,bind 0 0"
> +        fstab_pkgdir="${WORKDIR} /home/builder/${PN} none rw,bind,private 0 0"
>          grep -qxF "${fstab_pkgdir}" ${sbuild_fstab} || echo "${fstab_pkgdir}" >> ${sbuild_fstab}
>  
>          if [ -d ${DL_DIR} ]; then
> -            fstab_downloads="${DL_DIR} /downloads none rw,bind 0 0"
> +            fstab_downloads="${DL_DIR} /downloads none rw,bind,private 0 0"
>              grep -qxF "${fstab_downloads}" ${sbuild_fstab} || echo "${fstab_downloads}" >> ${sbuild_fstab}
>          fi
>  EOSUDO
> @@ -98,7 +98,7 @@ insert_mounts() {
>      sudo -s <<'EOSUDO'
>          set -e
>          for mp in ${SCHROOT_MOUNTS}; do
> -            FSTAB_LINE="${mp%%:*} ${mp#*:} none rw,bind 0 0"
> +            FSTAB_LINE="${mp%%:*} ${mp#*:} none rw,bind,private 0 0"
>              grep -qxF "${FSTAB_LINE}" ${SBUILD_CONF_DIR}/fstab || \
>                  echo "${FSTAB_LINE}" >> ${SBUILD_CONF_DIR}/fstab
>          done
> @@ -109,7 +109,7 @@ remove_mounts() {
>      sudo -s <<'EOSUDO'
>          set -e
>          for mp in ${SCHROOT_MOUNTS}; do
> -            FSTAB_LINE="${mp%%:*} ${mp#*:} none rw,bind 0 0"
> +            FSTAB_LINE="${mp%%:*} ${mp#*:} none rw,bind,private 0 0"
>              sed -i "\|${FSTAB_LINE}|d" ${SBUILD_CONF_DIR}/fstab
>          done
>  EOSUDO
> @@ -122,7 +122,7 @@ schroot_configure_ccache() {
>  
>          sbuild_fstab="${SBUILD_CONF_DIR}/fstab"
>  
> -        fstab_ccachedir="${CCACHE_DIR} /ccache none rw,bind 0 0"
> +        fstab_ccachedir="${CCACHE_DIR} /ccache none rw,bind,private 0 0"
>          grep -qxF "${fstab_ccachedir}" ${sbuild_fstab} || echo "${fstab_ccachedir}" >> ${sbuild_fstab}
>  
>          (flock 9
> diff --git a/meta/classes/sdk.bbclass b/meta/classes/sdk.bbclass
> index 71db6f3a..754fd4cd 100644
> --- a/meta/classes/sdk.bbclass
> +++ b/meta/classes/sdk.bbclass
> @@ -92,9 +92,16 @@ sdkchroot_configscript () {
>  
>  ROOTFS_POSTPROCESS_COMMAND:append:class-sdk = " sdkchroot_finalize"
>  sdkchroot_finalize() {
> -    sudo umount -R ${ROOTFSDIR}/dev || true
> -    sudo umount ${ROOTFSDIR}/proc || true
> -    sudo umount -R ${ROOTFSDIR}/sys || true
> +    mountpoint -q "${ROOTFSDIR}/dev/pts" && \
> +        sudo umount "${ROOTFSDIR}/dev/pts"
> +    mountpoint -q "${ROOTFSDIR}/dev/shm" && \
> +        sudo umount "${ROOTFSDIR}/dev/shm"
> +    mountpoint -q "${ROOTFSDIR}/dev" && \
> +        sudo umount "${ROOTFSDIR}/dev"
> +    mountpoint -q "${ROOTFSDIR}/proc" && \
> +        sudo umount "${ROOTFSDIR}/proc"
> +    mountpoint -q "${ROOTFSDIR}/sys" && \
> +        sudo umount "${ROOTFSDIR}/sys"
>  
>      # Remove setup scripts
>      sudo rm -f ${ROOTFSDIR}/chroot-setup.sh ${ROOTFSDIR}/configscript.sh
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> index faf22a50..6bc667e7 100644
> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> @@ -341,7 +341,7 @@ do_bootstrap() {
>              echo "deb-src ${line}" >>  "${ROOTFSDIR}/etc/apt/sources.list.d/base-apt.list"
>  
>              mkdir -p ${ROOTFSDIR}/base-apt
> -            mount --bind ${REPO_BASE_DIR} ${ROOTFSDIR}/base-apt
> +            mount -o bind,private "${REPO_BASE_DIR}" "${ROOTFSDIR}/base-apt"
>          else
>              install -v -m644 "${APTSRCS}" \
>                               "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"
> @@ -378,10 +378,10 @@ do_bootstrap() {
>  
>          # update APT
>          mount -o bind,private /dev ${ROOTFSDIR}/dev

That means that we mount /dev from "the host" into the rootfs/chroot
environment, right? "Submounts included", no?

I'm facing the following error with that. The isar build is executed
inside a container / pod running on a k8s cluster.

Inside the container the following steps are executed:

build-bookworm:
  script:
    - kas build <path-to-kas-file>
    - sudo rm -rf build/tmp/work/debian-bookworm-amd64/buildchroot-target

The rm command triggers the following errors:

rm: cannot remove 'build/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev/shm': Device or resource busy
rm: cannot remove 'build/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev/termination-log': Device or resource busy
rm: cannot remove 'build/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev/mqueue': Device or resource busy
rm: cannot remove 'build/tmp/work/debian-bookworm-amd64/sbuild-chroot-target/1.0-r0/rootfs/dev/pts': Device or resource busy

My assumption is that isar is not cleaning up all mounts. I think the
interesting part is about /dev/termination-log.
That device is mounted into the pod by the k8s infrastructure.

I think we should never mount that into the chroot, or make sure it is
properly unmounted again...

Any thoughts on that? Any additional information needed?

Florian

> -        mount --bind /dev/pts ${ROOTFSDIR}/dev/pts
> +        mount -o bind,private /dev/pts "${ROOTFSDIR}/dev/pts"
>          mount -t tmpfs none "${ROOTFSDIR}/dev/shm"
>          mount -t proc none ${ROOTFSDIR}/proc
> -        mount --rbind /sys ${ROOTFSDIR}/sys
> +        mount -o bind,private /sys "${ROOTFSDIR}/sys"
>          mount --make-rslave ${ROOTFSDIR}/sys
>  
>          export DEBIAN_FRONTEND=noninteractive
> @@ -400,12 +400,13 @@ do_bootstrap() {
>          chroot "${ROOTFSDIR}" /usr/bin/apt-get dist-upgrade -y \
>                                  -o Debug::pkgProblemResolver=yes
>  
> -        umount -l "${ROOTFSDIR}/dev/shm"
> -        umount -l "${ROOTFSDIR}/dev/pts"
> -        umount -l "${ROOTFSDIR}/dev"
> -        umount -l "${ROOTFSDIR}/proc"
> -        umount -l "${ROOTFSDIR}/sys"
> -        umount -l "${ROOTFSDIR}/base-apt" || true
> +        umount "${ROOTFSDIR}/dev/shm"
> +        umount "${ROOTFSDIR}/dev/pts"
> +        umount "${ROOTFSDIR}/dev"
> +        umount "${ROOTFSDIR}/proc"
> +        umount "${ROOTFSDIR}/sys"
> +        mountpoint -q "${ROOTFSDIR}/base-apt" && \
> +            umount "${ROOTFSDIR}/base-apt"
>  
>          # Finalize debootstrap by setting the link in deploy
>          ln -Tfsr "${ROOTFSDIR}" "${DEPLOY_ISAR_BOOTSTRAP}"
> diff --git a/meta/recipes-devtools/sdk-files/files/README.sdk b/meta/recipes-devtools/sdk-files/files/README.sdk
> index 3e06d8c5..29c09950 100644
> --- a/meta/recipes-devtools/sdk-files/files/README.sdk
> +++ b/meta/recipes-devtools/sdk-files/files/README.sdk
> @@ -29,7 +29,7 @@ $ sudo <sdk_rootfs>/mount_chroot.sh <sdk_rootfs>
>  
>  Bind-mount the project into the rootfs:
>  
> -$ sudo mount -o bind /path/to/project <sdk_rootfs>/mnt
> +$ sudo mount -o bind,private /path/to/project <sdk_rootfs>/mnt
>  
>  If you have relocated the SDK previously for using option 1, you need to call
>  this next:
> diff --git a/scripts/mount_chroot.sh b/scripts/mount_chroot.sh
> index e238f1cc..122bb33b 100755
> --- a/scripts/mount_chroot.sh
> +++ b/scripts/mount_chroot.sh
> @@ -2,7 +2,7 @@
>  
>  set -e
>  
> -mount /tmp     "$1/tmp"                 -o bind
> +mount /tmp     "$1/tmp"                 -o bind,private
>  mount proc     "$1/proc"    -t proc     -o nosuid,noexec,nodev
>  mount sysfs    "$1/sys"     -t sysfs    -o nosuid,noexec,nodev
>  mount devtmpfs "$1/dev"     -t devtmpfs -o mode=0755,nosuid
> -- 
> 2.34.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20240619104126.105252-1-amikan%40ilbers.de.

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/dccfd6b9e590e3429f74a990648e27c4d5e25ff8.camel%40siemens.com.

  parent reply	other threads:[~2024-08-28 13:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-19 10:41 Anton Mikanovich
2024-06-26  6:31 ` Uladzimir Bely
2024-08-28 13:05 ` 'Florian Bezdeka' via isar-users [this message]
2024-08-29  9:26   ` Anton Mikanovich
2024-08-29 12:26     ` 'Florian Bezdeka' via isar-users
2024-09-06 14:34       ` Anton Mikanovich
2024-10-01 11:59         ` 'Florian Bezdeka' via isar-users
2024-10-01 12:14           ` 'Jan Kiszka' via isar-users
2024-10-01 12:18             ` 'Florian Bezdeka' via isar-users
2024-10-01 12:28               ` 'Jan Kiszka' via isar-users
2024-10-03 14:37           ` Baurzhan Ismagulov
2024-10-04  7:28             ` 'Florian Bezdeka' via isar-users
2024-10-04  8:00               ` Uladzimir Bely
2024-10-04  8:16                 ` 'Jan Kiszka' via isar-users
2024-10-10  4:33                   ` 'Jan Kiszka' via isar-users
2024-10-10  9:43                     ` Baurzhan Ismagulov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dccfd6b9e590e3429f74a990648e27c4d5e25ff8.camel@siemens.com \
    --to=isar-users@googlegroups.com \
    --cc=amikan@ilbers.de \
    --cc=florian.bezdeka@siemens.com \
    --cc=iskochilov@ilbers.de \
    --cc=jan.kiszka@siemens.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox