[parted-devel] Resize removal

[Jim Meyering]

Petr Uzel wrote:
> On Fri, Dec 02, 2011 at 04:58:23PM -0500, Phillip Susi wrote:
>> On 11/29/2011 10:27 AM, Petr Uzel wrote:
>> >This has been discussed on this list some time ago:
>> >http://lists.alioth.debian.org/pipermail/parted-devel/2011-June/003873.html
>> >
>> >and it has been on my TODO list since then. I have some code that is
>> >working, but contains quite some TODOs - I'm attaching it here in case
>> >you would like to have a look. If you would like to finish/rewrite it,
>> >I would be more than happy as we need it in openSUSE, but I didn't
>> >find time to finish it myself yet.
>>
>> I removed some unused locals from your patch as well as the special
>> treatment of extended partitions, and added the warning when
>> shrinking partitions. Would you care to write the commit message?
>
> Hi Phillip,
>
> From a quick glance over you changes, it looks like a useful feature
> to me. Before all of this work is ready for inclusion, there's still quite
> some work:
>
> - BLKPG_RES_PARTITION has to be accepted to kernel (any news about
>   this?)
> - I have to address the TODOs in the resize{,part} patch and in the
>   testsuite
> - IMHO your 'online-resize' feature deserves a testsuite as well - would
>   you feel comfortable writing it?

Definitely.

> - write missing commit messages :)
> - last but not least: Jim, what do you think about the concept behind
>   the patches below?

I like the idea, a lot.

>> Attaching my complete series.

Thanks to both of you for pursuing that.
A couple of nits I've just noticed:

  - trailing blanks (make syntax-check should catch those)

  - reversed compare arguments: correct like this:
      -compare out /dev/null
      +compare /dev/null out
    If you rebase this series to apply to the latest on master,
    then "make syntax-check" will catch those, too.

>> From 4a136205b434316e641961379df8d8b259fdeb19 Mon Sep 17 00:00:00 2001
>> From: Petr Uzel <petr.uzel at suse.cz>
>> Date: Mon, 26 Sep 2011 17:21:01 +0200
>> Subject: [PATCH 1/4] parted: resize command
>>
>> TODO
>> ---
>>  parted/parted.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 91 insertions(+), 0 deletions(-)
>>
>> diff --git a/parted/parted.c b/parted/parted.c
>> index 66beba6..c5148a2 100644
>> --- a/parted/parted.c
>> +++ b/parted/parted.c
>> @@ -152,6 +152,9 @@ static const char* fs_type_msg_start = N_("FS-TYPE is one of: ");
>>  static const char* start_end_msg =    N_("START and END are disk locations, such as "
>>                  "4GB or 10%.  Negative values count from the end of the disk.  "
>>                  "For example, -1s specifies exactly the last sector.\n");
>> +static const char* end_msg =          N_("END is disk location, such as "
>> +                "4GB or 10%.  Negative value counts from the end of the disk.  "
>> +                "For example, -1s specifies exactly the last sector.\n");
>>  static const char* state_msg =        N_("STATE is one of: on, off\n");
>>  static const char* device_msg =       N_("DEVICE is usually /dev/hda or /dev/sda\n");
>>  static const char* name_msg =         N_("NAME is any word you want\n");
>> @@ -435,6 +438,21 @@ constraint_from_start_end (PedDevice* dev, PedGeometry* range_start,
>>                  range_start, range_end, 1, dev->length);
>>  }
>>
>> +
>> +static PedConstraint*
>> +constraint_from_start_end_fixed_start (PedDevice* dev, PedSector start_sector,
>> +                           PedGeometry* range_end)
>> +{
>> +        PedGeometry range_start;
>> +        range_start.dev = dev;
>> +        range_start.start = start_sector;
>> +        range_start.end = start_sector;
>> +        range_start.length = 1;
>> +
>> +        return ped_constraint_new (ped_alignment_any, ped_alignment_any,
>> +                &range_start, range_end, 1, dev->length);
>> +}
>> +
>>  void
>>  help_on (char* topic)
>>  {
>> @@ -1456,6 +1474,70 @@ error:
>>          return 0;
>>  }
>>
>> +
>> +static int
>> +do_resize (PedDevice** dev)
>> +{
>> +        PedDisk                 *disk;
>> +        PedPartition            *part = NULL;
>> +        PedSector               start, end, oldend;
>> +        PedGeometry             *range_end = NULL;
>> +        PedConstraint*          constraint;

In new code, please declare as close as possible to the first use.
E.g.,

    PedDisk *disk = ped_disk_new (*dev);

Putting it another way, you should have a good reason to
have any simple declaration after the opening curly brace
of a function definition.

I'll review thoroughly later.