[parted-devel] [PATCH 4/6] libparted: set dmraid partition uuid

[Jim Meyering]

Phillip Susi wrote:
> When dmraid creates the partition devices, it assigns them a UUID in the
> form of "DMRAID-XXXXX".  When kpartx creates the partitions, it assigns
> them a UUID of the form "partN-DMRAID-XXXX".  This patch has parted use
> the kpartx style.
> ---
>  NEWS                   |   10 ++++++++++
>  libparted/arch/linux.c |    8 ++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index d8fefc8..ea3b302 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -3,6 +3,7 @@ GNU parted NEWS                                    -*- outline -*-
>  * Noteworthy changes in release ?.? (????-??-??) [?]
>
>  ** Bug Fixes
> +

This newline addition belongs in the other patch.

>    libparted: Don't fail to manipulate partitions on dmraid disks that
>    have other partitions in use.
>
> @@ -21,6 +22,12 @@ GNU parted NEWS                                    -*- outline -*-
>
>  ** Changes in behavior
>
> +  libparted: set dmraid partition uuid.  When dmraid creates the
> +  partition devices, it assigns them a UUID in the form of
> +  "DMRAID-XXXXX".  When kpartx creates the partitions, it assigns
> +  them a UUID of the form "partN-DMRAID-XXXX".  Parted will now
> +  set the UUID and use the kpartx style.
> +
>    Device-mapper devices other than dmraid whole disks will no longer be
>    shown by parted -l.
>
> @@ -64,6 +71,9 @@ GNU parted NEWS                                    -*- outline -*-
>
>  ** Bug fixes
>
> +  libparted: Don't fail to manipulate partitions on dmraid disks that
> +  have other partitions in use.
> +
>    libparted: avoid an invalid free when creating many partitions in
>    a GPT partition table.  [bug introduced in parted-1.9.0]
>
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 5e5b527..5eb66ac 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2743,6 +2743,14 @@ _dm_add_partition (PedDisk* disk, const PedPartition* part)
>          dm_task_set_name (task, vol_name);
>          dm_task_add_target (task, 0, part->geom.length,
>                  "linear", params);
> +        char *dm_uuid;
> +        int uuid_len;
> +        if (_is_dmraid_device (disk->dev->path)) {
> +                uuid_len = (strlen (vol_name) + 8);
> +                dm_uuid = (char*) ped_malloc (uuid_len);
> +                snprintf (dm_uuid, uuid_len, "part%d-DMRAID-%s", part->num, vol_name);
> +                dm_task_set_uuid (task, dm_uuid);

This doesn't handle malloc failure.
How about something like this instead?

        char *dm_uuid = zasprintf ("part%d-DMRAID-%s", part->num, vol_name);
        if (dm_uuid)
                dm_task_set_uuid (task, dm_uuid);

How hard would it be to add a test?
If this new behavior is desirable enough to someone that
you're making the change, then it probably does deserve a test.

Thanks,
Jim