[Parted-maintainers] Bug#579948: [parted-devel] Some debugging info

[Hans de Goede]

Hi,

I did not realize that partitions starts must be
cylinder aligned on Sun, ack for the patch reverting
the Sun disklabel part of my patch.

Regards,

Hans


On 06/20/2010 11:03 AM, Jurij Smakov wrote:
> On Tue, Jun 15, 2010 at 07:53:21PM +0100, Colin Watson wrote:
>> On Mon, Jun 14, 2010 at 08:40:12PM +0200, Karel Zak wrote:
>>> On Mon, Jun 14, 2010 at 01:06:09PM +0100, Colin Watson wrote:
>>>> parted-devel, can anyone comment on this?  It seems to me that either
>>>> (a) _get_lax_constraint should be using ped_alignment_any for its start
>>>> alignment, rather than aligning to sectors * heads boundaries, or (b)
>>>> Sun labels really and truly require cylinder alignment, in which case
>>>> requests to use optimal alignment shouldn't be honoured.
>>>
>>> The begin of the partition has to be defined in cylinders:
>>>
>>>          struct __attribute__ ((packed)) _SunRawPartition {
>>>                  u_int32_t       start_cylinder; /* where the part starts... */
>>>                  u_int32_t       num_sectors;    /* ...and it's length */
>>>          };
>>>
>>> IMHO it does not make sense to use non-cylinder alignment here.
>>
>> In that case, I think that the sun part of
>> 723ea23c5df68cbe67d1f518ef484f4c77f516fa should be reverted.  CCing Hans
>> since that was his change.
>
> I have tried this patch out (by building custom
> libparted0-udeb_2.2-7_sparc.udeb and parted-udeb_2.2-7_sparc.udeb, and
> fetching/installing them during installation, before partitioning
> step). Unfortunately, I still get 16.8GB partitions, even though their
> size is now calculated correctly, so I don't get free space gaps as
> before. Example from the installer log:
>
> parted_server: Closing infifo and outfifo
> parted_server: main_loop: iteration 75
> parted_server: Opening infifo
> /bin/perform_recipe: IN: NEW_PARTITION =dev=sdb primary ext2
> 0-72908881919 beginning 100000001
> parted_server: Read command: NEW_PARTITION
> parted_server: command_new_partition()
> parted_server: Note =dev=sdb as changed
> parted_server: Opening outfifo
> parted_server: requested partition with type primary
> parted_server: requested partition with file system ext2
> parted_server: add_primary_partition(disk(142410400),0-195312)
> parted_server: OUT: OK
>
>
> parted_server: OUT: 1   0-16845373439   16845373440     primary ext2
> /dev/sdb1
>
>>
>> From: Colin Watson<cjwatson at ubuntu.com>
>> Date: Tue, 15 Jun 2010 19:49:40 +0100
>> Subject: [PATCH] sun: revert "implement disk flag operations"
>>
>> This reverts the libparted/labels/sun.c part of
>> 723ea23c5df68cbe67d1f518ef484f4c77f516fa.  Sun disk labels do not appear
>> to be able to handle non-cylinder alignment
>> (http://bugs.debian.org/579948).
>> ---
>>   libparted/labels/sun.c |   47 +----------------------------------------------
>>   1 files changed, 1 insertions(+), 46 deletions(-)
>>
>> diff --git a/libparted/labels/sun.c b/libparted/labels/sun.c
>> index 177a47c..e14a81d 100644
>> --- a/libparted/labels/sun.c
>> +++ b/libparted/labels/sun.c
>> @@ -109,7 +109,6 @@ struct _SunPartitionData {
>>   struct _SunDiskData {
>>   	PedSector		length; /* This is based on cyl - alt-cyl */
>>   	SunRawLabel		raw_label;
>> -	int                     cylinder_alignment;
>>   };
>>
>>   static PedDiskType sun_disk_type;
>> @@ -191,7 +190,6 @@ sun_alloc (const PedDevice* dev)
>>   	PED_ASSERT (bios_geom->cylinders == (PedSector) (dev->length / cyl_size),
>>                       return NULL);
>>   	sun_specific->length = ped_round_down_to (dev->length, cyl_size);
>> -        sun_specific->cylinder_alignment = 1;
>>
>>   	label =&sun_specific->raw_label;
>>   	memset(label, 0, sizeof(SunRawLabel));
>> @@ -258,42 +256,6 @@ sun_free (PedDisk *disk)
>>   }
>>
>>   static int
>> -sun_disk_set_flag (PedDisk *disk, PedDiskFlag flag, int state)
>> -{
>> -        SunDiskData *disk_specific = disk->disk_specific;
>> -        switch (flag) {
>> -        case PED_DISK_CYLINDER_ALIGNMENT:
>> -                disk_specific->cylinder_alignment = !!state;
>> -                return 1;
>> -        default:
>> -                return 0;
>> -        }
>> -}
>> -
>> -static int
>> -sun_disk_get_flag (const PedDisk *disk, PedDiskFlag flag)
>> -{
>> -        SunDiskData *disk_specific = disk->disk_specific;
>> -        switch (flag) {
>> -        case PED_DISK_CYLINDER_ALIGNMENT:
>> -                return disk_specific->cylinder_alignment;
>> -        default:
>> -                return 0;
>> -        }
>> -}
>> -
>> -static int
>> -sun_disk_is_flag_available (const PedDisk *disk, PedDiskFlag flag)
>> -{
>> -        switch (flag) {
>> -        case PED_DISK_CYLINDER_ALIGNMENT:
>> -               return 1;
>> -        default:
>> -               return 0;
>> -        }
>> -}
>> -
>> -static int
>>   _check_geometry_sanity (PedDisk* disk, SunRawLabel* label)
>>   {
>>   	PedDevice*	dev = disk->dev;
>> @@ -804,10 +766,7 @@ sun_partition_align (PedPartition* part, const PedConstraint* constraint)
>>   {
>>           PED_ASSERT (part != NULL, return 0);
>>
>> -        SunDiskData *disk_specific = part->disk->disk_specific;
>> -
>> -        if (disk_specific->cylinder_alignment&&
>> -            _ped_partition_attempt_align (part, constraint,
>> +        if (_ped_partition_attempt_align (part, constraint,
>>                                             _get_strict_constraint (part->disk)))
>>                   return 1;
>>           if (_ped_partition_attempt_align (part, constraint,
>> @@ -921,10 +880,6 @@ static PedDiskOps sun_disk_ops = {
>>   	clobber:		NULL,
>>   	write:			NULL_IF_DISCOVER_ONLY (sun_write),
>>
>> -	disk_set_flag:          sun_disk_set_flag,
>> -	disk_get_flag:          sun_disk_get_flag,
>> -	disk_is_flag_available: sun_disk_is_flag_available,
>> -
>>   	get_partition_alignment: sun_get_partition_alignment,
>>
>>   	partition_set_name:		NULL,
>> --
>> 1.7.0.4
>>
>>
>> --
>> Colin Watson                                       [cjwatson at debian.org]
>