Fix dynamic mountpoint code to allow multiple mounts when the user specifies the mountpoint.
From: "Brian C. Lane" bcl@redhat.com
The dynamic mountpoint detection patch revealed a problem with multiple mountpoints. Sometimes things are mounted in multiple places and the first one listed by the system isn't always the one you want to unmount.
This adds the mountpoint argument to unmount so that a specific location can be selected. It must exist or else FSError will be raised.
The old code worked because it replaced the _mountpoint value so it would unmount the last mounted location, but that wasn't correct because you then lost the information about previous mountpoints.
With the current code the caller is expected to keep track of mountpoints if more than one is used and to pass mountpoint= to unmount. --- blivet/formats/fs.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 4e3d39f..e1b5bba 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -628,6 +628,10 @@ def mount(self, options="", chroot="/", mountpoint=None): :keyword chroot: prefix to apply to mountpoint :keyword mountpoint: mountpoint (overrides self.mountpoint) :raises: FSError + + NOTE: When mounted multiple times the unmount method needs to be called with + a specific mountpoint to unmount, otherwise it will try to unmount the first + one listed by the system. """ if not self.exists: raise FSError("filesystem has not been created") @@ -678,23 +682,34 @@ def mount(self, options="", chroot="/", mountpoint=None): if not ret: log.warning("Failed to set SELinux context for newly mounted filesystem lost+found directory at %s to %s", lost_and_found_path, lost_and_found_context)
- def unmount(self): - """ Unmount this filesystem. """ + def unmount(self, mountpoint=None): + """ Unmount this filesystem. + + :param str mountpoint: Optional mountpoint to be unmounted. + :raises: FSError + + If mountpoint isn't passed this will unmount the first mountpoint listed by the + system. Override this behavior by passing a specific mountpoint. FSError will + be raised in either case if the path doesn't exist. + """ if not self.exists: raise FSError("filesystem has not been created")
- if not self.systemMountpoint: + # Prefer the explicit mountpoint path, fall back to first system mountpoint + mountpoint = mountpoint or self.systemMountpoint + + if not mountpoint: # not mounted return
- if not os.path.exists(self.systemMountpoint): + if not os.path.exists(mountpoint): raise FSError("mountpoint does not exist")
udev.settle() - rc = util.umount(self.systemMountpoint) + rc = util.umount(mountpoint) if rc: # try and catch whatever is causing the umount problem - util.run_program(["lsof", self.systemMountpoint]) + util.run_program(["lsof", mountpoint]) raise FSError("umount failed")
def readLabel(self):
From: "Brian C. Lane" bcl@redhat.com
Check to see if a mountpoint path is already mounted. --- blivet/mounts.py | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/blivet/mounts.py b/blivet/mounts.py index 069a970..a2cd208 100644 --- a/blivet/mounts.py +++ b/blivet/mounts.py @@ -50,6 +50,15 @@ def getMountpoint(self, devspec, subvolspec=None):
return self.mountpoints.get((devspec, subvolspec))
+ def isMounted(self, mountpoint): + """ Check to see if a path is already mounted + + :param str mountpoint: Path to check for + """ + self._cacheCheck() + + return mountpoint in self.mountpoints.values() + def _getActiveMounts(self): """ Get information about mounted devices from /proc/mounts and /proc/self/mountinfo
From: "Brian C. Lane" bcl@redhat.com
Instead of using self.status which checks to see if anything is mounted, not the specific mountpoint requested. --- blivet/formats/fs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index e1b5bba..8142081 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -642,7 +642,8 @@ def mount(self, options="", chroot="/", mountpoint=None): if not mountpoint: raise FSError("no mountpoint given")
- if self.status: + # Is something is already mounted at this path? + if mountsCache.isMounted(mountpoint): return
if not isinstance(self, NoDevFS) and not os.path.exists(self.device):
From: "Brian C. Lane" bcl@redhat.com
Otherwise it won't mount things. --- blivet/formats/fs.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 8142081..1320974 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -642,10 +642,6 @@ def mount(self, options="", chroot="/", mountpoint=None): if not mountpoint: raise FSError("no mountpoint given")
- # Is something is already mounted at this path? - if mountsCache.isMounted(mountpoint): - return - if not isinstance(self, NoDevFS) and not os.path.exists(self.device): raise FSError("device %s does not exist" % self.device)
@@ -656,6 +652,11 @@ def mount(self, options="", chroot="/", mountpoint=None): #mountpoint = os.path.join(chroot, mountpoint) chrootedMountpoint = os.path.normpath("%s/%s" % (chroot, mountpoint))
+ # Is something is already mounted at this path? + if mountsCache.isMounted(chrootedMountpoint): + log.debug("BCL: %s is already mounted, not remounting it. %s", chrootedMountpoint, self.device) + return + # passed in options override default options if not options or not isinstance(options, str): options = self.options
Could you piggyback a little patch on this that gets rid of the FIXME in FS.status definition? Cause it is definitely obsolete now.
anaconda-patches@lists.fedorahosted.org