[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