Re: [PATCH] make parted "do what I say" with IEC start and end values like 5GiB

[Jim Meyering]

Brian C. Lane wrote:
> On Fri, Mar 18, 2011 at 03:12:51PM +0100, Jim Meyering wrote:
>> Following up on a suggestion I made a month or so ago,
>> here's a proposed patch that's nearly complete (still no doc
>> or NEWS update, but the log should be close to what I'll put in .texi):
>
> I like this. It may also be helpful in solving (or providing a
> work-around) to rhbz#623268 where it sometimes snaps to the wrong place.

Hi Brian,

Thanks for the feedback.
I've added a paragraph to the info documentation, a note in NEWS,
and corrected some minor details in the commit log.
Here's what I expect to push:

>From 552805a03a79f7b6badaf26536d86db580360f98 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 22 Feb 2011 16:12:28 +0100
Subject: [PATCH] make parted "do what I say" with IEC start and end values
 like 5GiB

In a command like this,
  parted -s -- $dev mklabel gpt mkpart P-NAME 4MB -34s
parted interprets the "4MB" as merely a suggestion for the starting
sector number.  It uses half of the MB-units value as a "radius"
about the sector containing byte 4,000,000, and it may choose some
other more appropriate sector, based on partition table or alignment
constraints within the range 3,500,000B..4,500,000B.
Before, parted handled IEC units, i.e., KiB, MiB, GiB, etc.,
with identically "helpful" sloppiness, of course honoring the
power-of-two semantics.
Now, however if you use IEC units, i.e., KiB, MiB, GiB, etc.,
the "radius" is taken to be zero, so parted uses precisely
whatever multiple of a power of two you've specified.
Hence, adjusting the example above to use MiB, rather than "MB",
  parted -s -- $dev mklabel gpt mkpart P-NAME 4MiB -34s
With this change, that is equivalent to the following:
  parted -s -- $dev mklabel gpt mkpart P-NAME 4194304B -34s
I.e., it uses the sector containing precisely that byte, and
does not perform any "extra" adjustment.
* libparted/unit.c (is_power_of_2): New function.
(ped_unit_parse_custom): Use it to avoid interpreting a large
input string as "sloppy" (i.e. large radius) when it uses IEC
binary notation like 34KiB, 3GiB and 65TiB.
* tests/t0207-IEC-binary-notation.sh: New test.
* tests/Makefile.am (TESTS): Add it.
* doc/parted.texi (unit): Describe the new behavior.
* NEWS (Changes in behavior): Mention it here, too.
---
 NEWS                               |    9 ++++++++
 doc/parted.texi                    |   16 ++++++++++++-
 libparted/unit.c                   |   13 +++++++++++
 tests/Makefile.am                  |    1 +
 tests/t0207-IEC-binary-notation.sh |   40 ++++++++++++++++++++++++++++++++++++
 5 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 tests/t0207-IEC-binary-notation.sh

diff --git a/NEWS b/NEWS
index b7143eb..92e8ea7 100644
--- a/NEWS
+++ b/NEWS
@@ -50,6 +50,15 @@ GNU parted NEWS                                    -*- 
outline -*-
   "parted $dev print" now prints information about the device (model, size,
   transport, sector size) even when it fails to recognize the disk label.

+  specifying partition start or end values using MiB, GiB, etc. suffixes
+  now makes parted do what I want, i.e., use that precise value, and not
+  some other that is up to 500KiB or 500MiB away from what I specified.
+  Before, to get that behavior, you would have had to use carefully chosen
+  values with units of bytes ("B") or sectors ("s") to obtain the same
+  result, and with sectors, your usage would not be portable between devices
+  with varying sector sizes.  This change does not affect how parted handles
+  suffixes like KB, MB, GB, etc.
+

 * Noteworthy changes in release 2.3 (2010-05-28) [stable]

diff --git a/doc/parted.texi b/doc/parted.texi
index 1a57c07..ecb6574 100644
--- a/doc/parted.texi
+++ b/doc/parted.texi
@@ -1175,14 +1175,26 @@ which case this unit apply instead of the default unit 
for this
 particular number, but CHS and cylinder units are not supported as
 a suffix.  If no suffix is given, then the default unit is assumed.
 Parted will compute sensible ranges for the locations you specify
-(e.g. a range of +/- 500 MB when you specify the location in ``G'')
+(e.g. a range of +/- 500 MB when you specify the location in ``G'',
+and a range of +/- 500 KB when you specify the location in ``M'')
 and will select the nearest location in this range from the one you
 wrote that satisfies constraints from both the operation, the
 filesystem being worked on, the disk label, other partitions and so
 on.  Use the sector unit ``s'' to specify exact locations (if they
 do not satisfy all constraints, Parted will ask you for the nearest
 solution).  Note that negative numbers count back from the end of
-the disk, with ``-1s'' pointing to the end of the disk.
+the disk, with ``-1s'' pointing to the last sector of the disk.
+
+Note that as of parted-2.4, when you specify start and/or end values
+using IEC binary units like ``MiB'', ``GiB'', ``TiB'', etc., parted
+treats those values as exact, and equivalent to the same number
+specified in bytes (i.e., with the ``B'' suffix), in that it provides
address@hidden ``helpful'' range of sloppiness.  Contrast that with
+a partition start request of ``4GB'', which may actually resolve to
+some sector up to 500MB before or after that point.
+Thus, when creating a partition, you should prefer to specify
+units of bytes (``B''), sectors (``s''), or IEC binary units like ``MiB'',
+but not ``MB'', ``GB'', etc.

 Example:

diff --git a/libparted/unit.c b/libparted/unit.c
index 5885d83..dc4205b 100644
--- a/libparted/unit.c
+++ b/libparted/unit.c
@@ -480,6 +480,12 @@ parse_unit_suffix (const char* suffix, PedUnit 
suggested_unit)
        return suggested_unit;
 }

