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

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

[Felix Moessbauer]

Changes since v1:

- fix path to ROOTFSDIR

Felix Moessbauer (1):
  fix: mount pts into rootfs

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

-- 
2.30.2

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

[Felix Moessbauer]

This fixes a regression introduced in 4e1dacc.
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' )
         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"
-- 
2.30.2

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

[Henning Schild]

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.

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"

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"

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

[Moessbauer, Felix]

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

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

[Anton Mikanovich]

09.05.2022 17:34, Moessbauer, Felix wrote:
> 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.
>
I'm already working on schroot-based imager integration, will send the 
patchset
after some stabilization.
This changes will allow to get rid of buildchroots (and its manual mounts)
completely.

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

[Baurzhan Ismagulov]

On Mon, May 09, 2022 at 02:34:54PM +0000, Moessbauer, Felix wrote:
> > > 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.

I think the final solution should be removing buildchroots (we need a couple of
days for v1, +/-).

If an interim solution would be needed, mount --bind /dev/pts from the host
should be used. In my local projects, I had started with mount -t devpts but
switched to --bind in 2018 due to issues (at least "sudo: no tty present and no
askpass program specified", not sure about other packages).

In general, we'd like to work on removing the few remaining recursive mounts
from Isar due to various issues (exponential mounts, asymmetric umounts,
umounting problems).

For debugging issues like "which package version was available in isar-apt when
package X was being built", maybe we could evaluate something like an option
for cp -al or similar.

With kind regards,
Baurzhan.

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

[Moessbauer, Felix]

> -----Original Message-----
> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> Behalf Of Baurzhan Ismagulov
> Sent: Wednesday, May 11, 2022 10:04 AM
> To: isar-users@googlegroups.com
> Subject: Re: [PATCH v2 1/1] fix: mount pts into rootfs
> 
> On Mon, May 09, 2022 at 02:34:54PM +0000, Moessbauer, Felix wrote:
> > > > 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.
> 
> I think the final solution should be removing buildchroots (we need a couple of
> days for v1, +/-).
> 
> If an interim solution would be needed, mount --bind /dev/pts from the host
> should be used. In my local projects, I had started with mount -t devpts but
> switched to --bind in 2018 due to issues (at least "sudo: no tty present and no
> askpass program specified", not sure about other packages).

IMO we will need an interim solution as downstream layers will need some time to adapt to the sbuild / schroot infrastructure.
By that, it is likely that pre sbuild ISAR will still be used for a while.

I just tested mounting the pts with --bind an this also works. Will send out a v3 soon.

Felix

> 
> In general, we'd like to work on removing the few remaining recursive mounts
> from Isar due to various issues (exponential mounts, asymmetric umounts,
> umounting problems).
> 
> For debugging issues like "which package version was available in isar-apt when
> package X was being built", maybe we could evaluate something like an option
> for cp -al or similar.
> 
> With kind regards,
> Baurzhan.
> 
> --
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> oogle.com%2Fd%2Fmsgid%2Fisar-
> users%2FYntt4IxrlQRYBG8F%2540ilbers.de&amp;data=05%7C01%7Cfelix.moess
> bauer%40siemens.com%7C8809ce5e859c4d5fa1dd08da3324c730%7C38ae3bcd
> 95794fd4addab42e1495d55a%7C1%7C0%7C637878530630108288%7CUnknow
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=d9yj%2B0wy7ubLiCA8jV
> DRUhc60r%2FcgD%2BG9uQDmSYyiWY%3D&amp;reserved=0.