Supersedes PR#107.
The final patch, "Cache data..." is the only interesting one.
The main change from previous PR is that since mountinfo cache is only needed in _getActiveMounts(), it is only instantiated there. In #107 is was an attribute of the mounts cache, which was excessive. This way, the cache needs to be managed less, which is good.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/mounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/mounts.py b/blivet/mounts.py index b781d50..617ca21 100644 --- a/blivet/mounts.py +++ b/blivet/mounts.py @@ -70,7 +70,7 @@ def _getActiveMounts(self): self.mountpoints = defaultdict(list) for line in open("/proc/mounts").readlines(): try: - (devspec, mountpoint, fstype, _options, _rest) = line.split(None, 4) + (devspec, mountpoint, fstype, _rest) = line.split(None, 3) except ValueError: log.error("failed to parse /proc/mounts line: %s", line) continue
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/mounts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/mounts.py b/blivet/mounts.py index d7d6851..b781d50 100644 --- a/blivet/mounts.py +++ b/blivet/mounts.py @@ -65,7 +65,7 @@ def _getActiveMounts(self): """ Get information about mounted devices from /proc/mounts and /proc/self/mountinfo
- Refreshes self.mountpoints with current moutpoint information + Refreshes self.mountpoints with current mountpoint information """ self.mountpoints = defaultdict(list) for line in open("/proc/mounts").readlines():
From: mulhern amulhern@redhat.com
This avoids reading and parsing the file whenever additional btrfs information needs to be looked up.
Changes are: * Addition of a small, special purpose cache for btrfs information obtained from mountinfo. The only facility the cache offers is laziness. Since it currently is instantiated and destroyed in a single method where it is used in a very simple way it does not require other management facilities. * Slightly improved parsing of /proc/self/mountinfo which takes into account some variable number fields. * Make sure all components of key values are str or NoneType when inserted into mounts cache or when looked up. * Use context manager for "/proc/mounts" processing. * Log an error if a line in "/proc/mounts" is skipped because no subvolspec was obtained for the btrfs volume entry. * Use named constant MAIN_VOLUME_ID instead of 5 where appropriate.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/mounts.py | 93 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 22 deletions(-)
diff --git a/blivet/mounts.py b/blivet/mounts.py index 617ca21..a7763cf 100644 --- a/blivet/mounts.py +++ b/blivet/mounts.py @@ -21,10 +21,59 @@ # from collections import defaultdict from . import util +from .devicelibs import btrfs
import logging log = logging.getLogger("blivet")
+class _MountinfoCache(object): + """ Cache for info from /proc/self/mountinfo. Looks up the root of the + mount within the filesystem using a pair of mountpoint, mount + source as keys. + + This is a very simple helper class for MountsCache, and would + have to be altered for general purpose use. + + Note that only info for fstype btrfs is stored within the cache, + as MountsCache methods only require information for btrfs. + """ + + def __init__(self): + self._cache = None + + def _getCache(self): + """ Reads lines in /proc/self/mountinfo and builds a table. """ + cache = {} + + with open("/proc/self/mountinfo") as mountinfos: + for line in mountinfos: + fields = line.split() + separator_index = fields.index("-") + fstype = fields[separator_index + 1] + if not fstype.startswith("btrfs"): + continue + + root = fields[3] + mountpoint = fields[4] + devspec = fields[separator_index + 2] + cache[(devspec, mountpoint)] = root + + return cache + + def getRoot(self, devspec, mountpoint): + """ Retrieves the root of the mount within the filesystem + that corresponds to devspec and mountpoint. + + :param str devspec: device specification + :param str mountpoint: mountpoint + :rtype: str or NoneType + :returns: the root of the mount within the filesystem, if available + """ + if self._cache is None: + self._cache = self._getCache() + + return self._cache.get((devspec, mountpoint)) + class MountsCache(object): """ Cache object for system mountpoints; checks /proc/mounts and /proc/self/mountinfo for up-to-date information. @@ -40,7 +89,7 @@ def getMountpoints(self, devspec, subvolspec=None): :param devscpec: device specification, eg. "/dev/vda1" :type devspec: str :param subvolspec: btrfs subvolume specification, eg. ID or name - :type subvolspec: str + :type subvolspec: object (may be NoneType) :returns: list of mountpoints (path) :rtype: list of str or empty list
@@ -50,6 +99,9 @@ def getMountpoints(self, devspec, subvolspec=None): """ self._cacheCheck()
+ if subvolspec is not None: + subvolspec = str(subvolspec) + return self.mountpoints[(devspec, subvolspec)]
def isMountpoint(self, path): @@ -68,28 +120,25 @@ def _getActiveMounts(self): Refreshes self.mountpoints with current mountpoint information """ self.mountpoints = defaultdict(list) - for line in open("/proc/mounts").readlines(): - try: - (devspec, mountpoint, fstype, _rest) = line.split(None, 3) - except ValueError: - log.error("failed to parse /proc/mounts line: %s", line) - continue - - if fstype == "btrfs": - # get the subvol name from /proc/self/mountinfo - for line in open("/proc/self/mountinfo").readlines(): - fields = line.split() - _subvol = fields[3] - _mountpoint = fields[4] - _devspec = fields[9] - if _mountpoint == mountpoint and _devspec == devspec: - # empty _subvol[1:] means it is a top-level volume - subvolspec = _subvol[1:] or 5 - + mountinfo = _MountinfoCache() + + with open("/proc/mounts") as mounts: + for line in mounts: + try: + (devspec, mountpoint, fstype, _rest) = line.split(None, 3) + except ValueError: + log.error("failed to parse /proc/mounts line: %s", line) + continue + + if fstype == "btrfs": + root = mountinfo.getRoot(devspec, mountpoint) + if root is not None: + subvolspec = root[1:] or str(btrfs.MAIN_VOLUME_ID) self.mountpoints[(devspec, subvolspec)].append(mountpoint) - - else: - self.mountpoints[(devspec, None)].append(mountpoint) + else: + log.error("failed to obtain subvolspec for btrfs device %s at mountpoint %s", devspec, mountpoint) + else: + self.mountpoints[(devspec, None)].append(mountpoint)
def _cacheCheck(self): """ Computes the MD5 hash on /proc/mounts and updates the cache on change
From: mulhern amulhern@redhat.com
Set it from the MAIN_VOLUME_ID for a volume.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/blivet.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/blivet/blivet.py b/blivet/blivet.py index 0db833c..b89bef2 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -39,6 +39,7 @@ from .deviceaction import ActionDestroyFormat, ActionResizeDevice, ActionResizeFormat from .devicelibs.edd import get_edd_dict from .devicelibs.dasd import make_dasd_list, write_dasd_conf +from .devicelibs.btrfs import MAIN_VOLUME_ID from .errors import StorageError from .size import Size from .devicetree import DeviceTree @@ -1023,6 +1024,7 @@ def newBTRFS(self, *args, **kwargs): name = self.suggestContainerName(hostname=hostname) if "label" not in fmt_args: fmt_args["label"] = name + fmt_args['subvolspec'] = MAIN_VOLUME_ID
# discard fmt_type since it's btrfs always kwargs.pop("fmt_type", None)
You can set `fmt_args["subvolspec"] = name` in the subvol block just above the one you're changing here, just after we set `fmt_args["mountopts"]`.
Done.
The change here is that instead of setting the subvolspec in the BTRFS format constructor I set the subvolspec for a volume in the newBTRFS method. I believe that subvolume devices created by newBTRFS will not have their subvolspecs set properly, but I believe that that will prove significantly less serious than not having the parent subvolspec set properly.
It is not absolutely clear to me how to set the subvolspec for newly created subvolumes properly.
Added label: ACK.
anaconda-patches@lists.fedorahosted.org