[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH parted 2/5] parted: add --align commandline option to specify
From: |
Jim Meyering |
Subject: |
Re: [PATCH parted 2/5] parted: add --align commandline option to specify mkpart alignment |
Date: |
Thu, 10 Dec 2009 13:41:49 +0100 |
Hans de Goede wrote:
> The new --align commandline option can have the following values:
> none: Use the minimum alignment allowed by the disk type
> cyl: Align partitions to cylinders (the default)
> min: Use minimum alignment as given by the disk topology information
> opt: Use optimum alignment as given by the disk topology information
Thanks for doing this!
One nit: I want the --align={...} options to be complete words, not just
3-letter abbreviations. I will adjust this patch to use gnulib's argmatch
module. That will permit any unique abbreviation, i.e., --align={n,c,m,o} as
well as spelled-out --align=cylinder. Would you please adjust 3/5 NEWS
and the documentation, accordingly?
> Note the min and opt values will use layout information provided by the
> disk to align the logical partition table addresses to actual physical
> blocks on the disks. The min value is the minimum aligment needed to
> align the partition properly to physical blocks, which avoids
> performance degradation. Where as the optimum aligment align's to a
> multiple of the physical block size in a way that guarantees optimal
> performance.
>
> The min and opt values will only work when compiled with
> libblkid >= 2.17 and running on a kernel >= 2.6.31, otherwise they will
> behave as the none --align value.
...
> @@ -719,6 +729,11 @@ do_mkpart (PedDevice** dev)
> if (!disk)
> goto error;
>
> + if (ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT))
> + if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT,
> + alignment == ALIGNMENT_CYLINDER))
> + goto error_destroy_disk;
Hmm... on a related note, can ped_disk_is_flag_available simply test
for whether ops->disk_set_flag is non-NULL? I.e., is there a reason
not to eliminate the ops->disk_is_flag_available member function?