Instead of using part_to_dev to find such devices we open the device's directory under /sys/block/<dev_name> and look for entries starting with <dev_name>, eg. /sys/block/sda/sda1. --- daemon/listfs.ml | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/daemon/listfs.ml b/daemon/listfs.ml index 56ebadeda..eced55bce 100644 --- a/daemon/listfs.ml +++ b/daemon/listfs.ml @@ -24,31 +24,15 @@ let rec list_filesystems () = let has_lvm2 = Optgroups.lvm2_available () in let has_ldm = Optgroups.ldm_available () in + (* Devices. *) let devices = Devsparts.list_devices () in - let partitions = Devsparts.list_partitions () in - let mds = Md.list_md_devices () in - - (* Look to see if any devices directly contain filesystems - * (RHBZ#590167). However vfs-type will fail to tell us anything - * useful about devices which just contain partitions, so we also - * get the list of partitions and exclude the corresponding devices - * by using part-to-dev. - *) - let devices_containing_partitions = List.fold_left ( - fun set part -> - StringSet.add (Devsparts.part_to_dev part) set - ) StringSet.empty partitions in - let devices = List.filter ( - fun dev -> - not (StringSet.mem dev devices_containing_partitions) - ) devices in - - (* Use vfs-type to check for filesystems on devices. *) + let devices = List.filter is_not_partitioned_device devices in let ret = List.filter_map check_with_vfs_type devices in (* Use vfs-type to check for filesystems on partitions, but * ignore MBR partition type 42 used by LDM. *) + let partitions = Devsparts.list_partitions () in let ret = ret @ List.filter_map ( @@ -60,6 +44,7 @@ let rec list_filesystems () = ) partitions in (* Use vfs-type to check for filesystems on md devices. *) + let mds = Md.list_md_devices () in let ret = ret @ List.filter_map check_with_vfs_type mds in (* LVM. *) @@ -85,6 +70,24 @@ let rec list_filesystems () = List.flatten ret +(* Look to see if device can directly contain filesystem (RHBZ#590167). + * Partitioned devices cannot contain filesystem, so we will exclude + * such devices. + *) +and is_not_partitioned_device device = + assert (String.is_prefix device "/dev/"); + let dev_name = String.sub device 5 (String.length device - 5) in + let dev_dir = "/sys/block/" ^ dev_name in + + (* Open the device's directory under /sys/block/<dev_name> and + * look for entries starting with <dev_name>, eg. /sys/block/sda/sda1 + *) + let is_device_partition file = String.is_prefix file dev_name in + let files = Array.to_list (Sys.readdir dev_dir) in + let has_partition = List.exists is_device_partition files in + + not has_partition + (* Use vfs-type to check for a filesystem of some sort of [device]. * Returns [Some [device, vfs_type; ...]] if found (there may be * multiple devices found in the case of btrfs), else [None] if nothing -- 2.17.0

Test guestfish list-filesystems command finds file system on partitioned md device and doesn't take into account md device itself (similar to as physical devices are filtered out if they are partitioned). --- tests/md/Makefile.am | 3 +- tests/md/test-partitioned-md-devices.sh | 79 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100755 tests/md/test-partitioned-md-devices.sh diff --git a/tests/md/Makefile.am b/tests/md/Makefile.am index 42260af58..2e487568a 100644 --- a/tests/md/Makefile.am +++ b/tests/md/Makefile.am @@ -24,7 +24,8 @@ TESTS = \ test-list-md-devices.sh \ test-lvm-on-md-device.sh \ test-md-and-lvm-devices.sh \ - test-mdadm.sh + test-mdadm.sh \ + test-partitioned-md-devices.sh TESTS_ENVIRONMENT = $(top_builddir)/run --test diff --git a/tests/md/test-partitioned-md-devices.sh b/tests/md/test-partitioned-md-devices.sh new file mode 100755 index 000000000..36b6d8a25 --- /dev/null +++ b/tests/md/test-partitioned-md-devices.sh @@ -0,0 +1,79 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2018 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test guestfish list-filesystems command finds file system on partitioned +# md device and does't take into account md device itself (similar to as +# physical devices are skipped if they are partitioned) + +set -e + +$TEST_FUNCTIONS +skip_if_skipped + +disk1=list-filesystems2-1.img +disk2=list-filesystems2-2.img + +rm -f $disk1 $disk2 + +# Clean up if the script is killed or exits early +cleanup () +{ + status=$? + set +e + + # Don't delete the output files if non-zero exit + if [ "$status" -eq 0 ]; then rm -f $disk1 $disk2; fi + + exit $status +} +trap cleanup INT QUIT TERM EXIT + +output=$( +guestfish <<EOF +# Add 2 empty disks +sparse $disk1 50M +sparse $disk2 50M +run + +# Create a raid0 based on the 2 disks +md-create test "/dev/sda /dev/sdb" level:raid0 + +part-init /dev/md127 mbr +part-add /dev/md127 p 64 41023 +part-add /dev/md127 p 41024 81983 + +# Create filesystems +mkfs ext3 /dev/md127p1 +mkfs ext4 /dev/md127p2 + +list-filesystems +EOF +) + +expected="/dev/md127p1: ext3 +/dev/md127p2: ext4" + +if [ "$output" != "$expected" ]; then + echo "$0: error: actual output did not match expected output" + echo -e "actual:\n$output" + echo -e "expected:\n$expected" + exit 1 +fi + +# cleanup() is called implicitly which cleans up everything +exit 0 -- 2.17.0

Filter out Microsoft Reserved Partition and Windows Snapshot Partition. --- daemon/listfs.ml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/daemon/listfs.ml b/daemon/listfs.ml index a1ee38de3..57f2f622d 100644 --- a/daemon/listfs.ml +++ b/daemon/listfs.ml @@ -35,7 +35,7 @@ let rec list_filesystems () = (* Partitions. *) let partitions = Devsparts.list_partitions () in - let partitions = List.filter is_not_ldm_partition partitions in + let partitions = List.filter is_partition_can_hold_filesystem partitions in let ret = ret @ List.filter_map check_with_vfs_type partitions in (* MD. *) @@ -83,9 +83,10 @@ and is_not_partitioned_device device = * because these are members of a Windows dynamic disk group. Trying * to read them will cause errors (RHBZ#887520). Assuming that * libguestfs was compiled with ldm support, we'll get the filesystems - * on these later. + * on these later. We also ignore Microsoft Reserved Partition and + * Windows Snapshot Partition. *) -and is_not_ldm_partition partition = +and is_partition_can_hold_filesystem partition = let device = Devsparts.part_to_dev partition in let partnum = Devsparts.part_to_partnum partition in let parttype = Parted.part_get_parttype device in @@ -101,7 +102,11 @@ and is_not_ldm_partition partition = (* Windows Logical Disk Manager metadata partition. *) | "5808C8AA-7E8F-42E0-85D2-E1E90434CFB3" (* Windows Logical Disk Manager data partition. *) - | "AF9B60A0-1431-4F62-BC68-3311714A69AD" -> false + | "AF9B60A0-1431-4F62-BC68-3311714A69AD" + (* Microsoft Reserved Partition. *) + | "E3C9E316-0B5C-4DB8-817D-F92DF00215AE" + (* Windows Snapshot Partition. *) + | "CADDEBF1-4400-4DE8-B103-12117DCF3CCF" -> false | _ -> true ) else true -- 2.17.0