meta: Drop lazy and recursive unmounts

Message ID 20240619104126.105252-1-amikan@ilbers.de
State Accepted, archived
Headers show
Series meta: Drop lazy and recursive unmounts | expand

Commit Message

Anton Mikanovich June 19, 2024, 10:41 a.m. UTC
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(-)

Comments

Uladzimir Bely June 26, 2024, 6:31 a.m. UTC | #1
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(-)

Applied to next.
Florian Bezdeka Aug. 28, 2024, 1:05 p.m. UTC | #2
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.
Anton Mikanovich Aug. 29, 2024, 9:26 a.m. UTC | #3
28/08/2024 16:05, Florian Bezdeka wrote:
> 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:
>> 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

Hello Florian,

The line you are pointing to is inside isar-bootstrap preparations, 
while your
error is pointing to busy mountpoints in sbuild-chroot, managed by
rootfs.bbclass.
Anyway private mount private mount does not forward or receive
propagations. It means no external submounts will be mounted inside 
rootfs and
no Isar mounts will be mounted to external /dev.
Can you check which mountpoints are left after the build (before rm)?
Florian Bezdeka Aug. 29, 2024, 12:26 p.m. UTC | #4
On Thu, 2024-08-29 at 12:26 +0300, Anton Mikanovich wrote:
> 28/08/2024 16:05, Florian Bezdeka wrote:
> > 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:
> > > 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/sbuild-chroot-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
> 
> Hello Florian,
> 
> The line you are pointing to is inside isar-bootstrap preparations, 
> while your
> error is pointing to busy mountpoints in sbuild-chroot, managed by
> rootfs.bbclass.
> Anyway private mount private mount does not forward or receive
> propagations. It means no external submounts will be mounted inside 
> rootfs and
> no Isar mounts will be mounted to external /dev.
> Can you check which mountpoints are left after the build (before rm)?

I think this is the relevant line from "mount":

tmpfs on /builds/<project>/build/tmp/work/debian-bookworm-amd64/sbuild-
chroot-target/1.0-r0/rootfs/dev type tmpfs
(rw,nosuid,size=65536k,mode=755,inode64)

Florian
Anton Mikanovich Sept. 6, 2024, 2:34 p.m. UTC | #5
29/08/2024 15:26, Florian Bezdeka wrote:
> I think this is the relevant line from "mount":
>
> tmpfs on /builds/<project>/build/tmp/work/debian-bookworm-amd64/sbuild-
> chroot-target/1.0-r0/rootfs/dev type tmpfs
> (rw,nosuid,size=65536k,mode=755,inode64)
>
> Florian

So it is sbuild who should manage mount in this rootfs. Not sure how 
this patch
influence on internal sbuild mount/unmount logic. But sbuild shouldn't 
left any
mounts without any errors. Do you have any packages failed to be built?

Are there any lost schroot sessions running before external rm you have 
issue
with? This can be checked with 'schroot -la' command.

You can also try to add some verbose to sbuild with following change:

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index bcc3f828..b54e3ab9 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -113,6 +113,8 @@ dpkg_runbuild() {
      DSC_FILE=$(find ${WORKDIR} -maxdepth 1 -name 
"${DEBIAN_SOURCE}_*.dsc" -print)

      sbuild -A -n -c ${SBUILD_CHROOT} \
+        -D \
+        --chroot-setup-commands="mount" \
          --host=${PACKAGE_ARCH} --build=${BUILD_ARCH} ${profiles} \
          --no-run-lintian --no-run-piuparts --no-run-autopkgtest 
--resolve-alternatives \
          --bd-uninstallable-explainer=apt \

This will be very helpfull to get some build logs from your setup.

Patch

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
-        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