[parted-devel] [PATCH 3/7] libparted: Add support for partition resize

[Brian C. Lane]

On Sun, May 25, 2014 at 05:11:29PM -0400, Phillip Susi wrote:
> +        if (part->type & PED_PARTITION_EXTENDED) {
> +                if (disk->dev->sector_size == 512) {
> +                        linux_part.length = 2;
> +                        PedPartition *walk;
> +                        /* if the second sector is claimed by a logical partition,
> +                           then there's just no room for lilo, so don't try to use it */
> +                        for (walk = part->part_list; walk; walk = walk->next) {
> +                                if (walk->geom.start == part->geom.start+1)
> +                                        linux_part.length = 1;
> +                        }
> +                } else linux_part.length = 1;
> +        }
> +        else
> +                linux_part.length = part->geom.length * disk->dev->sector_size;

else clauses should use {} when the main block of the if uses {} so that
it is clear where the blocks end.

> +static int
> +_dm_resize_partition (PedDisk* disk, const PedPartition* part)
> +{
> +        LinuxSpecific*  arch_specific = LINUX_SPECIFIC (disk->dev);
> +        char*           params = NULL;
> +        char*           vol_name = NULL;
> +        const char*     dev_name = NULL;
> +        uint32_t        cookie = 0;

Does this preserve the uuid? Or does it need to do the same thing as we
do in dm_add_partition?

> +#ifdef BLKPG_RESIZE_PARTITION
> +                resize_partition = _blkpg_resize_partition;
> +#else
> +                resize_partition = 0;
> +#endif

I'd prefer to see NULL used instead of 0 here.

> @@ -2901,11 +3032,23 @@ _disk_sync_part_table (PedDisk* disk)
>                  /* get start and length of existing partition */
>                  if (get_partition_start_and_length(part,
>                                                     &start, &length)
> -                    && start == part->geom.start
> -                    && length == part->geom.length) {
> -                        ok[i - 1] = 1;
> -                        /* partition is unchanged, so nothing to do */
> -                        continue;
> +                    && start == part->geom.start)
> +                {
> +                        if (length == part->geom.length) {
> +                                ok[i - 1] = 1;
> +                                /* partition is unchanged, so nothing to do */
> +                                continue;
> +                        }
> +                        if (resize_partition
> +                            && start == part->geom.start
> +			    && length != part->geom.length)
> +			{

This check for != length is redundant since it just passed by an ==

Also, watch out for tab vs. space usage.

> +                sprintf (bad_part_list + strlen (bad_part_list), "%d, ", i);
> +        }

Not related to this patchset, but we should think about making the
bad_part_list be less verbose when there is a range of partitions. On
GPT you end up with a giant list like 1, 2, 3, ... 128. Using ranges
would be nicer looking (but lets do that in a different patch).

-- 
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)