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 or from the name for a subvolume.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/blivet.py | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/blivet/blivet.py b/blivet/blivet.py index 0db833c..4568565 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 @@ -1010,6 +1011,7 @@ def newBTRFS(self, *args, **kwargs): # already in use within the parent volume name = self.suggestDeviceName(mountpoint=mountpoint) fmt_args["mountopts"] = "subvol=%s" % name + fmt_args["subvolspec"] = name kwargs.pop("metaDataLevel", None) kwargs.pop("dataLevel", None) else: @@ -1023,6 +1025,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)
anaconda-patches@lists.fedorahosted.org