From e686959b4356b99791bf51a843a476e956de78c0 Mon Sep 17 00:00:00 2001 From: David Lehman Date: Wed, 18 Jul 2012 17:09:31 -0500 Subject: [PATCH 3/6] Fix handling of existing btrfs subvolumes. Some of the highlights: We weren't setting self.originalFormat correctly when implicitly setting the btrfs format. Also, we didn't have any code to discern subvolumes when parsing /etc/fstab. We were passing volume id as volume path/name when populating the device tree. Lastly, we were clobbering the subvol= mount option when mounting a btrfs subvolume to check it for existing roots. --- pyanaconda/storage/__init__.py | 12 +++++++----- pyanaconda/storage/devices.py | 35 ++++++++++++++++++++++++----------- pyanaconda/storage/devicetree.py | 32 +++++++++++++++++++++++++++++--- pyanaconda/storage/formats/fs.py | 4 ++++ 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 0d92b81..b37794a 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -2701,8 +2701,9 @@ def findExistingInstallations(devicetree): log.warning("setup of %s failed: %s" % (device.name, e)) continue + options = device.format.options + ",ro" try: - device.format.mount(options="ro", mountpoint=ROOT_PATH) + device.format.mount(options=options, mountpoint=ROOT_PATH) except Exception as e: log.warning("mount of %s as %s failed: %s" % (device.name, device.format.type, @@ -2784,17 +2785,18 @@ def parseFSTab(devicetree, chroot=None): for line in f.readlines(): # strip off comments (line, pound, comment) = line.partition("#") - fields = line.split(None, 3) + fields = line.split(None, 4) - if len(fields) < 4: + if len(fields) < 5: continue - (devspec, mountpoint, fstype, rest) = fields + (devspec, mountpoint, fstype, options, rest) = fields # find device in the tree device = devicetree.resolveDevice(devspec, cryptTab=cryptTab, - blkidTab=blkidTab) + blkidTab=blkidTab, + options=options) if device is None: continue diff --git a/pyanaconda/storage/devices.py b/pyanaconda/storage/devices.py index 6b979e8..8c2713b 100644 --- a/pyanaconda/storage/devices.py +++ b/pyanaconda/storage/devices.py @@ -3943,19 +3943,31 @@ class BTRFSDevice(StorageDevice): def _temp_dir_prefix(self): return "btrfs-tmp.%s" % self.id - def _do_temp_mount(self): + def _do_temp_mount(self, orig=False): if self.format.status or not self.exists: return tmpdir = tempfile.mkdtemp(prefix=self._temp_dir_prefix) - self.format.mount(mountpoint=tmpdir) + if orig: + fmt = self.originalFormat + else: + fmt = self.format + + fmt.mount(mountpoint=tmpdir) def _undo_temp_mount(self): - if self.format.status: - mountpoint = self.format._mountpoint - if os.path.basename(mountpoint).startswith(self._temp_dir_prefix): - self.format.unmount() - os.rmdir(mountpoint) + if getattr(self.format, "_mountpoint", None): + fmt = self.format + elif getattr(self.originalFormat, "_mountpoint", None): + fmt = self.originalFormat + else: + return + + mountpoint = fmt._mountpoint + + if os.path.basename(mountpoint).startswith(self._temp_dir_prefix): + fmt.unmount() + os.rmdir(mountpoint) @property def path(self): @@ -3990,6 +4002,7 @@ class BTRFSVolumeDevice(BTRFSDevice): label=label, volUUID=self.uuid, device=self.path) + self.originalFormat = self.format label = getattr(self.format, "label", None) if label: @@ -4058,7 +4071,7 @@ class BTRFSVolumeDevice(BTRFSDevice): subvols = [] self.setup(orig=True) try: - self._do_temp_mount() + self._do_temp_mount(orig=True) except FSError as e: log.debug("btrfs temp mount failed: %s" % e) return subvols @@ -4127,10 +4140,10 @@ class BTRFSSubVolumeDevice(BTRFSDevice): def _destroy(self): log_method_call(self, self.name, status=self.status) - self.volume._do_temp_mount() - mountpoint = self.volume.format._mountpoint + self.volume._do_temp_mount(orig=True) + mountpoint = self.volume.originalFormat._mountpoint if not mountpoint: raise RuntimeError("btrfs subvol destroy requires mounted volume") btrfs.delete_subvolume(mountpoint, self.name) - self.volume._removeSubVolume() + self.volume._removeSubVolume(self.name) self.volume._undo_temp_mount() diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index ae69be2..cd45cf6 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -1529,11 +1529,12 @@ class DeviceTree(object): if vol_path in [sv.name for sv in btrfs_dev.subvolumes]: continue fmt = getFormat("btrfs", device=btrfs_dev.path, exists=True, - mountopts="subvol=%d" % vol_id) + mountopts="subvol=%s" % vol_path) subvol = BTRFSSubVolumeDevice(vol_path, vol_id=vol_id, format=fmt, - parents=[btrfs_dev]) + parents=[btrfs_dev], + exists=True) self._addDevice(subvol) def handleUdevDeviceFormat(self, info, device): @@ -2148,7 +2149,7 @@ class DeviceTree(object): """ Return a list of a device's children. """ return [c for c in self._devices if device in c.parents] - def resolveDevice(self, devspec, blkidTab=None, cryptTab=None): + def resolveDevice(self, devspec, blkidTab=None, cryptTab=None, options=None): # find device in the tree device = None if devspec.startswith("UUID="): @@ -2232,6 +2233,31 @@ class DeviceTree(object): lv = "%s-%s" % (vg_name, lv_name) device = self.getDeviceByName(lv) + def get_opt_val(opt_name, options): + for opt in options.split(","): + if "=" not in opt: + continue + + name, val = opt.split("=") + if name == opt_name: + return val.strip() + + # check mount options for btrfs volumes in case it's a subvol + if device and device.type == "btrfs volume" and options: + attr = None + if "subvol=" in options: + attr = "name" + val = get_opt_val("subvol", options) + elif "subvolid=" in options: + attr = "vol_id" + val = get_opt_val("subvolid", options) + + if attr and val: + for subvol in device.subvolumes: + if getattr(subvol, attr, None) == val: + device = subvol + break + if device: log.debug("resolved '%s' to '%s' (%s)" % (devspec, device.name, device.type)) else: diff --git a/pyanaconda/storage/formats/fs.py b/pyanaconda/storage/formats/fs.py index 248dd9c..f94606b 100644 --- a/pyanaconda/storage/formats/fs.py +++ b/pyanaconda/storage/formats/fs.py @@ -1123,6 +1123,10 @@ class BTRFS(FS): # filesystem creation is done in storage.devicelibs.btrfs.create_volume pass + def destroy(self, *args, **kwargs): + # filesystem creation is done in storage.devicelibs.btrfs.delete_volume + pass + def setup(self, *args, **kwargs): log_method_call(self, type=self.mountType, device=self.device, mountpoint=self.mountpoint) -- 1.7.7.6