[parted-devel] Resize removal

Petr Uzel petr.uzel at suse.cz
Tue Dec 6 12:20:32 UTC 2011


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


> Attaching my complete series.
> 

> 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;
> +        
> +
> +        disk = ped_disk_new (*dev);
> +        if (!disk)
> +                goto error;
> +
> +        if (ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT))
> +                if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT,
> +                                       alignment == ALIGNMENT_CYLINDER))
> +                        goto error_destroy_disk;
> +
> +        if (!command_line_get_partition (_("Partition number?"), disk, &part))
> +                goto error_destroy_disk;
> +        if (!_partition_warn_busy (part))
> +                goto error_destroy_disk;
> +
> +        start = part->geom.start;
> +        end = oldend = part->geom.end;
> +        if (!command_line_get_sector (_("End?"), *dev, &end, &range_end, NULL))
> +                goto error_destroy_disk;
> +        /* Do not move start of the partition */
> +        constraint = constraint_from_start_end_fixed_start (*dev, start, range_end);
> +        if (!ped_disk_set_partition_geom (disk, part, constraint,
> +                                          start, end))
> +                goto error_destroy_constraint;
> +        /* warn when shrinking partition - might lose data */
> +        if (part->geom.end < oldend)
> +                if (ped_exception_throw (
> +                            PED_EXCEPTION_WARNING,
> +                            PED_EXCEPTION_YES_NO,
> +                            _("Shrinking a partition can cause data loss, " \
> +                              "are you sure you want to continue?")) != PED_EXCEPTION_YES)
> +                        goto error_destroy_constraint;
> +        ped_disk_commit (disk);
> +        ped_disk_destroy (disk);
> +        ped_constraint_destroy (constraint);
> +        if (range_end != NULL)
> +                ped_geometry_destroy (range_end);
> +
> +        if ((*dev)->type != PED_DEVICE_FILE)
> +                disk_is_modified = 1;
> +
> +        return 1;
> +
> +error_destroy_constraint:
> +        ped_constraint_destroy (constraint);
> +error_destroy_disk:
> +        ped_disk_destroy (disk);
> +error:
> +        if (range_end != NULL)
> +                ped_geometry_destroy (range_end);
> +        return 0;
> +}
> +
> +
>  static int
>  do_rm (PedDevice** dev)
>  {
> @@ -1820,6 +1902,15 @@ _("rescue START END                         rescue a lost partition near "
>  NULL),
>          str_list_create (_(start_end_msg), NULL), 1));
>  
> +//XXX: mention that this command does never move start of the partition
> +command_register (commands, command_create (
> +        str_list_create_unique ("resize", _("resize"), NULL),
> +        do_resize,
> +        str_list_create (
> +_("resize NUMBER END                        resize partition NUMBER"),
> +NULL),
> +        str_list_create (_(number_msg), _(end_msg), NULL), 1));
> +
>  command_register (commands, command_create (
>          str_list_create_unique ("rm", _("rm"), NULL),
>          do_rm,
> -- 
> 1.7.5.4
> 
> 

> From a5638fc30af3241868df4f6a9f51341f92baa929 Mon Sep 17 00:00:00 2001
> From: Petr Uzel <petr.uzel at suse.cz>
> Date: Tue, 27 Sep 2011 09:11:29 +0200
> Subject: [PATCH 2/4] tests: excersise resize command
> 
> a lot of TODOs
> ---
>  tests/Makefile.am               |    1 +
>  tests/t3200-resize-partition.sh |   92 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+), 0 deletions(-)
>  create mode 100755 tests/t3200-resize-partition.sh
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 616684e..9837029 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -36,6 +36,7 @@ TESTS = \
>    t2310-dos-extended-2-sector-min-offset.sh \
>    t2400-dos-hfs-partition-type.sh \
>    t2500-probe-corrupt-hfs.sh \
> +  t3200-resize-partition.sh \
>    t3200-type-change.sh \
>    t3300-palo-prep.sh \
>    t3310-flags.sh \
> diff --git a/tests/t3200-resize-partition.sh b/tests/t3200-resize-partition.sh
> new file mode 100755
> index 0000000..0735ce0
> --- /dev/null
> +++ b/tests/t3200-resize-partition.sh
> @@ -0,0 +1,92 @@
> +#!/bin/sh
> +# exercise the resize sub-command
> +# based on t3000-resize-fs.sh test
> +
> +# Copyright (C) 2009-2011 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +. "${srcdir=.}/init.sh"; path_prepend_ ../parted
> +
> +require_root_
> +require_scsi_debug_module_
> +
> +ss=$sector_size_
> +
> +default_start=1024s
> +default_end=2048s
> +
> +# create memory-backed device
> +scsi_debug_setup_ dev_size_mb=5 > dev-name ||
> +  skip_ 'failed to create scsi_debug device'
> +dev=$(cat dev-name)
> +
> +# TODO test simple shrink
> +# TODO test expand past end of the disk
> +# TODO test expand past begin of next partition
> +# TODO test shrink before start
> +# TODO test everything with GPT
> +# TODO more tests with extended/logical partitions
> +
> +parted -s $dev mklabel msdos > out 2> err || fail=1
> +# expect no output
> +compare out /dev/null || fail=1
> +compare err /dev/null || fail=1
> +
> +# ensure that the disk is large enough
> +dev_n_sectors=$(parted -s $dev u s p|sed -n '2s/.* \([0-9]*\)s$/\1/p')
> +device_sectors_required=$(echo $default_end | sed 's/s$//')
> +# Ensure that $dev is large enough for this test
> +test $device_sectors_required -le $dev_n_sectors || fail=1
> +
> +# create an empty partition
> +parted -s $dev mkpart primary $default_start $default_end > out 2>&1 || fail=1
> +echo "Warning: The resulting partition is not properly" \
> +     "aligned for best performance." > exp
> +compare out exp || fail=1
> +
> +# print partition table
> +parted -m -s $dev u s p > out 2>&1 || fail=1
> +
> +# FIXME: check expected output
> +
> +# wait for new partition device to appear
> +wait_for_dev_to_appear_ ${dev}1 || { warn_ "${dev}1 did not appear"  fail=1; }
> +sleep 1
> +
> +
> +# extend the filesystem to end on sector 4096
> +new_end=4096s
> +parted -s $dev resize 1 $new_end > out 2> err || fail=1
> +# expect no output
> +compare out /dev/null || fail=1
> +compare err /dev/null || fail=1
> +
> +# print partition table
> +parted -m -s $dev u s p > out 2>&1 || fail=1
> +
> +# compare against expected output
> +sed -n 3p out > k && mv k out || fail=1
> +printf "1:$default_start:$new_end:3073s:::$ms;\n" > exp || fail=1
> +compare out exp || fail=1
> +
> +# Remove the partition explicitly, so that mklabel doesn't evoke a warning.
> +parted -s $dev rm 1 || fail=1
> +
> +# Create a clean partition table for the next iteration.
> +parted -s $dev mklabel msdos > out 2>&1 || fail=1
> +# expect no output
> +compare out /dev/null || fail=1
> +
> +Exit $fail
> -- 
> 1.7.5.4
> 
> 

> From ab8e2b2bbcf637117ecd79ed59210927cbc50b8f Mon Sep 17 00:00:00 2001
> From: Phillip Susi <psusi at cfl.rr.com>
> Date: Tue, 29 Nov 2011 14:05:48 -0500
> Subject: [PATCH 3/4] Add support for BLKPG ioctl partition resize
> 
> When resizing a partition ( same partition number, same
> start sector, different end sector ), if removing the old
> partition fails because it is in use, try to use the
> new BLKPG_RES_PARTITION request to update the kernel
> partition table with the new size.
> ---
>  libparted/arch/linux.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 1da3343..2b3baa3 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2428,6 +2428,55 @@ _blkpg_remove_partition (PedDisk* disk, int n)
>                                      BLKPG_DEL_PARTITION);
>  }
>  
> +#ifdef BLKPG_RES_PARTITION
> +static int _blkpg_resize_partition (PedDisk* disk, const PedPartition *part)
> +{
> +        struct blkpg_partition  linux_part;
> +        const char*             vol_name;
> +        char*                   dev_name;
> +
> +        PED_ASSERT(disk != NULL);
> +        PED_ASSERT(disk->dev->sector_size % PED_SECTOR_SIZE_DEFAULT == 0);
> +
> +        if (!_has_partitions (disk))
> +                return 0;
> +        dev_name = _device_get_part_path (disk->dev, part->num);
> +        if (!dev_name)
> +                return 0;
> +        memset (&linux_part, 0, sizeof (linux_part));
> +        linux_part.start = part->geom.start * disk->dev->sector_size;
> +        /* see fs/partitions/msdos.c:msdos_partition(): "leave room for LILO" */
> +        if (part->type & PED_PARTITION_EXTENDED)
> +                linux_part.length = part->geom.length == 1 ? 512 : 1024;
> +        else
> +                linux_part.length = part->geom.length * disk->dev->sector_size;
> +        linux_part.pno = part->num;
> +        strncpy (linux_part.devname, dev_name, BLKPG_DEVNAMELTH);
> +        if (vol_name)
> +                strncpy (linux_part.volname, vol_name, BLKPG_VOLNAMELTH);
> +
> +        free (dev_name);
> +
> +        if (!_blkpg_part_command (disk->dev, &linux_part,
> +                                  BLKPG_RES_PARTITION)) {
> +                return ped_exception_throw (
> +                        PED_EXCEPTION_ERROR,
> +                        PED_EXCEPTION_IGNORE_CANCEL,
> +                        _("Error informing the kernel about modifications to "
> +                          "partition %s -- %s.  This means Linux won't know "
> +                          "about any changes you made to %s until you reboot "
> +                          "-- so you shouldn't mount it or use it in any way "
> +                          "before rebooting."),
> +                        linux_part.devname,
> +                        strerror (errno),
> +                        linux_part.devname)
> +                                == PED_EXCEPTION_IGNORE;
> +        }
> +
> +        return 1;
> +}
> +#endif
> +
>  /* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL
>     to that value, where DEV_BASE is the last component of DEV->path.
>     Upon success, return true.  Otherwise, return false. */
> @@ -2596,6 +2645,15 @@ _disk_sync_part_table (PedDisk* disk)
>                                  if (geom.start == part->geom.start
>  				    && length == part->geom.length)
>                                          ok[i - 1] = 1;
> +#ifdef BLKPG_RES_PARTITION
> +                                if (geom.start == part->geom.start
> +                                    && length != part->geom.length)
> +                                {
> +                                        /* try to resize */
> +                                        if (_blkpg_resize_partition (disk, part))
> +                                                ok[i - 1] = 1;
> +                                }
> +#endif
>                                  /* If the new partition is unchanged and the
>  				   existing one was not removed because it was
>  				   in use, then reset the error flag and do not
> -- 
> 1.7.5.4
> 
> 

> From 7e967911132839d97ce90a2db154d8aedaee49fa Mon Sep 17 00:00:00 2001
> From: Phillip Susi <psusi at cfl.rr.com>
> Date: Wed, 30 Nov 2011 13:13:58 -0500
> Subject: [PATCH 4/4] Make _partition_warn_busy actually a warning instead of
>  an error
> 
> This function was throwing a PED_EXCEPTION_ERROR with only the
> PED_EXCEPTION_CANCEL option.  Converted to a PED_EXCEPTION_WARNING.
> ---
>  parted/parted.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/parted/parted.c b/parted/parted.c
> index c5148a2..680fb79 100644
> --- a/parted/parted.c
> +++ b/parted/parted.c
> @@ -222,14 +222,16 @@ _partition_warn_busy (PedPartition* part)
>  
>          if (ped_partition_is_busy (part)) {
>                  path = ped_partition_get_path (part);
> -                ped_exception_throw (
> -                        PED_EXCEPTION_ERROR,
> -                        PED_EXCEPTION_CANCEL,
> -                        _("Partition %s is being used. You must unmount it "
> -                          "before you modify it with Parted."),
> -                        path);
> -                free (path);
> -                return 0;
> +                if (ped_exception_throw (
> +                            PED_EXCEPTION_WARNING,
> +                            PED_EXCEPTION_YES_NO,
> +                            _("Partition %s is being used. Are you sure you " \
> +                              "want to continue?"),
> +                            path) != PED_EXCEPTION_YES)
> +                {
> +                        free (path);
> +                        return 0;
> +                }
>          }
>          return 1;
>  }
> -- 
> 1.7.5.4
> 
> 


Petr

--
Petr Uzel
IRC: ptr_uzl @ freenode
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20111206/8289cd3d/attachment.pgp>


More information about the parted-devel mailing list