[parted-devel] [PATCH 3/6] libparted: don't probe every dm device in probe_all

Jim Meyering jim at meyering.net
Sat Oct 20 12:48:27 UTC 2012


Phillip Susi wrote:
> We were probing every dm device.  Only probe dmraid whole disk ( non
> partition ) devices instead.  This removes the clutter of LVM logical
> volumes, and dmraid partitions from the list, which usually do not make
> sense to partition.
> ---
>  NEWS                   |    3 ++
>  libparted/arch/linux.c |   86 +++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/Makefile.am      |    1 +
>  tests/t6002-dm-busy.sh |    1 +
>  tests/t6003-dm-hide.sh |   60 +++++++++++++++++++++++++++++++++
>  5 files changed, 150 insertions(+), 1 deletion(-)
>  create mode 100644 tests/t6003-dm-hide.sh
>
> diff --git a/NEWS b/NEWS
> index f182cce..d8fefc8 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,9 @@ GNU parted NEWS                                    -*- outline -*-
>
>  ** Changes in behavior
>
> +  Device-mapper devices other than dmraid whole disks will no longer be
> +  shown by parted -l.

Thanks for writing that, and especially for the new test.
Unfortunately for me, the new test would malfunction on F17,
so I ended up working around what appeared to be race-related problems.
The test-related changes I made are in the third patch below.
The first merely adjusts wording in NEWS: it's better to use an
"active" voice (here's an example from HACKING):

    When writing prose (documentation, comments, log entries), use an
    active voice, not a passive one.  I.e., say "print the frobnozzle",
    not "the frobnozzle will be printed".

The next patch removes some unnecessary strdup calls and adjusts
the formatting.  I'll merge those into your patch, but leave the
test-changing c-set separate.

>From 15cc987c27b6fb79968a18ccfb409ae32a73fb89 Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim at meyering.net>
Date: Fri, 19 Oct 2012 17:32:47 +0200
Subject: [PATCH 1/3] FIXME NEWS amendment

---
 NEWS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 533bc3c..89541fd 100644
--- a/NEWS
+++ b/NEWS
@@ -25,8 +25,8 @@ GNU parted NEWS                                    -*- outline -*-

 ** Changes in behavior

-  Device-mapper devices other than dmraid whole disks will no longer be
-  shown by parted -l.
+  parted -l no longer lists device-mapper devices other than
+  dmraid whole disks.

   Added new Linux-specific partition GUID type code
   (0FC63DAF-8483-4772-8E79-3D69D8477DE4) for Linux filesystem data on GPT
--
1.8.0.rc2.11.gd25c58c


>From f8ea4ae2a20c862d35dc9febbb5e6a53cfdaf689 Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim at meyering.net>
Date: Fri, 19 Oct 2012 17:53:59 +0200
Subject: [PATCH 2/3] STRPREFIX: define/use; adjust formatting; remove
 unnecessary strdup

---
 libparted/arch/linux.c | 53 +++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 957fcfa..083591f 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -65,6 +65,8 @@
 # define _GL_ATTRIBUTE_FORMAT(spec) /* empty */
 #endif

+#define STRPREFIX(a, b) (strncmp (a, b, strlen (b)) == 0)
+
 #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))

 #ifndef __NR__llseek
@@ -478,37 +480,29 @@ bad:
         return r;
 }

-/* Checks whether the given device-mapper device is part of a dmraid array,
- * by checking for the string "DMRAID-" at the start of the UUID.
- */
+/* Return nonzero if device-mapper device, DEVPATH, is part of a dmraid
+   array.  Use the heuristic of checking for the string "DMRAID-" at the
+   start of its UUID.  */
 static int
 _is_dmraid_device (const char *devpath)
 {
-        int             rc = 0;
-
-        char *dm_name = strrchr (devpath, '/');
-        if (dm_name && *dm_name && *(++dm_name))
-                dm_name = strdup (dm_name);
-        else
-                dm_name = strdup (devpath);
-        if (!dm_name)
-                return 0;
+        int rc = 0;

+        char const *dm_name = strrchr (devpath, '/');
+        char const *dm_basename = dm_name && *(++dm_name) ? dm_name : devpath;
         struct dm_task *task = dm_task_create (DM_DEVICE_DEPS);
         if (!task)
                 return 0;

-        dm_task_set_name (task, dm_name);
+        dm_task_set_name (task, dm_basename);
         if (!dm_task_run (task))
                 goto err;

         const char *dmraid_uuid = dm_task_get_uuid (task);
-        if (strncmp (dmraid_uuid, "DMRAID-", 7) == 0) {
+        if (STRPREFIX (dmraid_uuid, "DMRAID-"))
                 rc = 1;
-        }

 err:
-        free (dm_name);
         dm_task_destroy (task);
         return rc;
 }
@@ -516,11 +510,11 @@ err:
 /* We consider a dm device that is a linear mapping with a  *
  * single target that also is a dm device to be a partition */

