7:53 p.m.
This patch series:
1. Addresses comments from v5 series review
2. Large commit is splitted to more granular commits for better code review.
Mykola Ivanets (6):
daemon: Changing the way that we detect if a device contains
partitions.
daemon: list-filesystems: Ignore partitioned MD devices.
tests: list-filesystems command ignores partitioned MD devices.
daemon: list-filesystems: Change the way we filter out LDM partitions.
daemon: list-filesystems: Filter out Microsoft Reserved and Windows
Snapshot partitions.
daemon: list-ilesystems: Filter out MBR extended partitions.
Nikolay Ivanets (1):
daemon: Reimplement 'part_get_mbr_part_type' API in OCaml.
daemon/listfs.ml | 127 +++++++++++++-----------
daemon/parted.c | 106 --------------------
daemon/parted.ml | 13 +++
generator/actions_core.ml | 1 +
tests/md/Makefile.am | 3 +-
tests/md/test-partitioned-md-devices.sh | 79 +++++++++++++++
6 files changed, 166 insertions(+), 163 deletions(-)
create mode 100755 tests/md/test-partitioned-md-devices.sh
--
2.17.0
7:53 p.m.
New subject: [PATCH v6 1/7] daemon: Reimplement 'part_get_mbr_part_type' API in OCaml.
From: Nikolay Ivanets <stenavin(a)gmail.com>
Instead of parsing 'parted' output OCaml implementation relies on the following
facts:
1. MBR partition table can hold up to 4 "primary" partitions.
2. Partition with number greater then 4 is "logical" partition.
3. Partition with number less then or equal to 4 with MBR ID 0x05 or 0x0f is
"extended" partition (http://thestarman.pcministry.com/asm/mbr/PartTypes.htm;
https://en.wikipedia.org/wiki/Partition_type) or "primary" otherwise.
---
daemon/parted.c | 106 --------------------------------------
daemon/parted.ml | 13 +++++
generator/actions_core.ml | 1 +
3 files changed, 14 insertions(+), 106 deletions(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index e5435b5ea..d67c6c507 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -602,112 +602,6 @@ do_part_get_name (const char *device, int partnum)
}
}
-char *
-do_part_get_mbr_part_type (const char *device, int partnum)
-{
- CLEANUP_FREE char *parttype;
- char *part_type;
-
- parttype = do_part_get_parttype (device);
- if (parttype == NULL)
- return NULL;
-
- /* machine parseable output by 'parted -m' did not provide
- * partition type info.
- * Use traditional style.
- */
- CLEANUP_FREE char *out = print_partition_table (device, false);
- if (!out)
- return NULL;
-
- CLEANUP_FREE_STRING_LIST char **lines = split_lines (out);
-
- if (!lines)
- return NULL;
-
- size_t start = 0, end = 0, row;
-
- for (row = 0; lines[row] != NULL; ++row)
- if (STRPREFIX (lines[row], "Number")) {
- start = row + 1;
- break;
- }
-
- if (start == 0) {
- reply_with_error ("parted output has no \"Number\" line");
- return NULL;
- }
-
- for (row = start; lines[row] != NULL; ++row)
- if (STREQ (lines[row], "")) {
- end = row;
- break;
- }
-
- if (end == 0) {
- reply_with_error ("parted output has no blank after end of table");
- return NULL;
- }
-
- /* Now parse the lines. */
- size_t i;
- int64_t temp_int64;
- int part_num;
- char temp_type[16] = {'\0'};
- for (i = 0, row = start; row < end; ++i, ++row) {
- if (STREQ (parttype, "gpt")) {
- memcpy (temp_type, "primary", strlen ("primary"));
- if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64 "B%"
SCNi64 "B",
- &part_num,
- &temp_int64,
- &temp_int64,
- &temp_int64) != 4) {
- reply_with_error ("could not parse row from output of parted print command:
%s", lines[row]);
- return NULL;
- }
- } else {
- if (sscanf (lines[row], "%d%" SCNi64 "B%" SCNi64 "B%"
SCNi64 "B" "%15s",
- &part_num,
- &temp_int64,
- &temp_int64,
- &temp_int64,
- temp_type) != 5) {
- reply_with_error ("could not parse row from output of parted print command:
%s", lines[row]);
- return NULL;
- }
- }
-
- if (part_num != partnum)
- continue;
-
- if (STRPREFIX (temp_type, "primary")) {
- part_type = strdup ("primary");
- if (part_type == NULL)
- goto error;
- } else if (STRPREFIX (temp_type, "logical")) {
- part_type = strdup ("logical");
- if (part_type == NULL)
- goto error;
- } else if (STRPREFIX (temp_type, "extended")) {
- part_type = strdup ("extended");
- if (part_type == NULL)
- goto error;
- } else
- goto error;
-
- return part_type;
- }
-
- if (row == end) {
- reply_with_error ("could not find partnum: %d", partnum);
- return NULL;
- }
-
- error:
- reply_with_error ("strdup failed");
- return NULL;
-}
-
static char *
extract_uuid (const char *value)
{
diff --git a/daemon/parted.ml b/daemon/parted.ml
index ce8da8a60..42ccb84c5 100644
--- a/daemon/parted.ml
+++ b/daemon/parted.ml
@@ -125,6 +125,19 @@ let part_get_parttype device =
| _ ->
failwithf "%s: cannot parse the output of parted" device
+let part_get_mbr_part_type device partnum =
+ let parttype = part_get_parttype device in
+ let mbr_id = part_get_mbr_id device partnum in
+
+ (* 0x05 - extended partition.
+ * 0x0f - extended partition using BIOS INT 13h extensions.
+ *)
+ match parttype, partnum, mbr_id with
+ | "msdos", (1|2|3|4), (0x05|0x0f) -> "extended"
+ | "msdos", (1|2|3|4), _ -> "primary"
+ | "msdos", _, _ -> "logical"
+ | _, _, _ -> "primary"
+
let part_set_gpt_attributes device partnum attributes =
if partnum <= 0 then failwith "partition number must be >= 1";
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index 394576398..a049159ec 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -9213,6 +9213,7 @@ All data will be zeroed, but metadata and the like is
preserved." };
{ defaults with
name = "part_get_mbr_part_type"; added = (1, 29, 32);
style = RString (RPlainString, "partitiontype"), [String (Device,
"device"); Int "partnum"], [];
+ impl = OCaml "Parted.part_get_mbr_part_type";
tests = [
InitEmpty, Always, TestResultString (
[["part_init"; "/dev/sda"; "mbr"];
--
2.17.0
7:53 p.m.
New subject: [PATCH v6 2/7] daemon: Changing the way that we detect if a device contains partitions.
Instead of using part_to_dev to find such devices we open the device's
directory under /sys/block/<device> and look for entries starting with
<device>, eg. /sys/block/sda/sda1.
---
daemon/listfs.ml | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index 56ebadeda..55ace8e9c 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,23 @@ let rec list_filesystems () =
List.flatten ret
+(* Look to see if device can directly contain filesystem (RHBZ#590167).
+ * Partitioned devices cannot contain fileystem, so we will exclude such devices.
+ *)
+and is_not_partitioned_device device =
+ assert (String.is_prefix device "/dev/");
+ let device = String.sub device 5 (String.length device - 5) in
+ let dir = "/sys/block/" ^ device in
+
+ try
+ (* Open the device's directory under /sys/block/<device> and
+ * look for entries starting with <device>, eg. /sys/block/sda/sda1
+ *)
+ let parts = Array.to_list (Sys.readdir dir) in
+ let has_partition = List.exists (fun part -> String.is_prefix part device) parts
in
+ not has_partition
+ with Sys_error (_) -> true
+
(* 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
4:44 a.m.
New subject: [PATCH v6 2/7] daemon: Changing the way that we detect if a device contains partitions.
On Tue, May 01, 2018 at 03:53:59AM +0300, Mykola Ivanets wrote:
Instead of using part_to_dev to find such devices we open the
device's
directory under /sys/block/<device> and look for entries starting with
<device>, eg. /sys/block/sda/sda1.
+(* Look to see if device can directly contain filesystem
(RHBZ#590167).
+ * Partitioned devices cannot contain fileystem, so we will exclude such devices.
+ *)
Let's keep lines to < 80 characters.
+and is_not_partitioned_device device =
+ assert (String.is_prefix device "/dev/");
+ let device = String.sub device 5 (String.length device - 5) in
+ let dir = "/sys/block/" ^ device in
+
+ try
+ (* Open the device's directory under /sys/block/<device> and
+ * look for entries starting with <device>, eg. /sys/block/sda/sda1
+ *)
+ let parts = Array.to_list (Sys.readdir dir) in
+ let has_partition = List.exists (fun part -> String.is_prefix part device) parts
in
Also this line is >= 80 characters, but could easily be changed
so it's shorter.
+ not has_partition
+ with Sys_error (_) -> true
This is the real problem. What Sys_error is expected here? As far as
I can tell if anything throws Sys_error (or any other exception) here
then it's a real bug, so we shouldn't hide the error message. If
you're not expecting a particular Sys_error, then don't try to catch
anything.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
7:54 p.m.
New subject: [PATCH v6 3/7] daemon: list-filesystems: Ignore partitioned MD devices.
Ignore partitioned MD devices the same way we ignore regular partitioned
devices because they cannot contain filesystems as well.
---
daemon/listfs.ml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index 55ace8e9c..4014e3fe9 100644
--- a/daemon/listfs.ml
+++ b/daemon/listfs.ml
@@ -43,8 +43,9 @@ let rec list_filesystems () =
None (* ignore type 42 *)
) partitions in
- (* Use vfs-type to check for filesystems on md devices. *)
+ (* MD. *)
let mds = Md.list_md_devices () in
+ let mds = List.filter is_not_partitioned_device mds in
let ret = ret @ List.filter_map check_with_vfs_type mds in
(* LVM. *)
--
2.17.0
7:54 p.m.
New subject: [PATCH v6 4/7] tests: list-filesystems command ignores partitioned MD devices.
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
7:54 p.m.
New subject: [PATCH v6 5/7] daemon: list-filesystems: Change the way we filter out LDM partitions.
1. Now we use GPT partition type to filter out LDM partitions.
2. We also check for filesystems on LDM volumes because LDM partitions
doesn't contain filesystems (list_ldm_partitions is not called anymore).
3. Obvious repetitive comments are moved to a function description.
---
daemon/listfs.ml | 70 ++++++++++++++++++++++++------------------------
1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index 4014e3fe9..0d71a39db 100644
--- a/daemon/listfs.ml
+++ b/daemon/listfs.ml
@@ -20,6 +20,10 @@ open Printf
open Std_utils
+(* Enumerate block devices (including MD, LVM, LDM and partitions) and use
+ * vfs-type to check for filesystems on devices. Some block devices cannot
+ * contain filesystems, so we filter them out.
+ *)
let rec list_filesystems () =
let has_lvm2 = Optgroups.lvm2_available () in
let has_ldm = Optgroups.ldm_available () in
@@ -29,19 +33,10 @@ let rec list_filesystems () =
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.
- *)
+ (* Partitions. *)
let partitions = Devsparts.list_partitions () in
- let ret =
- ret @
- List.filter_map (
- fun part ->
- if not has_ldm || not (is_mbr_partition_type_42 part) then
- check_with_vfs_type part
- else
- None (* ignore type 42 *)
- ) partitions in
+ let partitions = List.filter is_not_ldm_partition partitions in
+ let ret = ret @ List.filter_map check_with_vfs_type partitions in
(* MD. *)
let mds = Md.list_md_devices () in
@@ -52,7 +47,6 @@ let rec list_filesystems () =
let ret =
if has_lvm2 then (
let lvs = Lvm.lvs () in
- (* Use vfs-type to check for filesystems on LVs. *)
ret @ List.filter_map check_with_vfs_type lvs
)
else ret in
@@ -61,11 +55,7 @@ let rec list_filesystems () =
let ret =
if has_ldm then (
let ldmvols = Ldm.list_ldm_volumes () in
- let ldmparts = Ldm.list_ldm_partitions () in
- (* Use vfs-type to check for filesystems on Windows dynamic disks. *)
- ret @
- List.filter_map check_with_vfs_type ldmvols @
- List.filter_map check_with_vfs_type ldmparts
+ ret @ List.filter_map check_with_vfs_type ldmvols
)
else ret in
@@ -87,6 +77,33 @@ and is_not_partitioned_device device =
let has_partition = List.exists (fun part -> String.is_prefix part device) parts
in
not has_partition
with Sys_error (_) -> true
+
+(* We should ignore Windows Logical Disk Manager (LDM) partitions,
+ * 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.
+ *)
+and is_not_ldm_partition 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
+
+ let is_gpt = parttype = "gpt" in
+ let is_mbr = parttype = "msdos" in
+ let is_gpt_or_mbr = is_gpt || is_mbr in
+
+ if is_gpt_or_mbr then (
+ (* MBR partition id will be converted into corresponding GPT type. *)
+ let gpt_type = Parted.part_get_gpt_type device partnum in
+ match gpt_type with
+ (* Windows Logical Disk Manager metadata partition. *)
+ | "5808C8AA-7E8F-42E0-85D2-E1E90434CFB3"
+ (* Windows Logical Disk Manager data partition. *)
+ | "AF9B60A0-1431-4F62-BC68-3311714A69AD" -> false
+ | _ -> true
+ )
+ else true
(* Use vfs-type to check for a filesystem of some sort of [device].
* Returns [Some [device, vfs_type; ...]] if found (there may be
@@ -143,20 +160,3 @@ and check_with_vfs_type device =
else
Some [mountable, vfs_type]
-
-(* We should ignore partitions that have MBR type byte 0x42, 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.
- *)
-and is_mbr_partition_type_42 partition =
- try
- let partnum = Devsparts.part_to_partnum partition in
- let device = Devsparts.part_to_dev partition in
- let mbr_id = Parted.part_get_mbr_id device partnum in
- mbr_id = 0x42
- with exn ->
- if verbose () then
- eprintf "is_mbr_partition_type_42: %s: %s\n"
- partition (Printexc.to_string exn);
- false
--
2.17.0
7:54 p.m.
New subject: [PATCH v6 6/7] daemon: list-filesystems: Filter out Microsoft Reserved and Windows Snapshot partitions.
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 0d71a39db..fa7813378 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. *)
@@ -82,9 +82,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
@@ -100,7 +101,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
7:54 p.m.
New subject: [PATCH v6 7/7] daemon: list-ilesystems: Filter out MBR extended partitions.
Extended MBR partitions cannot hold filesystems - filter them out.
---
daemon/listfs.ml | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index fa7813378..2ed7da6e3 100644
--- a/daemon/listfs.ml
+++ b/daemon/listfs.ml
@@ -83,7 +83,7 @@ and is_not_partitioned_device device =
* to read them will cause errors (RHBZ#887520). Assuming that
* libguestfs was compiled with ldm support, we'll get the filesystems
* on these later. We also ignore Microsoft Reserved Partition and
- * Windows Snapshot Partition.
+ * Windows Snapshot Partition as well as MBR extended partitions.
*)
and is_partition_can_hold_filesystem partition =
let device = Devsparts.part_to_dev partition in
@@ -106,10 +106,17 @@ and is_partition_can_hold_filesystem partition =
| "E3C9E316-0B5C-4DB8-817D-F92DF00215AE"
(* Windows Snapshot Partition. *)
| "CADDEBF1-4400-4DE8-B103-12117DCF3CCF" -> false
- | _ -> true
+ | _ -> not (is_mbr_extended parttype device partnum)
)
else true
+and is_mbr_extended parttype device partnum =
+ if parttype = "msdos" then (
+ let mbr_type = Parted.part_get_mbr_part_type device partnum in
+ mbr_type = "extended"
+ )
+ else false
+
(* 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
4:53 a.m.
New subject: [PATCH v6 7/7] daemon: list-ilesystems: Filter out MBR extended partitions.
On Tue, May 01, 2018 at 03:54:04AM +0300, Mykola Ivanets wrote:
Extended MBR partitions cannot hold filesystems - filter them out.
---
daemon/listfs.ml | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index fa7813378..2ed7da6e3 100644
--- a/daemon/listfs.ml
+++ b/daemon/listfs.ml
@@ -83,7 +83,7 @@ and is_not_partitioned_device device =
* to read them will cause errors (RHBZ#887520). Assuming that
* libguestfs was compiled with ldm support, we'll get the filesystems
* on these later. We also ignore Microsoft Reserved Partition and
- * Windows Snapshot Partition.
+ * Windows Snapshot Partition as well as MBR extended partitions.
*)
and is_partition_can_hold_filesystem partition =
let device = Devsparts.part_to_dev partition in
@@ -106,10 +106,17 @@ and is_partition_can_hold_filesystem partition =
| "E3C9E316-0B5C-4DB8-817D-F92DF00215AE"
(* Windows Snapshot Partition. *)
| "CADDEBF1-4400-4DE8-B103-12117DCF3CCF" -> false
- | _ -> true
+ | _ -> not (is_mbr_extended parttype device partnum)
This is a bit confusing. I think the equivalent can be
achieved by changing the final function to:
if is_gpt_or_mbr then (
if is_mbr_extended parttype device partnum then
false
else (
(* MBR partition id will be converted into corresponding GPT type. *)
let gpt_type = Parted.part_get_gpt_type device partnum in
match gpt_type with
(* Windows Logical Disk Manager metadata partition. *)
| "5808C8AA-7E8F-42E0-85D2-E1E90434CFB3"
(* Windows Logical Disk Manager data partition. *)
| "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
The other patches (where I didn't have comments) look OK to me.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
2320
days inactive
2321
days old
9 comments
2 participants
participants (2)
-
Mykola Ivanets -
Richard W.M. Jones