+static bool
+is_power_of_2 (long long n)
+{
+  return (n & (n - 1)) == 0;
+}
+
 /**
  * If \p str contains a valid description of a location on \p dev, then
  * \p *sector is modified to describe the location and a geometry is created
@@ -530,6 +536,13 @@ ped_unit_parse_custom (const char* str, const PedDevice* 
dev, PedUnit unit,
        radius = ped_div_round_up (unit_size, dev->sector_size) - 1;
        if (radius < 0)
                radius = 0;
+       /* If the user specifies units in a power of 2, e.g., 4MiB, as in
+              parted -s -- $dev mklabel gpt mkpart P-NAME 4MiB -34s
+          do not use 4MiB as the range.  Rather, presume that they
+          are specifying precisely the starting or ending number,
+          and treat "4MiB" just as we would treat "4194304B".  */
+       if (is_power_of_2 (unit_size))
+               radius = 0;

        *sector = num * unit_size / dev->sector_size;
        /* negative numbers count from the end */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4f95429..93e5995 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -13,6 +13,7 @@ TESTS = \
   t0202-gpt-pmbr.sh \
   t0205-gpt-list-clobbers-pmbr.sh \
   t0206-gpt-print-with-corrupt-primary-clobbers-pmbr.sh \
+  t0207-IEC-binary-notation.sh \
   t0220-gpt-msftres.sh \
   t0250-gpt.sh \
   t0280-gpt-corrupt.sh \
diff --git a/tests/t0207-IEC-binary-notation.sh 
b/tests/t0207-IEC-binary-notation.sh
new file mode 100644
index 0000000..2228ce1
--- /dev/null
+++ b/tests/t0207-IEC-binary-notation.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+# Show how parted treats a starting or ending sector number w/IEC units.
+
+# Copyright (C) 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
+
+ss=$sector_size_
+n_sectors=3000
+dev=dev-file
+
+dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1
+parted --align=none -s $dev mklabel gpt mkpart p1 $((64*1024))B 
$((1024*1024))B \
+    > err 2>&1 || fail=1
+compare err /dev/null || fail=1
+parted -m -s $dev u s p > exp || fail=1
+
+rm $dev
+dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1
+parted --align=none -s $dev mklabel gpt mkpart p1 64KiB 1MiB \
+    > err 2>&1 || fail=1
+compare err /dev/null || fail=1
+parted -m -s $dev u s p > out || fail=1
+
+compare out exp || fail=1
+
+Exit $fail
--
1.7.4.1.686.g46300