-static int _dm_is_part (const char *path)
+static int
+_dm_is_part (const char *path)
 {
-        int             rc = 0;
-
-        struct dm_task *task = dm_task_create(DM_DEVICE_DEPS);
+        int rc = 0;
+        struct dm_task *task = dm_task_create (DM_DEVICE_DEPS);
         if (!task)
                 return 0;

@@ -528,35 +522,36 @@ static int _dm_is_part (const char *path)
         if (!dm_task_run(task))
                 goto err;

-        struct dm_info* info = alloca(sizeof *info);
+        struct dm_info *info = alloca (sizeof *info);
         memset(info, '\0', sizeof *info);
-        dm_task_get_info(task, info);
+        dm_task_get_info (task, info);
         if (!info->exists)
                 goto err;

-        struct dm_deps *deps = dm_task_get_deps(task);
+        struct dm_deps *deps = dm_task_get_deps (task);
         if (!deps)
                 goto err;

         if (deps->count != 1)
                 goto err;
-        if (!_is_dm_major(major(deps->device[0])))
+        if (!_is_dm_major (major (deps->device[0])))
                 goto err;
-        dm_task_destroy(task);
-        if (!(task = dm_task_create(DM_DEVICE_TABLE)))
+        dm_task_destroy (task);
+        if (!(task = dm_task_create (DM_DEVICE_TABLE)))
                 return 0;
-        dm_task_set_name(task, path);
-        if (!dm_task_run(task))
+        dm_task_set_name (task, path);
+        if (!dm_task_run (task))
                 goto err;

         char *target_type = NULL;
         char *params = NULL;
         uint64_t start, length;

-        dm_get_next_target(task, NULL, &start, &length, &target_type, &params);
+        dm_get_next_target (task, NULL, &start, &length, &target_type, &params);
         if (strcmp (target_type, "linear"))
                 goto err;
         rc = 1;
+
 err:
         dm_task_destroy(task);
         return rc;
--
1.8.0.rc2.11.gd25c58c


>From 5f675ba428ed1f9b144b2142490db0c392f9dc17 Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim at meyering.net>
Date: Fri, 19 Oct 2012 18:09:19 +0200
Subject: [PATCH 3/3] tests: make t6003-dm-hide work reliably on F17

* tests/t6003-dm-hide.sh: Adjust to work reliably on Fedora 17.
---
 tests/t6003-dm-hide.sh | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/t6003-dm-hide.sh b/tests/t6003-dm-hide.sh
index 3cfdc43..59baae9 100644
--- a/tests/t6003-dm-hide.sh
+++ b/tests/t6003-dm-hide.sh
@@ -19,7 +19,6 @@
 . "${srcdir=.}/init.sh"; path_prepend_ ../parted

 require_root_
-lvm_init_root_dir_

 test "x$ENABLE_DEVICE_MAPPER" = xyes \
   || skip_ "no device-mapper support"
@@ -32,7 +31,10 @@ d1=
 f1=
 dev=
 cleanup_fn_() {
-    dmsetup remove $linear_
+    # Insist.  Sometimes the initial removal fails (race?).
+    # When that happens, a second removal appears to be sufficient.
+    dmsetup remove $linear_ || dmsetup remove $linear_
+
     test -n "$d1" && losetup -d "$d1"
     rm -f "$f1"
 }
@@ -41,20 +43,25 @@ f1=$(pwd)/1; d1=$(loop_setup_ "$f1") \
   || fail=1

 # setup: create a mapping
-echo "0 2048 linear $d1 0" | dmsetup create $linear_ || fail=1
-dev="$DM_DEV_DIR/mapper/$linear_"
-
-# device should not show up
+echo 0 2048 linear $d1 0 | dmsetup create $linear_ || fail=1
+dev=/dev/mapper/$linear_

+# No "DMRAID-" UUID prefix, hence the device should not show up.
 parted -l >out 2>&1
-! grep $linear_ out || fail=1
+grep "^Disk $dev:" out && fail=1

+# Unless we perform both dmsetup-remove *and* losetup -d,
+# the following dmsetup-create would fail with EBUSY.
 dmsetup remove $linear_
-echo "0 2048 linear $d1 0" | dmsetup create $linear_ -u "DMRAID-fake" || fail=1
+losetup -d "$d1" || fail=1
+# Reopen (or get new) loop device.
+d1=$(loop_setup_ "$f1") || fail=1

-# device should now show up
+# This time, use a fake UUID.
+echo 0 2048 linear $d1 0 | dmsetup create $linear_ -u "DMRAID-fake-$$" || fail=1

+# Thus, the device should now show up.
 parted -l >out 2>&1
-grep $linear_ out || fail=1
+grep "^Disk $dev:" out || fail=1

 Exit $fail
--
1.8.0.rc2.11.gd25c58c



More information about the parted-devel mailing list