Patches 1 and 2 are slightly modified from the versions I posted last week. The main change is introducing the MAIN_VOLUME_ID constant in the first patch instead of the second.
Patches 1, 3, and 5 together add handling for various btrfs craziness like nested subvolumes and setting a mountpoint for the main volume.
Patch 4 fixes a bug when editing the btrfs autopart layout in the custom spoke. I can't think of any regressions it might cause, and have added that area to my list of unit test ideas.
Subvolumes are nested. That is to say that removing subvolume foo will also remove subvolumes foo/bar and foo/bar/baz. --- blivet/devicelibs/btrfs.py | 12 +++++++++--- blivet/devices.py | 14 ++++++++++++-- blivet/devicetree.py | 17 ++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/blivet/devicelibs/btrfs.py b/blivet/devicelibs/btrfs.py index 2f0cee4..bde165d 100644 --- a/blivet/devicelibs/btrfs.py +++ b/blivet/devicelibs/btrfs.py @@ -32,6 +32,9 @@ _ = lambda x: gettext.ldgettext("blivet", x) import logging log = logging.getLogger("blivet")
+# this is the volume id btrfs always assigns to the top-level volume/tree +MAIN_VOLUME_ID = 5 + def btrfs(args, capture=False): if capture: exec_func = util.capture_output @@ -98,15 +101,18 @@ def create_snapshot(source, dest): def scan_device(path): return btrfs(["device", "scan", path])
+_subvol_list_regex = r'ID (\d+) gen \d+ parent (\d+) top level \d+ path (.+)\n' + # get a list of subvolumes from a mounted btrfs filesystem def list_subvolumes(mountpoint): if not os.path.ismount(mountpoint): raise ValueError("volume not mounted")
- args = ["subvol", "list", mountpoint] + args = ["subvol", "list", "-p", mountpoint] buf = btrfs(args, capture=True) vols = [] - for group in re.findall(r'ID (\d+) gen \d+ top level \d+ path (.+)\n', buf): - vols.append({"id": int(group[0]), "path": group[1]}) + for group in re.findall(_subvol_list_regex, buf): + vols.append({"id": int(group[0]), "parent": int(group[1]), + "path": group[2]})
return vols diff --git a/blivet/devices.py b/blivet/devices.py index 54ece22..82f1872 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -4346,6 +4346,7 @@ class BTRFSDevice(StorageDevice):
class BTRFSVolumeDevice(BTRFSDevice): _type = "btrfs volume" + vol_id = btrfs.MAIN_VOLUME_ID
def __init__(self, *args, **kwargs): self.dataLevel = kwargs.pop("dataLevel", None) @@ -4373,7 +4374,7 @@ class BTRFSVolumeDevice(BTRFSDevice): label=label, volUUID=self.uuid, device=self.path, - mountopts="subvolid=0") + mountopts="subvolid=%d" % self.vol_id) self.originalFormat = copy.copy(self.format)
def _setFormat(self, format): @@ -4529,7 +4530,16 @@ class BTRFSSubVolumeDevice(BTRFSDevice):
@property def volume(self): - return self.parents[0] + parent = self.parents[0] + vol = None + while True: + if not isinstance(parent, BTRFSSubVolumeDevice): + vol = parent + break + + parent = parent.parents[0] + + return vol
def setupParents(self, orig=False): """ Run setup method of all parent devices. """ diff --git a/blivet/devicetree.py b/blivet/devicetree.py index bad135c..8ab1e5f 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1590,14 +1590,29 @@ class DeviceTree(object): for subvol_dict in btrfs_dev.listSubVolumes(): vol_id = subvol_dict["id"] vol_path = subvol_dict["path"] + parent_id = subvol_dict["parent"] if vol_path in [sv.name for sv in btrfs_dev.subvolumes]: continue + + # look up the parent subvol + parent = None + subvols = [btrfs_dev] + btrfs_dev.subvolumes + for sv in subvols: + if sv.vol_id == parent_id: + parent = sv + break + + if parent is None: + log.error("failed to find parent (%d) for subvol %s" + % (parent_id, vol_path)) + raise DeviceTreeError("could not find parent for subvol") + fmt = getFormat("btrfs", device=btrfs_dev.path, exists=True, mountopts="subvol=%s" % vol_path) subvol = BTRFSSubVolumeDevice(vol_path, vol_id=vol_id, format=fmt, - parents=[btrfs_dev], + parents=[parent], exists=True) self._addDevice(subvol)
On Fri, 2013-11-22 at 10:00 -0600, David Lehman wrote:
Subvolumes are nested. That is to say that removing subvolume foo will also remove subvolumes foo/bar and foo/bar/baz.
blivet/devicelibs/btrfs.py | 12 +++++++++--- blivet/devices.py | 14 ++++++++++++-- blivet/devicetree.py | 17 ++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/blivet/devicelibs/btrfs.py b/blivet/devicelibs/btrfs.py index 2f0cee4..bde165d 100644 --- a/blivet/devicelibs/btrfs.py +++ b/blivet/devicelibs/btrfs.py @@ -32,6 +32,9 @@ _ = lambda x: gettext.ldgettext("blivet", x) import logging log = logging.getLogger("blivet")
+# this is the volume id btrfs always assigns to the top-level volume/tree +MAIN_VOLUME_ID = 5
def btrfs(args, capture=False): if capture: exec_func = util.capture_output @@ -98,15 +101,18 @@ def create_snapshot(source, dest): def scan_device(path): return btrfs(["device", "scan", path])
+_subvol_list_regex = r'ID (\d+) gen \d+ parent (\d+) top level \d+ path (.+)\n'
I think this should be all uppercase as a constant and compiled.
--- blivet/devicelibs/btrfs.py | 14 ++++++++++++++ blivet/devices.py | 27 +++++++++++++++++++++++++++ blivet/devicetree.py | 3 +++ 3 files changed, 44 insertions(+)
diff --git a/blivet/devicelibs/btrfs.py b/blivet/devicelibs/btrfs.py index bde165d..531c294 100644 --- a/blivet/devicelibs/btrfs.py +++ b/blivet/devicelibs/btrfs.py @@ -116,3 +116,17 @@ def list_subvolumes(mountpoint): "path": group[2]})
return vols + +def get_default_subvolume(mountpoint): + if not os.path.ismount(mountpoint): + raise ValueError("volume not mounted") + + args = ["subvol", "get-default", mountpoint] + buf = btrfs(args, capture=True) + m = re.match(r'ID (\d+) .*', buf) + try: + default = int(m.groups()[0]) + except IndexError: + raise BTRFSError("failed to get default subvolume from %s" % mountpoint) + + return default diff --git a/blivet/devices.py b/blivet/devices.py index 82f1872..4b71b4c 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -4482,6 +4482,8 @@ class BTRFSVolumeDevice(BTRFSDevice): subvols = btrfs.list_subvolumes(self.originalFormat._mountpoint) except BTRFSError as e: log.debug("failed to list subvolumes: %s" % e) + else: + self._getDefaultSubVolumeID() finally: self._undo_temp_mount()
@@ -4498,6 +4500,31 @@ class BTRFSVolumeDevice(BTRFSDevice): def removeSubVolume(self, name): raise NotImplementedError()
+ def _getDefaultSubVolumeID(self): + subvolid = None + try: + subvolid = btrfs.get_default_subvolume(self.originalFormat._mountpoint) + except BTRFSError as e: + log.debug("failed to get default subvolume id: %s" % e) + + self.defaultSubVolumeID = subvolid + + @property + def defaultSubVolume(self): + default = None + if self.defaultSubVolumeID is None: + return None + + if self.defaultSubVolumeID == self.vol_id: + return self + + for sv in self.subvolumes: + if sv.vol_id == self.defaultSubVolumeID: + default = sv + break + + return default + def _create(self): log_method_call(self, self.name, status=self.status) btrfs.create_volume(devices=[d.path for d in self.parents], diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 8ab1e5f..df0bd0c 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -2361,6 +2361,9 @@ class DeviceTree(object): elif "subvolid=" in options: attr = "vol_id" val = util.get_option_value("subvolid", options) + elif device.defaultSubVolume: + # default subvolume + device = device.defaultSubVolume
if attr and val: for subvol in device.subvolumes:
Normally the volume is a container, but it is possible that it has a mountpoint associated with it, which means it could be passed as the device in that case.
Related: rhbz#1016959 --- blivet/devicefactory.py | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py index 4c95709..50362c1 100644 --- a/blivet/devicefactory.py +++ b/blivet/devicefactory.py @@ -404,6 +404,8 @@ class DeviceFactory(object): container = device.vg elif hasattr(device, "volume"): container = device.volume + elif hasattr(device, "subvolumes"): + container = device elif name: for c in self.storage.devices: if c.name == name and c in self.container_list: @@ -1641,3 +1643,12 @@ class BTRFSFactory(DeviceFactory): def _set_name(self): super(BTRFSFactory, self)._set_name() self.device.format.options = "subvol=" + self.device.name + + def _reconfigure_device(self): + if self.device == self.container: + # This is a btrfs volume -- the only thing not handled already is + # updating the mountpoint. + self.device.format.mountpoint = self.mountpoint + return + + super(BTRFSFactory, self)._reconfigure_device()
The previous method of summing the subvolumes' req_size doesn't work when the initial layout is a product of autopart since the base sizes do not match the actual sizes due to growth. --- blivet/devicefactory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py index 50362c1..8aad76e 100644 --- a/blivet/devicefactory.py +++ b/blivet/devicefactory.py @@ -1584,7 +1584,7 @@ class BTRFSFactory(DeviceFactory): # automatic if self.container and not self.device: # For new subvols the size is in addition to the volume's size. - size += sum(s.req_size for s in self.container.subvolumes) + size += self.container.size
size += self._get_device_space() elif self.container_size == SIZE_POLICY_MAX:
btrfs allows mounting of non-leaf devices, which is unusual.
This could be a top-level volume or a subvolume that contains other subvolumes.
The volume could have a mountpoint in a previous installation or it could have one assigned via kickstart, so we must be able to handle showing them.
Since the btrfs volume is treated as a container the only configuration allowed on one is setting a mountpoint or running the modify container dialog. --- pyanaconda/ui/gui/spokes/custom.py | 46 ++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 1950a5b..f19da80 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -316,7 +316,7 @@ class ConfirmDeleteDialog(GUIObject): self.window.destroy()
# pylint: disable-msg=W0221 - def refresh(self, mountpoint, device, rootName): + def refresh(self, mountpoint, device, rootName, subvols=False): GUIObject.refresh(self) label = self.builder.get_object("confirmLabel")
@@ -331,7 +331,10 @@ class ConfirmDeleteDialog(GUIObject): else: txt = device
- label_text = _("Are you sure you want to delete all of the data on %s?") % txt + if not subvols: + label_text = _("Are you sure you want to delete all of the data on %s?") % txt + else: + label_text = _("Are you sure you want to delete all of the data on %s, including subvolumes and/or snapshots?") % txt
label.set_text(label_text)
@@ -796,7 +799,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): def unusedDevices(self): unused_devices = [d for d in self.__storage.unusedDevices if d.disks and d.mediaPresent and - not d.partitioned and d.isleaf] + not d.partitioned and + (d.isleaf or d.type.startswith("btrfs"))] # add incomplete VGs and MDs incomplete = [d for d in self.__storage.devicetree._devices if not getattr(d, "complete", True)] @@ -909,7 +913,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): @property def new_devices(self): # A device scheduled for formatting only belongs in the new root. - new_devices = [d for d in self._devices if d.isleaf and + new_devices = [d for d in self._devices if (d.isleaf or + d.type.startswith("btrfs")) + and not d.format.exists and not d.partitioned]
@@ -1261,6 +1267,16 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._populate_right_side(selector) return
+ # If the device is a btrfs volume, the only things we can set/update + # are mountpoint and container-wide settings. + if device_type == DEVICE_TYPE_BTRFS and hasattr(use_dev, "subvolumes"): + size = 0 + changed_size = False + encrypted = False + changed_encryption = False + raid_level = None + changed_raid_level = False + with ui_storage_logger(): # create a new factory using the appropriate size and type factory = devicefactory.get_device_factory(self.__storage, @@ -1659,11 +1675,12 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._device_container_raid_level = get_raid_level(use_dev.vg) self._device_container_encrypted = use_dev.vg.encrypted self._device_container_size = use_dev.vg.size_policy - elif hasattr(use_dev, "volume"): - self._device_container_name = use_dev.volume.name - self._device_container_raid_level = get_raid_level(use_dev.volume) - self._device_container_encrypted = use_dev.volume.encrypted - self._device_container_size = use_dev.volume.size_policy + elif hasattr(use_dev, "volume") or hasattr(use_dev, "subvolumes"): + volume = getattr(use_dev, "volume", use_dev) + self._device_container_name = volume.name + self._device_container_raid_level = get_raid_level(volume) + self._device_container_encrypted = volume.encrypted + self._device_container_size = volume.size_policy else: self._device_container_name = None self._device_container_raid_level = None @@ -1870,7 +1887,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._populate_raid(raid_level) self._populate_container(device=use_dev) # do this last in case this was set sensitive in on_device_type_changed - if use_dev.exists: + if use_dev.exists or use_dev.type == "btrfs volume": fancy_set_sensitive(self._nameEntry, False)
### @@ -2075,6 +2092,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): try: if device.isDisk: self.__storage.initializeDisk(device) + elif device.type.startswith("btrfs") and not device.isleaf: + self.__storage.recursiveRemove(device) else: self.__storage.destroyDevice(device) except StorageError as e: @@ -2188,8 +2207,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): # schedule actions to delete the thing. dialog = ConfirmDeleteDialog(self.data) with enlightbox(self.window, dialog.window): + subvols = (device.type.startswith("btrfs") and + not device.isleaf) dialog.refresh(getattr(device.format, "mountpoint", ""), - device.name, root_name) + device.name, root_name, subvols=subvols) rc = dialog.run()
if rc == 0: @@ -2665,7 +2686,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): map(really_show, [self._containerLabel, self._containerCombo, self._modifyContainerButton])
# make the combo and button insensitive for existing LVs - can_change_container = (device is not None and not device.exists) + can_change_container = (device is not None and not device.exists and + device != container) fancy_set_sensitive(self._containerCombo, can_change_container) container_exists = getattr(container, "exists", False) self._modifyContainerButton.set_sensitive(not container_exists)
On Fri, 2013-11-22 at 10:00 -0600, David Lehman wrote:
Patches 1 and 2 are slightly modified from the versions I posted last week. The main change is introducing the MAIN_VOLUME_ID constant in the first patch instead of the second.
Patches 1, 3, and 5 together add handling for various btrfs craziness like nested subvolumes and setting a mountpoint for the main volume.
Patch 4 fixes a bug when editing the btrfs autopart layout in the custom spoke. I can't think of any regressions it might cause, and have added that area to my list of unit test ideas.
Apart from the one minor comment this looks good to me.
anaconda-patches@lists.fedorahosted.org