[parted-devel] [PATCH] Always allow at least two sectors for extended boot record

[Jim Meyering]

Petr Uzel wrote:

> On Thu, Apr 08, 2010 at 02:26:28PM +0200, Jim Meyering wrote:
>> 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?
>
> Just a blind guess: did you run the test with parted with 'reintroduce
> BLKPG' patch applied? AFAIK the bug doesn't show with BLKRRPART.

Yes, actually that was exactly it.
I've since been able to show that the test does exercise the fix,
but the changes introduced failures in 2 or 3 root-only tests.
I think I've just fixed the last of those.

I expect to push these two patches and associated changes tomorrow.

Thanks,

Jim