RE: [PATCH v2 1/1] fix: mount pts into rootfs

["Moessbauer, Felix" <felix.moessbauer@siemens.com>]

> -----Original Message-----
> From: Schild, Henning (T CED SES-DE) <henning.schild@siemens.com>
> Sent: Wednesday, May 4, 2022 5:54 PM
> To: Moessbauer, Felix (T CED SES-DE) <felix.moessbauer@siemens.com>;
> Kiszka, Jan (T CED) <jan.kiszka@siemens.com>
> Cc: isar-users@googlegroups.com
> Subject: Re: [PATCH v2 1/1] fix: mount pts into rootfs
> 
> Am Wed,  4 May 2022 14:38:02 +0200
> schrieb Felix Moessbauer <felix.moessbauer@siemens.com>:
> 
> > This fixes a regression introduced in 4e1dacc.
> 
> I wonder whether we should not instead look into reverting 4e1dacc and finding
> a solution for just the sub-mount it was written for.

IMO we should try to mimic what the schroot provides (and create the devices instead of bind-mounting).
In the sbuild-chroot (from ubely/sbuild), the following is available (but I did not check how these were created):

find /dev/
/dev/
/dev/stderr
/dev/stdout
/dev/stdin
/dev/fd
/dev/null
/dev/zero
/dev/full
/dev/random
/dev/urandom
/dev/tty
/dev/console
/dev/pts
/dev/pts/1
/dev/pts/0
/dev/pts/ptmx
/dev/shm
/dev/ptmx

> 
> i.e.
> mount --rbind /dev dev && umount dev/shm && mount -t tmpfs none dev/shm
> 
> seems to work just fine, leaving dev/pts mounted and also dev/mqueue which
> was also removed by 4e1dacc or dev/hugepages which i found on another host
> 
> Who knows what all those sub-mounts are for and what else has been left out.
> But the commit message ends with the recursion having been removed on
> purpose.
> 
> > Pseudo ttys might be required during package building as well as
> > package installation (e.g. to make expect work).
> >
> > The patch recursively mounts all pts from the host.
> >
> > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > ---
> >  meta/classes/buildchroot.bbclass                    | 3 ++-
> >  meta/classes/rootfs.bbclass                         | 3 ++-
> >  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 2 ++
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/classes/buildchroot.bbclass
> > b/meta/classes/buildchroot.bbclass index 3d2211b9..527f8ded 100644
> > --- a/meta/classes/buildchroot.bbclass
> > +++ b/meta/classes/buildchroot.bbclass
> > @@ -43,7 +43,8 @@ buildchroot_do_mounts() {
> >          fi
> >          mountpoint -q '${BUILDCHROOT_DIR}/dev' ||
> >              ( mount --bind /dev '${BUILDCHROOT_DIR}/dev' &&
> > -              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' )
> > +              mount -t tmpfs none '${BUILDCHROOT_DIR}/dev/shm' &&
> > +              mount --rbind /dev/pts '${BUILDCHROOT_DIR}/dev/pts' )
> 
> if recursion was removed on purpose we should probably mount "-t devpts" and
> not "--rbind"

Yes, I am happy to send a v3 with that fix, but before doing so we should agree on a direction.
In the end, it would probably be better to let schroot handle our buildchroots.
This shouldn't be too much effort, once the sbuilder patches are merged.

Felix

> 
> Henning
> 
> >          mountpoint -q '${BUILDCHROOT_DIR}/proc' ||
> >              mount -t proc none '${BUILDCHROOT_DIR}/proc'
> >          mountpoint -q '${BUILDCHROOT_DIR}/sys' || diff --git
> > a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass index
> > 7e9fb0f3..9f3ec0e3 100644
> > --- a/meta/classes/rootfs.bbclass
> > +++ b/meta/classes/rootfs.bbclass
> > @@ -35,7 +35,8 @@ rootfs_do_mounts() {
> >          set -e
> >          mountpoint -q '${ROOTFSDIR}/dev' || \
> >              ( mount --bind /dev '${ROOTFSDIR}/dev' &&
> > -              mount -t tmpfs none '${ROOTFSDIR}/dev/shm' )
> > +              mount -t tmpfs none '${ROOTFSDIR}/dev/shm' &&
> > +              mount --rbind /dev/pts '${ROOTFSDIR}/dev/pts' )
> >          mountpoint -q '${ROOTFSDIR}/proc' || \
> >              mount -t proc none '${ROOTFSDIR}/proc'
> >          mountpoint -q '${ROOTFSDIR}/sys' || \ diff --git
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > 145d5e87..ba8ad094 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -362,6
> > +362,7 @@ do_bootstrap() {
> >          # update APT
> >          mount --bind /dev ${ROOTFSDIR}/dev
> > +        mount --rbind /dev/pts ${ROOTFSDIR}/dev/pts
> >          mount -t tmpfs none "${ROOTFSDIR}/dev/shm"
> >          mount -t proc none ${ROOTFSDIR}/proc
> >          mount --rbind /sys ${ROOTFSDIR}/sys @@ -383,6 +384,7 @@
> > do_bootstrap() {
> >                                  -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"