Re: [Libguestfs] Regression with LDM devices

On Mon, Mar 28, 2022 at 01:25:42PM +0300, Nikolay Ivanets wrote: > Hello > > This commit caused a regression with LDM devices: > https://github.com/libguestfs/libguestfs/commit/86577ee3883836c1c4fff258c... > > 1. list-filesystems double all LDM volumes: > >> <fs> list-filesystems > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5: ntfs > /dev/sda1: ntfs > /dev/sda2: vfat > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5: ntfs > > 2. inspect-os in this case detects 2 OSes: > >> <fs> inspect-os > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5 > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5 > The issue is that now we explicitly adds DM devices to a list of > file systems, but latter we also selectively adds LDM volumes. Which > also DM devices: >> <fs> list-ldm-volumes > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5 > >> <fs> list-dm-devices > /dev/mapper/ldm_vol_AQ-W10-EFI-VM-Dg0_Volume5 I can understand why this would happen ... From a historical point of view, list-filesystems wasn't very well specified or implemented. In the simple case where you just have whole block devices, partitions and LVs, it means something like "examine every block device, partition, and LV, and return the filesystems found", with some additional hot spicy sauce on top for ignoring a block device if it's partitioned (or a partition if it contains a PV etc). The implementation worked only in simple cases and we've layered more hacks on top since then. I wonder if what we really want to do is something like: - for each { block device, partition, LV, LDM, etc } - examine it for a filesystem, if found, add to list - deduplicate the final list In particular we'd get rid of functions like "is_not_partitioned_device". There's some danger that when examining (eg) a block device, the tools we use for finding filesystems are not very accurate and could find a filesystem where it's actually located on a contained structure. I think the main (only?) danger would be with RAID0/1 devices? The more fundamental problem is that we don't have a good idea of what devices contain other devices (partly because Linux itself doesn't have a good idea of this concept, and partly because the problem doesn't really make sense since devices can contain other devices in quite arbitrary ways). I'm just thinking out loud. > It would be easy to fix leaving only unique file systems eliminating > duplicates, It would fix this problem, but ... > however it would not help if LDM contains volumes of > type other than simple. Such a volumes consist of LDM partitions > which are also DM devices and thus the mentioned patch will include > them into a list if file systems but LDM partitions cannot contains > file system. Here you can see an example of both cases where > list-filesystems returns doubled LDM volumes as well as LDM > partitions. Later cannot contains file system: ... I guess this is like the RAID example? >> <fs> list-filesystems > /dev/mapper/ldm_part_WIN-RSVUTBNGR5D-Dg0_Disk1-01: unknown > /dev/mapper/ldm_part_WIN-RSVUTBNGR5D-Dg0_Disk1-02: ntfs > /dev/mapper/ldm_part_WIN-RSVUTBNGR5D-Dg0_Disk2-01: unknown > /dev/mapper/ldm_part_WIN-RSVUTBNGR5D-Dg0_Disk2-02: ntfs > /dev/mapper/ldm_part_WIN-RSVUTBNGR5D-Dg0_Disk3-01: unknown > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume1: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume10: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume11: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume2: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume3: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume4: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume5: vfat > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume6: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume7: vfat > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume8: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume9: unknown > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume1: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume10: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume11: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume2: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume3: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume4: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume5: vfat > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume6: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume7: vfat > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume8: ntfs > /dev/mapper/ldm_vol_WIN-RSVUTBNGR5D-Dg0_Volume9: unknown I think for this particular bug we could (and in fact, should) just add a deduplication step at the end. Dead easy patch. However, longer term this just adds another hack on the pile of hacks, and I'm not sure how to fix this properly, if indeed that is possible at all. Maybe Laszlo has some ideas here.

><fs> list-dm-devices /dev/mapper/luks-0d619854-ccd5-43b1-8883-991fec5ef713 /dev/mapper/luks-4e9e7a6f-a68c-42fd-92b4-8f4f2579a389