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

[Jim Meyering]

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.

This sounds like a good change.
As usual, I prefer not to make behavior changes
unless they're accompanied by a test suite addition
that exercises the affected code.

Can you help prepare the test?

In addition, please adjust the style of any new or changed
code (below) so that declarations are no longer placed at the top
of a function.  Instead, whenever possible, declare at point
of first use.

>  NEWS                   |    3 ++
>  libparted/arch/linux.c |   77 +++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 69 insertions(+), 11 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 4fede6c..f616b72 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -46,6 +46,9 @@ GNU parted NEWS                                    -*- outline -*-
>
>  ** Changes in behavior
>
> +  Device-mapper devices other than dmraid whole disks will no longer be
> +  shown by parted -l.
> +
>    Floppy drives are no longer scanned on linux: they cannot be partitioned
>    anyhow, and some users have a misconfigured BIOS that claims to have a
>    floppy when they don't, and scanning gets hung up.
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index a521652..6d1b5e9 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -291,10 +291,14 @@ static int
>  _dm_add_partition (PedDisk* disk, const PedPartition* part);
>  static int
>  _dm_remove_partition(PedDisk* disk, int partno);
> +static int
> +_dm_is_part (const char *path);
>  static bool
>  _dm_get_partition_start_and_length(PedPartition const *part,
>                                     unsigned long long *start,
>                                     unsigned long long *length);
> +static int
> +_is_dmraid_device (char* devpath);

Instead of fwd-declaring, just move the definition "up" to precede
the first use.

>  static int
>  _read_fd (int fd, char **buf)
> @@ -512,13 +516,52 @@ _probe_dm_devices ()
>                 if (stat (buf, &st) != 0)
>                         continue;
>
> -               if (_is_dm_major(major(st.st_rdev)))
> +               if (_is_dm_major(major(st.st_rdev)) && _is_dmraid_device (buf)
> +                   && !_dm_is_part(buf))
>                         _ped_device_probe (buf);
>         }
>         closedir (mapper_dir);
>
>         return 1;
>  }
> +
> +/* 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.
> + */
> +static int
> +_is_dmraid_device (char* devpath)

This should be `const char *devpath'
Please put the "*" next to the variable, not the type.
Below, too.

> +{
> +        struct dm_task* task = NULL;
> +        int             rc = 0;
> +        const char*     dmraid_uuid;
> +        char*           dm_name = NULL;
> +
> +        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;
> +
> +        task = dm_task_create (DM_DEVICE_DEPS);
> +        if (!task)
> +                return 0;
> +
> +        dm_task_set_name (task, dm_name);
> +        if (!dm_task_run (task))
> +                goto err;
> +
> +        dmraid_uuid = dm_task_get_uuid (task);
> +        if (strncmp (dmraid_uuid, "DMRAID-", 7) == 0) {
> +                rc = 1;
> +        }
> +
> +err:
> +        free (dm_name);
> +        dm_task_destroy (task);
> +        return rc;
> +}
>  #endif
>
>  static int
> @@ -2711,20 +2754,26 @@ _dm_remove_partition(PedDisk* disk, int partno)
>          return 1;
>  }
>
> +/* 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 (struct dm_info *this, char *name)
> +_dm_is_part (const char *path)
>  {
>          struct dm_task* task = NULL;
>          struct dm_info* info = alloca(sizeof *info);
>          struct dm_deps* deps = NULL;
>          int             rc = 0;
>          unsigned int    i;
> +        char *target_type = NULL;
> +        uint64_t start, length;
> +        char *params;
>
>          task = dm_task_create(DM_DEVICE_DEPS);
>          if (!task)
>                  return 0;
>
> -        dm_task_set_name(task, name);
> +        dm_task_set_name(task, path);
>          if (!dm_task_run(task))
>                  goto err;
>
> @@ -2737,14 +2786,20 @@ _dm_is_part (struct dm_info *this, char *name)
>          if (!deps)
>                  goto err;
>
> -        for (i = 0; i < deps->count; i++) {
> -                unsigned int ma = major(deps->device[i]),
> -                             mi = minor(deps->device[i]);
> -
> -                if (ma == this->major && mi == this->minor)
> -                        rc = 1;
> -        }
> -
> +        if (deps->count != 1)
> +                goto err;
> +        if (!_is_dm_major(major(deps->device[0])))
> +                goto err;
> +        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))
> +                goto err;
> +        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;