[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?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]