This is a follow-up on this discussion https://github.com/yast/yast-storage-ng/pull/184#pullrequestreview-29672717
The mail is mainly targeted to Josef, so we continue the discussion in a more persistent and prominent place than a merged pull request. Don't feel guilty if you decide to ignore it. :-) Before introducing the Y2Storage wrapper classes on top of the Storage ones, we had a query API (already discussed in this list) based on the concept of Y2Storage::DevicesLists. I think we don't need that anymore with the wrapper. I would go for an approach based on pure Ruby arrays, now that we don't have to fear not-found exceptions or to care about downcasting. I would simply add the following convenience methods to Y2Storage::Device and that's it. https://github.com/yast/yast-storage-ng/pull/184/commits/0f733850786242cddf79c7c3f8a733428d8c5674 So, how would we query the devicegraph? Let's explain it by example. Most examples are based on real cases I have seen while porting code in other packages... others are there for the sake of the challenge ;) 1) The filesystem, encrypted or not, in a partition. That is, we want the filesystem and we don't care whether there is an encryption (LUKS) layer sitting between the partition and that filesystem. 2) The filesystem directly placed in a partition, nil if no filesystem or if the filesystem is not directly there (e.g. there is an encryption layer in between). 3) The encrypted filesystem in a partition, nil if the partition is not formatted or if its directly formatted (without the intermediate encryption layer) 4) The disk that match a disk name or partition name, only if it's GPT 5) Filesystems (encrypted or not) on logical partitions of a given disk 6) Non-encrypted filesystem on logical partitions of a given disk 7) Logical partitions for the disk that match a disk name or contains partition name. 8) From a given set of volume groups, physical volumes that are not directly into a disk (or an encrypted disk) (e.g. they sit in a partition or a RAID device) 9) Checking if a given partition contains a btrfs filesystem (encrypted or not) 10) Disks hosting a given filesystem either directly or through one of its partitions. "Disks" in plural because in our devicegraphs, a filesystem can be lay across several block devices. 11) Disks "supporting" the existence of a given filesystem, for whatever technology is used. For example, if the filesystem is in a LV (LVM) and that LV sits on a VG, we want all the disks hosting the PVs of that VG, either directly or through partitions or RAID devices (yes, this is one of the examples based on real code). 12) Disks of a filesystem, only if the plain disk is directly formatted (no partitions, RAID or encryption involved). Otherwise nil 13) Disks of a filesystem, only if the disk is directly formatted (no partitions), nil otherwise, but supporting encrypted filesystems. 14) Devices with a extX filesystem (encrypted or not) 15) Partitions with a extX filesystem (encrypted or not) How would we do that just with the wrapper and plain arrays? 1) the_partition.blk_filesystem 2) the_partition.direct_blk_filesystem 3) the_partition.encrypted? ? the_partition.blk_filesystem : nil 4) disk = devgraph.disks.detect {|d| d.name_or_partition?(the_name) } (disk && disk.gpt?) ? disk : nil # or disk = Y2Storage::Disk.find_by_name_or_partition(devgraph, the_name) (disk && disk.gpt?) ? disk : nil 5) parts = the_disk.partitions.select {|p| p.type.is?(:logical) } parts.map {|p| p.blk_filesystem }.compact 6) parts = the_disk.partitions.select {|p| p.type.is?(:logical) } parts.map {|p| p.direct_blk_filesystem }.compact 7) disk = devgraph.disks.detect {|d| d.name_or_partition?(the_name) } disk.partitions.select {|p| p.type.is?(:logical) } 8) all_pvs = the_vol_groups.map(&:lvm_pvs).flatten all_pvs.reject { |pv| pv.blk_device.plain_device.disk? } 9) !!(partition.filesystem && partition.filesystem.type.is?(:btrfs)) 10) # See notes below devgraph.disks.select do |d| d.filesystem == the_filesystem || d.partitions.any? {|p| p.filesystem == the_filesystem } end # Beware, this is not equivalent because it also returns filesystems in # partitions of a software RAID or any other partitionable device that # is not a disk the_filesystem.ancestors.select do |a| (a.disk? || a.partition?) && a.filesystem == the_filesystem end 11) the_filesystem.ancestors.select(&:disk?).uniq 12) filesystem.blk_devices.select(&:disk?) 13) filesystem.plain_blk_devices.select(&:disk?) 14) all_ext = devgraph.blk_filesystems.select {|fs| fs.type.is?(:ext) } all_ext.map {|fs| fs.plain_blk_devices }.flatten 15) devgraph.partitions.select do |p| p.blk_filesystem && p.blk_filesystem.type.is?(:ext) end Feel free to provide more "challenges" and/or test alternative API against the current ones. And yes, before you ask, it would make sense to add this to BlkDevice alias_method :filesystem, :blk_filesystem alias_method :direct_filesystem, :direct_blk_filesystem Moreover, if we find ourselves too often checking for something in a disk and all its partitions, we could add a convenience method Y2Storage::Partitionable#this_and_partitions. Then some examples would change a bit, like: 10) devgraph.disks.select do |d| d.this_and_partitions.any? {|i| i.filesystem == the_filesystem } end Looking forward alternatives and/or feedback. Cheers. -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
