particle-os/debian-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

mounts/ostree.deployment: rework unmounting

Browse source 
This unwinds part of a25ae2b. The way the code ended up both
self.tree and self.mountpoint ended up pointing to the exactly
same path and so we'd end up doing two `umount -R` operations
on the same path. This ended up being a duplicate unmount.

On Fedora 39 this yields an error like:

```
mount/ostree.deployment (org.osbuild.ostree.deployment): umount: /var/osbuild/store/stage/uuid-efaac9370d25455d9e8df6d847ecb5b3/data/tree: not mounted
mount/ostree.deployment (org.osbuild.ostree.deployment): Traceback (most recent call last):
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/var/b/shared/code/github.com/osbuild/osbuild/mounts/org.osbuild.ostree.deployment", line 136, in <module>
mount/ostree.deployment (org.osbuild.ostree.deployment):     main()
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/var/b/shared/code/github.com/osbuild/osbuild/mounts/org.osbuild.ostree.deployment", line 132, in main
mount/ostree.deployment (org.osbuild.ostree.deployment):     service.main()
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/var/b/shared/code/github.com/osbuild/osbuild/osbuild/host.py", line 252, in main
mount/ostree.deployment (org.osbuild.ostree.deployment):     self.stop()
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/var/b/shared/code/github.com/osbuild/osbuild/osbuild/mounts.py", line 126, in stop
mount/ostree.deployment (org.osbuild.ostree.deployment):     self.umount()
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/var/b/shared/code/github.com/osbuild/osbuild/mounts/org.osbuild.ostree.deployment", line 125, in umount
mount/ostree.deployment (org.osbuild.ostree.deployment):     subprocess.run(["umount", "-R", self.tree],
mount/ostree.deployment (org.osbuild.ostree.deployment):   File "/usr/lib64/python3.12/subprocess.py", line 571, in run
mount/ostree.deployment (org.osbuild.ostree.deployment):     raise CalledProcessError(retcode, process.args,
mount/ostree.deployment (org.osbuild.ostree.deployment): subprocess.CalledProcessError: Command '['umount', '-R', '/var/osbuild/store/stage/uuid-efaac9370d25455d9e8df6d847ecb5b3/data/tree']
' returned non-zero exit status 1.

⏱  Duration: 103s
```

I think this was necessary because of a bug in util-linux that
mean some of the accounting information got out of date when
doing a `mount --move` operation, which we use here. I think this
bug (or bugs) is now fixed [1][2] in util-linux v2.39 (in Fedora 39),
which is now causing the above pasted error on F39.

Let's just add code here that mentions the problem and workaround
it with a loop to keep unmounting (essentially what the umount -R
should have done to overmounted filesystems if the mountinfo/utab
was correct) and also mention when we should be able to drop this
workaround.

[1] a04149fbb7
[2] 8cf6c50757
This commit is contained in:
Dusty Mabe 2024-01-09 23:23:25 -05:00
parent 21626926f7
commit 0da68e9af5
1 changed files with 34 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

44
mounts/org.osbuild.ostree.deployment
View file

@ -63,7 +63,6 @@ class OSTreeDeploymentMount(mounts.MountService):
def __init__(self, args):
super().__init__(args)
self.tree = None
self.mountpoint = None
self.check = False
@ -72,7 +71,22 @@ class OSTreeDeploymentMount(mounts.MountService):
subprocess.run([
"mount", "--bind", "--make-private", source, target,
], check=True)
return target
def is_mounted(self):
# Use `mountpoint` command here to determine if the mountpoint is mounted.
# We would use os.path.ismount() here but that only works if a device is
# mounted (i.e. it doesn't use the mountinfo file in the heuristic and
# thus things like `mount --move` wouldn't show up). The exit codes from
# `mountpoint` are:
#
# 0 success; the directory is a mountpoint, or device is block device on --devno
# 1 failure; incorrect invocation, permissions or system error
# 32 failure; the directory is not a mountpoint, or device is not a block device on --devno
#
cp = subprocess.run(["mountpoint", "-q", self.mountpoint], check=False)
if cp.returncode not in [0, 32]:
cp.check_returncode() # will raise error
return cp.returncode == 0
def mount(self, args: Dict):
@ -89,7 +103,7 @@ class OSTreeDeploymentMount(mounts.MountService):
# is contained inside tree, since "moving a mount residing
# under a shared mount is invalid and unsupported."
# - `mount(8)`
self.tree = self.bind_mount(tree, tree)
self.bind_mount(tree, tree)
root = ostree.deployment_path(tree, osname, ref, serial)
@ -116,16 +130,26 @@ class OSTreeDeploymentMount(mounts.MountService):
if self.mountpoint:
subprocess.run(["sync", "-f", self.mountpoint],
check=self.check)
subprocess.run(["umount", "-R", self.mountpoint],
subprocess.run(["umount", "-v", "-R", self.mountpoint],
check=self.check)
# Handle bug in older util-linux mount where the
# mountinfo/utab wouldn't have updated information
# when mount --move is performed, which means that
# umount -R wouldn't unmount all overmounted mounts
# on the target because it was operating on outdated
# information. The umount -R behavior is fixed in v2.39
# of util-linux most likely by [1] or [2] or both. This
# loop can be removed when all hosts we care about have
# moved to v2.39+.
# [1] https://github.com/karelzak/util-linux/commit/a04149fbb7c1952da1194d1514e298ff07dbc7ca
# [2] https://github.com/karelzak/util-linux/commit/8cf6c5075780598fe3b30e7a7753d8323d093e22
while self.is_mounted():
print(f"extra unmount {self.mountpoint}")
subprocess.run(["umount", "-v", self.mountpoint],
check=self.check)
self.mountpoint = None
if self.tree:
subprocess.run(["umount", "-R", self.tree],
check=self.check)
self.tree = None
def main():
service = OSTreeDeploymentMount.from_args(sys.argv[1:])