Colin Watson wrote:
> Unless specifically told otherwise, the Linux kernel considers extended
> boot records to be two sectors long, in order to "leave room for LILO".
> When using anything other than cylinder alignment, libparted was only
> allowing one sector in the minimum extended partition geometry, which in
> some situations (e.g. following Phillip Susi's patch to reintroduce
> BLKPG) could confuse the kernel into thinking that the EBR and the first
> logical partition overlapped.
>
> * libparted/labels/dos.c (_get_min_extended_part_geom): Allow at least
> two sectors for the extended boot record.
> ---
>  libparted/labels/dos.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
> index 6fb4aef..c25d35a 100644
> --- a/libparted/labels/dos.c
> +++ b/libparted/labels/dos.c
> @@ -1747,7 +1747,11 @@ _get_min_extended_part_geom (const PedPartition* 
> ext_part,
>       min_geom = ped_geometry_duplicate (&walk->geom);
>       if (!min_geom)
>               return NULL;
> -     ped_geometry_set_start (min_geom, walk->geom.start - 1 * head_size);
> +     /* We must always allow at least two sectors at the start, to leave
> +      * room for LILO.  See linux/fs/partitions/msdos.c.
> +      */
> +     ped_geometry_set_start (min_geom,
> +                             walk->geom.start - PED_MAX (1 * head_size, 2));
>
>       for (walk = ext_part->part_list; walk; walk = walk->next) {
>               if (!ped_partition_is_active (walk) || walk->num == 5)

Colin,

Here is your patch followed by a quick attempt at exercising it.
As you can see from the FIXME comment, this test appears to be
exercising different code.  Can you suggest how to fix it?

>From a223da5d2539cc8f33023b049266ab3017dad9cf Mon Sep 17 00:00:00 2001
From: Colin Watson <[email protected]>
Date: Mon, 29 Mar 2010 14:40:54 +0100
Subject: [PATCH 1/2] dos: always allow at least two sectors for extended boot 
record

Unless specifically told otherwise, the Linux kernel considers extended
boot records to be two sectors long, in order to "leave room for LILO".
When using anything other than cylinder alignment, libparted was only
allowing one sector in the minimum extended partition geometry, which in
some situations (e.g. following Phillip Susi's patch to reintroduce
BLKPG) could confuse the kernel into thinking that the EBR and the first
logical partition overlapped.

* libparted/labels/dos.c (_get_min_extended_part_geom): Allow at least
two sectors for the extended boot record.
---
 libparted/labels/dos.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
index 5eee7dd..8f3fcdb 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -1752,7 +1752,11 @@ _get_min_extended_part_geom (const PedPartition* 
ext_part,
        min_geom = ped_geometry_duplicate (&walk->geom);
        if (!min_geom)
                return NULL;
-       ped_geometry_set_start (min_geom, walk->geom.start - 1 * head_size);
+       /* We must always allow at least two sectors at the start, to leave
+        * room for LILO.  See linux/fs/partitions/msdos.c.
+        */
+       ped_geometry_set_start (min_geom,
+                               walk->geom.start - PED_MAX (1 * head_size, 2));

        for (walk = ext_part->part_list; walk; walk = walk->next) {
                if (!ped_partition_is_active (walk) || walk->num == 5)
--
1.7.1.rc0.239.g8b27e


>From 0ea363de119da40544d3b2616ae237a96d0122d1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Tue, 30 Mar 2010 11:57:35 +0200
Subject: [PATCH 2/2] tests: test for new 2-sector minimum ext-logical-separation

* tests/t2310-dos-extended-2-sector-min-offset.sh: New file.
* tests/Makefile.am (TESTS): Add it.
---
 tests/Makefile.am                               |    1 +
 tests/t2310-dos-extended-2-sector-min-offset.sh |   66 +++++++++++++++++++++++
 2 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100644 tests/t2310-dos-extended-2-sector-min-offset.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 49829c5..4667522 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -22,6 +22,7 @@ TESTS = \
   t2100-mkswap.sh \
   t2200-dos-label-recog.sh \
   t2300-dos-label-extended-bootcode.sh \
+  t2310-dos-extended-2-sector-min-offset.sh \
   t2400-dos-hfs-partition-type.sh \
   t3000-resize-fs.sh \
   t3200-type-change.sh \
diff --git a/tests/t2310-dos-extended-2-sector-min-offset.sh 
b/tests/t2310-dos-extended-2-sector-min-offset.sh
new file mode 100644
index 0000000..50dac28
--- /dev/null
+++ b/tests/t2310-dos-extended-2-sector-min-offset.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+# Ensure that parted leaves at least 2 sectors between the beginning
+# of an extended partition and the first logical partition.
+# Before parted-2.3, it could be made to leave just one, and that
+# would cause trouble with the Linux kernel.
+
+# Copyright (C) 2010 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/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  parted --version
+fi
+
+: ${srcdir=.}
+. $srcdir/t-lib.sh
+
+require_root_
+require_scsi_debug_module_
+
+# check for scsi_debug module
+modprobe -n scsi_debug ||
+  skip_test_ "you lack the scsi_debug kernel module"
+
+# create memory-backed device
+scsi_debug_setup_ dev_size_mb=1 > dev-name ||
+  skip_test_ 'failed to create scsi_debug device'
+scsi_dev=$(cat dev-name)
+p1=${scsi_dev}1
+
+cat <<EOF > exp || framework_failure
+BYT;
+$scsi_dev:2048s:scsi:512:512:msdos:Linux scsi_debug;
+1:64s:128s:65s:::lba;
+5:65s:128s:64s:::;
+EOF
+
+fail=0
+
+# Create a DOS label with an extended partition starting at sector 64.
+parted -s $scsi_dev mklabel msdos || fail=1
+parted --align=min -s $scsi_dev mkpart extended 64s 128s> out 2>&1 || fail=1
+parted -m -s $scsi_dev u s print
+compare out /dev/null || fail=1
+
+# Provoke a failure by starting just one sector after start of extended part.
+# FIXME: currently this does not fail as I expected it would.
+parted --align=min -s $scsi_dev mkpart logical  65s 128s > out 2>&1 || fail=1
+compare out /dev/null || fail=1
+
+parted -m -s $scsi_dev u s print > out 2>&1
+compare out exp || fail=1
+
+Exit $fail
--
1.7.1.rc0.239.g8b27e

_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel

Reply via email to