Bug#579948: [parted-devel] Some debugging info
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.
>
>
> From: Colin Watson
> 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).
Thanks! I've applied that.
However, since it would have made a test fail,
I first applied this change:
>From a582ca642f4817dd02e65a3ecc55e951008969b2 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 26 Jun 2010 09:22:59 +0200
Subject: [PATCH] tests: adjust sun-partition-creating test to conform
* tests/t4000-sun-raid-type.sh: Adjust partition size so the
end falls on a cylinder boundary.
---
tests/t4000-sun-raid-type.sh |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/t4000-sun-raid-type.sh b/tests/t4000-sun-raid-type.sh
index 5ef8ab4..853809e 100755
--- a/tests/t4000-sun-raid-type.sh
+++ b/tests/t4000-sun-raid-type.sh
@@ -26,7 +26,7 @@ ss=$sector_size_
N=2000 # number of sectors
dev=sun-disk-file
-exp="BYT;\n---:${N}s:file:$ss:$ss:sun:;\n1:0s:50s:51s"
+exp="BYT;\n---:${N}s:file:$ss:$ss:sun:;\n1:0s:127s:128s"
test_expect_success \
'create an empty file as a test disk' \
'dd if=/dev/zero of=$dev bs=${ss}c count=$N 2> /dev/null'
@@ -38,7 +38,7 @@ test_expect_success 'check for empty output' 'compare out
/dev/null'
test_expect_success \
'create a single partition' \
-'parted -s $dev unit s mkpart ext2 0s 50s > out 2>&1'
+'parted -s $dev unit s mkpart ext2 0s 127s > out 2>&1'
test_expect_success 'check for empty output' 'compare out /dev/null'
test_expect_success \
--
1.7.1.755.geb6f2
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#579948: [parted-devel] Some debugging info
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 10001
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
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,
Bug#579948: [parted-devel] Some debugging info
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 10001
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
> 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 (par
Bug#579948: [parted-devel] Some debugging info
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.
From: Colin Watson
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 [cjwat...@debian.org]
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#579948: [parted-devel] Some debugging info
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.
Karel
--
Karel Zak
http://karelzak.blogspot.com
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
