5:16 p.m.
Instead of parsing 'parted' output OCaml implementation relies on the following
facts:
1. The function is applicable for MBR partitions only (as noted in documentation and as
function name suggests).
2. An attempt to call the function for non-MBR partition fails with
"part_get_mbr_part_type can only be used on MBR Partitions" error and NULL is
returned.
3. MBR partition table can hold up to 4 "primary" partitions.
4. Partition with number >= 5 is logical partition
5. Extnded partition number is <= 4 and has MBR id 0x05 or 0x0f
(http://thestarman.pcministry.com/asm/mbr/PartTypes.htm;
https://en.wikipedia.org/wiki/Partition_type).
---
daemon/parted.c | 106 ----------------------------------------------
daemon/parted.ml | 14 ++++++
daemon/parted.mli | 2 +
generator/actions_core.ml | 1 +
4 files changed, 17 insertions(+), 106 deletions(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index e5435b5..d67c6c5 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 ce8da8a..75d9d37 100644
--- a/daemon/parted.ml
+++ b/daemon/parted.ml
@@ -125,6 +125,20 @@ 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 id within the first 1024 cylinders.
+ * 0x0f - extended partition id beyond the first 1024 cylinders.
+ *)
+ match parttype, partnum, mbr_id with
+ | "msdos", (1|2|3|4), (0x05|0x0f) -> "extended"
+ | "msdos", (1|2|3|4), _ -> "primary"
+ | "msdos", _, _ -> "logical"
+ | _, _, _ ->
+ failwithf "part_get_mbr_part_type can only be used on MBR Partitions"
+
let part_set_gpt_attributes device partnum attributes =
if partnum <= 0 then failwith "partition number must be >= 1";
diff --git a/daemon/parted.mli b/daemon/parted.mli
index d547f2f..0f59d29 100644
--- a/daemon/parted.mli
+++ b/daemon/parted.mli
@@ -28,6 +28,8 @@ val part_list : string -> partition list
val part_get_parttype : string -> string
+val part_get_mbr_part_type : string -> int -> string
+
val part_get_gpt_type : string -> int -> string
val part_get_gpt_guid : string -> int -> string
val part_get_gpt_attributes : string -> int -> int64
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index 544cb6e..307e414 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.9.5
5:16 p.m.
New subject: [PATCH v2 2/3] daemon: list-filesystems: Don't list partitions which cannot hold file system.
Filter out:
1. Partitioned md devices (as we do this for physial devices).
2. Extended MBR partitions.
3. LDM partitions (MBR and GPT partitions used by Windows Logical Disk Manager)
4. Microsoft Reseved Partitions.
---
daemon/devsparts.ml | 13 +++++++
daemon/devsparts.mli | 1 +
daemon/listfs.ml | 105 +++++++++++++++++++++++++++------------------------
3 files changed, 70 insertions(+), 49 deletions(-)
diff --git a/daemon/devsparts.ml b/daemon/devsparts.ml
index 54108bb..42dc78d 100644
--- a/daemon/devsparts.ml
+++ b/daemon/devsparts.ml
@@ -71,6 +71,19 @@ let list_devices () =
map_block_devices ~return_md:false ((^) "/dev/") in
sort_device_names devices
+let is_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
+ List.exists (fun part -> String.is_prefix part device) parts
+ with Sys_error (_) -> false
+
let rec list_partitions () =
let partitions = map_block_devices ~return_md:true add_partitions in
let partitions = List.flatten partitions in
diff --git a/daemon/devsparts.mli b/daemon/devsparts.mli
index 7b669c2..d3224a1 100644
--- a/daemon/devsparts.mli
+++ b/daemon/devsparts.mli
@@ -21,5 +21,6 @@ val list_partitions : unit -> string list
val part_to_dev : string -> string
val part_to_partnum : string -> int
val is_whole_device : string -> bool
+val is_partitioned_device : string -> bool
val nr_devices : unit -> int
val device_index : string -> int
diff --git a/daemon/listfs.ml b/daemon/listfs.ml
index 370ffb4..1349ae0 100644
--- a/daemon/listfs.ml
+++ b/daemon/listfs.ml
@@ -25,39 +25,18 @@ let rec list_filesystems () =
let has_ldm = Ldm.available () in
let devices = Devsparts.list_devices () in
- let partitions = Devsparts.list_partitions () in
+ let devices = List.filter check_device devices 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
+ let mds = List.filter check_device mds in
(* Use vfs-type to check for filesystems on devices. *)
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 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 = Devsparts.list_partitions () in
+ let partitions = List.filter check_partition partitions in
+
+ (* Use vfs-type to check for filesystems on partitions. *)
+ let ret = ret @ List.filter_map check_with_vfs_type partitions in
(* Use vfs-type to check for filesystems on md devices. *)
let ret = ret @ List.filter_map check_with_vfs_type mds in
@@ -75,16 +54,61 @@ 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
List.flatten ret
+(* 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
+ * exclude such devices.
+ *)
+and check_device device =
+ not (Devsparts.is_partitioned_device device)
+
+(* 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. We also ignores Microsoft Reserved Partitions (MSR)
+ * and extended MBR partitions because they don't contain filesystem.
+ *)
+and check_partition partition =
+ try
+ (* Windows Logical Disk Manager metadata partition. *)
+ let gpt_ldm_metadata = "5808C8AA-7E8F-42E0-85D2-E1E90434CFB3" in
+ (* Windows Logical Disk Manager data partition. *)
+ let gpt_ldm_data = "AF9B60A0-1431-4F62-BC68-3311714A69AD" in
+ (* Microsoft Reserved Partition. *)
+ let gpt_msr = "E3C9E316-0B5C-4DB8-817D-F92DF00215AE" in
+
+ 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
+
+ (* MBR partition id will be converted into corresponding GPT type. *)
+ let gpt_type = Parted.part_get_gpt_type device partnum in
+
+ let is_ldm = gpt_type = gpt_ldm_metadata || gpt_type = gpt_ldm_data in
+ let is_ldm = Ldm.available () && (is_gpt || is_mbr) && is_ldm in
+ let is_msr = is_gpt && gpt_type = gpt_msr in
+
+ let mbr_type = Parted.part_get_mbr_part_type device partnum in
+ let is_extended = mbr_type = "extended" in
+
+ not (is_ldm || is_msr || is_extended)
+ with exn ->
+ if verbose () then
+ eprintf "check_partition: %s: %s\n"
+ partition (Printexc.to_string exn);
+ 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
@@ -140,20 +164,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.9.5
5:16 p.m.
New subject: [PATCH v2 3/3] tests: md: Test guestfish list-filesystems command skipps 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 42260af..2e48756 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 0000000..36b6d8a
--- /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.9.5
2494
days inactive
2494
days old
2 comments
1 participants
participants (1)
-
Mykola Ivanets