Michael Reed wrote:
> Comments below.
More comments below....
>
> Jim Meyering wrote:
>> Michael Reed <[EMAIL PROTECTED]> wrote:
>>> (I'm copying parted-devel as this ties into GPT-MBR hybrid discussion in
>>> that
>>> there is a need to be able to boot BIOS based systems from disks larger than
>>> 2TB and still be able to address all the storage.)
>>>
>>> Using an on board hardware raid5 "disk" of 5,851,561,984 512 byte sectors,
>>> or roughly 2.9TB, we installed name_brand_Enterprise_distro. We created an
>>> 800 MB swap partition (1), a 25GB root partition (2), and the rest of the
>>> space
>>> to a large scratch xfs file system.
>>>
>>> The OS installation went fine and upon reboot the system complained that the
>>> large scratch file system was, essentially, corrupt. Here's my analysis of
>>> what happened.
>>>
>>> Via analysis of the MSDOS partition table, I can state that it can address
>>> a maximum of 4TB of space. Each partition entry consists of a starting
>>> block
>>> and a length. Both of these fields are 32 bits in length. No individual
>>> partition can begin beyond 2TB. No individual partition can be longer than
>>> 2TB. In order to address 4TB, at least two partition table entries must be
>>> used, for example:
>>>
>>> start length
>>> 0x00000000 0xfffffffe
>>> 0xffffffff 0xffffffff
>>>
>>> (It's interesting to note that Linux will handle 4TB of space defined via
>>> the msdos label, and Windows 2003/2008 appears to stop at 2TB.)
>>>
>>> Working with parted after the system was up I can draw these conclusions:
>>>
>>> 1) parted doesn't complain that a partition length is too long
>>> for the msdos label in which it will be stored.
>>>
>>> 2) parted "truncates" the partition
>>>
>>> 3) parted "lies" to the OS about the size of the partition.
>> Thank you for the detailed bug report!
>> With the patch below, I've fixed Parted so it won't do that any more.
>
> I will try to find real hardware with which to test this today. I'll
> let you know how it goes. Please hold off on posting the patch until
> I can play with it. You'll hear back from me Monday (CA business
> hours) at the latest.
>
> Does this effect the ability to create > 2TB with GPT labels?
If I had READ THE PATCH more closely, I would have noticed the conditioning
on the msdos label:
if (!(part->type & PED_PARTITION_METADATA)
&& strcmp (disk->type->name, "msdos") == 0) {
This clearly doesn't effect GPT labels.
Mike
>
> Thank you for taking the time to do this work.
>
> Comments within the patch....
>
>> The patch to parted is deceptively simple looking.
>> Just finding the right place in the call tree to insert the
>> tests was not easy. Name/semantic mismatches (and lack of comments)
>> led me to experiment with two others first.
>>
>> An early version of the patch did not have this guard condition:
>>
>> if (!(part->type & PED_PARTITION_METADATA)
>>
>> Everything appeared to work until I tried to *print*
>> a partition table containing the largest (2TB-1-sector) partition.
>> Without the guard, it'd complain about the starting sector being too large.
>> The actual sector number may have been 2^32+2k.
>>
>> Designing the tests was a challenge, too.
>> Just creating a file that parted can use as a "device" in which to
>> create such a large partition (>= 2TB) is tricky, since many common
>> file system types have a maximum file size of just under 2TB.
>> But thanks to sparse files, the actual space required is just a few MB.
>>
>> Since I ended up choosing to create and mount an XFS partition, the test
>> does useful work only when run by root. You may also need to install
>> the xfsprogs package to get mkfs.xfs. You can run the test manually
>> like this:
>>
>> (cd tests && sudo ./t4100-msdos-partition-limits.sh --verbose)
>>
>> Here's the change-set I expect to push today or tomorrow:
>> -----------------
>> >From 1bf00ea84e80d9f04aed3c4a15ddb307b1807f5e Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <[EMAIL PROTECTED]>
>> Date: Thu, 10 Jan 2008 14:51:56 +0100
>> Subject: [PATCH] Enforce inherent limitations of the dos partition table
>> format.
>>
>> * libparted/disk.c (_check_partition): Enforce the 32-bit limitation
>> on a partition's starting sector number and length (in sectors).
>> With the usual 512-byte sector size, this limits the maximum
>> partition size to just under 2TB.
>> * tests/t4100-msdos-partition-limits.sh: New file. Test the above.
>> * tests/Makefile.am (TESTS): Add t4100-msdos-partition-limits.sh.
>>
>> Signed-off-by: Jim Meyering <[EMAIL PROTECTED]>
>> ---
>> libparted/disk.c | 41 ++++++++-
>> tests/Makefile.am | 3 +-
>> tests/t4100-msdos-partition-limits.sh | 169
>> +++++++++++++++++++++++++++++++++
>> 3 files changed, 211 insertions(+), 2 deletions(-)
>> create mode 100755 tests/t4100-msdos-partition-limits.sh
>>
>> diff --git a/libparted/disk.c b/libparted/disk.c
>> index 087fbbf..135e230 100644
>> --- a/libparted/disk.c
>> +++ b/libparted/disk.c
>> @@ -1,6 +1,6 @@
>> /*
>> libparted - a library for manipulating disk partitions
>> - Copyright (C) 1999, 2000, 2001, 2002, 2003, 2005, 2007
>> + Copyright (C) 1999, 2000, 2001, 2002, 2003, 2005, 2007, 2008
>> Free Software Foundation, Inc.
>>
>> This program is free software; you can redistribute it and/or modify
>> @@ -1735,6 +1735,45 @@ _check_partition (PedDisk* disk, PedPartition* part)
>> return 0;
>> }
>>
>> + if (!(part->type & PED_PARTITION_METADATA)
>> + && strcmp (disk->type->name, "msdos") == 0) {
>> + /* Enforce some restrictions inherent in the DOS
>> + partition table format. Without these, one would be able
>> + to create a 2TB partition (or larger), and it would work,
>> + but only until the next reboot. This was insidious: the
>> + too-large partition would work initially, because with
>> + Linux-2.4.x and newer we set the partition start sector
>> + and length (in sectors) accurately and directly via the
>> + BLKPG ioctl. However, only the last 32 bits of each
>> + number would be written to the partition table, and the
>> + next time the system would read/use those corrupted numbers
>> + it would usually complain about an invalid partition.
>> + The same applies to the starting sector number. */
>> +
>> + /* The partition length, in sectors, must fit in 32 bytes. */
>
> You probably want this to be "32 bits".
>
>> + if (part->geom.length > UINT32_MAX) {
>> + ped_exception_throw (
>> + PED_EXCEPTION_ERROR,
>> + PED_EXCEPTION_CANCEL,
>> + _("partition length of %jd sectors exceeds"
>> + " the DOS-partition-table-imposed maximum"
>> + " of 2^32-1"),
>> + part->geom.length);
>> + return 0;
>> + }
>> +
>> + /* The starting sector number must fit in 32 bytes. */
>> + if (part->geom.start > UINT32_MAX) {
>> + ped_exception_throw (
>> + PED_EXCEPTION_ERROR,
>> + PED_EXCEPTION_CANCEL,
>> + _("starting sector number, %jd exceeds"
>> + " the DOS-partition-table-imposed maximum"
>> + " of 2^32-1"), part->geom.start);
>> + return 0;
>> + }
>> + }
>> +
>> return 1;
>> }
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 3a3020e..b9cd205 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -7,7 +7,8 @@ TESTS = \
>> t2000-mkfs.sh \
>> t2100-mkswap.sh \
>> t3000-constraints.sh \
>> - t3100-resize-ext2-partion.sh
>> + t3100-resize-ext2-partion.sh \
>> + t4100-msdos-partition-limits.sh
>>
>> EXTRA_DIST = \
>> $(TESTS) test-lib.sh mkdtemp
>> diff --git a/tests/t4100-msdos-partition-limits.sh
>> b/tests/t4100-msdos-partition-limits.sh
>> new file mode 100755
>> index 0000000..13e32af
>> --- /dev/null
>> +++ b/tests/t4100-msdos-partition-limits.sh
>> @@ -0,0 +1,169 @@
>> +#!/bin/sh
>> +
>> +# Copyright (C) 2008 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/>.
>> +
>> +test_description='msdos: enforce limits on partition start sector and
>> length'
>> +
>> +# Need root privileges to use mount.
>> +privileges_required_=1
>> +
>> +. ./init.sh
>> +
>> +####################################################
>> +# Create and mount a file system capable of dealing with >=2TB files.
>> +# We must be able to create a file with an apparent length of 2TB or larger.
>> +# It needn't be a large file system.
>> +fs=fs_file
>> +mp=`pwd`/mount-point
>> +n=128
>> +
>> +test_expect_success \
>> + 'create an XFS file system' \
>> + '
>> + dd if=/dev/zero of=$fs bs=1MB count=2 seek=20 &&
>> + mkfs.xfs -q $fs &&
>> + mkdir "$mp"
>> +
>> + '
>> +
>> +# Unmount upon interrupt, failure, etc., as well as upon normal completion.
>> +cleanup_() { cd "$test_dir_" && umount "$mp" > /dev/null 2>&1; }
>> +
>> +test_expect_success \
>> + 'mount it' \
>> + '
>> + mount -o loop $fs "$mp" &&
>> + cd "$mp"
>> +
>> + '
>> +dev=loop-file
>> +
>> +do_mkpart()
>> +{
>> + start_sector=$1
>> + end_sector=$2
>> + # echo '********' $(echo $end_sector - $start_sector + 1 |bc)
>> + dd if=/dev/zero of=$dev bs=1b count=2k seek=$end_sector 2> /dev/null &&
>> + parted -s $dev mklabel msdos &&
>> + parted -s $dev mkpart p xfs ${start_sector}s ${end_sector}s
>> +}
>> +
>> +# Specify the starting sector number and length in sectors,
>> +# rather than start and end.
>> +do_mkpart_start_and_len()
>> +{
>> + start_sector=$1
>> + len=$2
>> + end_sector=$(echo $start_sector + $len - 1|bc)
>> + do_mkpart $start_sector $end_sector
>> +}
>> +
>> +test_expect_success \
>> + 'a partition length of 2^32-1 works.' \
>> + '
>> + end=$(echo $n+2^32-2|bc) &&
>> + do_mkpart $n $end
>> + '
>> +
>> +cat > exp <<EOF
>> +Model: (file)
>> +Disk: 4294969470s
>> +Sector size (logical/physical): 512B/512B
>> +Partition Table: msdos
>> +
>> +Number Start End Size Type File system Flags
>> + 1 ${n}s ${end}s 4294967295s primary
>> +
>> +EOF
>> +
>> +test_expect_success \
>> + 'print the result' \
>> + 'parted -s $dev unit s p > out 2>&1 &&
>> + sed "s/Disk .*:/Disk:/;s/ *$//" out > k && mv k out &&
>> + diff -u out exp
>> + '
>> +
>> +test_expect_failure \
>> + 'a partition length of exactly 2^32 sectors provokes failure.' \
>> + 'do_mkpart $n $(echo $n+2^32-1|bc) > err 2>&1'
>> +
>> +msg='Error: partition length of 4294967296 sectors exceeds the '\
>> +'DOS-partition-table-imposed maximum of 2^32-1'
>> +test_expect_success \
>> + 'check for new diagnostic' \
>> + 'echo "$msg" > exp && diff -u err exp'
>> +
>> +# FIXME: investigate this.
>> +# Unexpectedly to me, both of these failed with this same diagnostic:
>> +#
>> +# Error: partition length of 4294967296 sectors exceeds the \
>> +# DOS-partition-table-imposed maximum of 2^32-1" > exp &&
>> +#
>> +# I expected the one below to fail with a length of _4294967297_.
>> +# Debugging, I see that _check_partition *does* detect this,
>> +# but the diagnostic doesn't get displayed because of the wonders
>> +# of parted's exception mechanism.
>> +
>> +test_expect_failure \
>> + 'a partition length of 2^32+1 sectors provokes failure.' \
>> + 'do_mkpart $n $(echo $n+2^32|bc) > err 2>&1'
>> +
>> +test_expect_success \
>> + 'check for new diagnostic' \
>> + 'echo "$msg" > exp && diff -u err exp'
>> +
>> +# =========================================================
>> +# Now consider partition starting sector numbers.
>> +msg='Error: starting sector number, 4294967296 exceeds the '\
>> +'DOS-partition-table-imposed maximum of 2^32-1'
>> +
>> +test_expect_success \
>> + 'a partition start sector number of 2^32-1 works.' \
>> + 'do_mkpart_start_and_len $(echo 2^32-1|bc) 1000'
>> +
>> +cat > exp <<EOF
>> +Model: (file)
>> +Disk: 4294970342s
>> +Sector size (logical/physical): 512B/512B
>> +Partition Table: msdos
>> +
>> +Number Start End Size Type File system Flags
>> + 1 4294967295s 4294968294s 1000s primary
>> +
>> +EOF
>> +
>> +test_expect_success \
>> + 'print the result' \
>> + 'parted -s $dev unit s p > out 2>&1 &&
>> + sed "s/Disk .*:/Disk:/;s/ *$//" out > k && mv k out &&
>> + diff -u out exp
>> + '
>> +
>> +test_expect_failure \
>> + 'a partition start sector number of 2^32 must fail.' \
>> + 'do_mkpart_start_and_len $(echo 2^32|bc) 1000 > err 2>&1'
>> +test_expect_success \
>> + 'check for new diagnostic' \
>> + 'echo "$msg" > exp && diff -u err exp'
>> +
>> +test_expect_failure \
>> + 'a partition start sector number of 2^32+1 must fail, too.' \
>> + 'do_mkpart_start_and_len $(echo 2^32+1|bc) 1000 > err 2>&1'
>> +test_expect_success \
>> + 'check for new diagnostic' \
>> + 'echo "$msg" > exp && diff -u err exp'
>> +
>> +test_done
>> --
>> 1.5.4.rc2.85.g71fd
>>
>> _______________________________________________
>> parted-devel mailing list
>> [EMAIL PROTECTED]
>> http://lists.alioth.debian.org/mailman/listinfo/parted-devel
>>
>>
>
>
> _______________________________________________
> parted-devel mailing list
> [EMAIL PROTECTED]
> http://lists.alioth.debian.org/mailman/listinfo/parted-devel
>
>
_______________________________________________
bug-parted mailing list
bug-parted@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-parted
