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/86577ee3883836c1c4fff258c05261bd3858e22b
>
> 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.
Thanks,
> ? Mykola Ivanets
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org