Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpmostreepayload: Rework binds to be recursive #903

Merged
merged 1 commit into from Dec 13, 2016

Conversation

cgwalters
Copy link
Contributor

Our hardcoded list of mount points missed critical ones
like /sys/firmware/efi/efivars, which broke installation
on EFI.

I started to add that to the list, but then I realized there's a much
simpler solution - make the binds recursive and let mount do the
work for us. This is a bit more risky of a change, but it'll
clearly be more maintainable long term.

Down the line...we may want to simply walk over all the toplevel mount
points in /mnt/sysimage, but I doubt anyone is going to add anything
new that's not a subdirectory of /sys.

See https://pagure.io/atomic-wg/issue/185

Our hardcoded list of mount points missed critical ones
like `/sys/firmware/efi/efivars`, which broke installation
on EFI.

I started to add that to the list, but then I realized there's a much
simpler solution - make the binds recursive and let `mount` do the
work for us.  This is a bit more risky of a change, but it'll
clearly be more maintainable long term.

Down the line...we may want to simply walk over all the toplevel mount
points in `/mnt/sysimage`, but I doubt anyone is going to add anything
new that's not a subdirectory of `/sys`.

See https://pagure.io/atomic-wg/issue/185

Related: https://github.com/rhinstaller/anaconda/issues/900
@cgwalters
Copy link
Contributor Author

cgwalters commented Dec 9, 2016

Here's an updates.img for this (more info on updates).

curl -L -O 'https://fedorapeople.org/~walters/updates.img{,.asc}'
gpg2 --verify updates.img.asc

@jkonecny12
Copy link
Member

Jenkins, it's ok to test.

Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@rvykydal
Copy link
Contributor

Looks good to me.

@dustymabe
Copy link
Contributor

can we get this merged? - we'd like to get things fixed so our next two week release of Atomic Host has a fix for this.

@M4rtinK M4rtinK merged commit bab1f5f into rhinstaller:master Dec 13, 2016
@dustymabe
Copy link
Contributor

dustymabe commented Dec 13, 2016

can we get this merged in for f25 and cut a new rpm for Fedora?

@cgwalters cgwalters mentioned this pull request Dec 14, 2016
cgwalters added a commit to cgwalters/anaconda that referenced this pull request Jul 12, 2017
In rhinstaller#903 we
changed all of rpmostreepayload's internal mounts to be
recursive to fix `/boot/efi`.  And indeed all should be recursive,
except one - the `/sysroot` mount.  By asking for that to be
recursive we've created a loop.

I noticed a while ago that the output of `findmnt` during an install was insane
(the deployment directory was visible twice, etc.), but never debugged it. It
turns out this was the cause.

Fixing this makes the mounts look a lot more sane, and will help
for future work I have for `/var` mounts.
cgwalters added a commit to cgwalters/anaconda that referenced this pull request Jul 12, 2017
In rhinstaller#903 we
changed all of rpmostreepayload's internal mounts to be
recursive to fix `/boot/efi`.  And indeed all should be recursive,
except one - the `/sysroot` mount.  By asking for that to be
recursive we've created a loop.

I noticed a while ago that the output of `findmnt` during an install was insane
(the deployment directory was visible twice, etc.), but never debugged it. It
turns out this was the cause.

Fixing this makes the mounts look a lot more sane, and will help
for future work I have for `/var` mounts.
cgwalters added a commit to cgwalters/anaconda that referenced this pull request Jul 24, 2017
In rhinstaller#903 we
changed all of rpmostreepayload's internal mounts to be
recursive to fix `/boot/efi`.  And indeed all should be recursive,
except one - the `/sysroot` mount.  By asking for that to be
recursive we've created a loop.

I noticed a while ago that the output of `findmnt` during an install was insane
(the deployment directory was visible twice, etc.), but never debugged it. It
turns out this was the cause.

Fixing this makes the mounts look a lot more sane, and will help
for future work I have for `/var` mounts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants