Re: [PATCH 1/5] fdisk: add device topology to the API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

On Sun, Jun 03, 2012 at 08:15:17PM +0200, Davidlohr Bueso wrote:
> From: Davidlohr Bueso <dave@xxxxxxx>
> 
> This patch adds device topology discovery to the internal API. This functionality is static only to the API and therefore hidden
> from general fdisk code. Functionality itself doesn't really change, min_io_size, io_size, logical and physical sector sizes and
> alignment offset are added to the fdisk_context structure and elements are accessed from there. The logical sector size (sector_size)
> is now unsigned long instead of unsigned int, this as no effect otherwise.
> 
> A few things to notice:
>  - The patch is larger than I wanted but we need to modify function parameters across fdisk and its labels to use the topology data from
>    cxt-> instances. Hopefully this will be pretty much it regarding this kind of modifications - perhaps geometry will need something
>    of the like too.
> 
>  - The -b option must override internal discovery.
> 
>  - A new helper function has added to verify if the device provides topology information, this replaces the 'has_topology' global variable.

Could you please wrap the commit message not to have that long lines?
IMHO wrapping the lines makes the message more readable.
72 chars seems to be common value.

Also, please don't add any new trailing whitespace. Git can be
configured to give a warning on committing new trailing whitespace.

> 
> Signed-off-by: Davidlohr Bueso <dave@xxxxxxx>
> ---
>  fdisk/fdisk.c         |  309 ++++++++++++++++++++-----------------------------
>  fdisk/fdisk.h         |   34 ++++--
>  fdisk/fdiskbsdlabel.c |   18 ++--
>  fdisk/fdiskdoslabel.c |   75 ++++++------
>  fdisk/fdiskdoslabel.h |    8 +-
>  fdisk/fdisksgilabel.c |   49 ++++----
>  fdisk/fdisksgilabel.h |    4 +-
>  fdisk/fdisksunlabel.c |   53 +++++----
>  fdisk/fdisksunlabel.h |   12 +-
>  fdisk/utils.c         |   73 ++++++++++++-
>  10 files changed, 329 insertions(+), 306 deletions(-)
> 
> diff --git a/fdisk/fdisk.c b/fdisk/fdisk.c
> index 1143b2f..d7ff2e5 100644
> --- a/fdisk/fdisk.c
> +++ b/fdisk/fdisk.c
> @@ -47,9 +47,6 @@
>  #ifdef HAVE_LINUX_BLKPG_H
>  #include <linux/blkpg.h>
>  #endif
> -#ifdef HAVE_LIBBLKID
> -#include <blkid.h>
> -#endif
>  
>  #include "gpt.h"
>  
> @@ -144,19 +141,12 @@ sector_t sector_offset = 1, sectors;
>  
>  unsigned int	heads,
>  	cylinders,
> -	sector_size = DEFAULT_SECTOR_SIZE,
>  	user_set_sector_size = 0,
>  	units_per_sector = 1,
>  	display_in_cyl_units = 0;
>  
>  sector_t total_number_of_sectors; /* in logical sectors */
> -unsigned long grain = DEFAULT_SECTOR_SIZE,
> -	      io_size = DEFAULT_SECTOR_SIZE,
> -	      min_io_size = DEFAULT_SECTOR_SIZE,
> -	      phy_sector_size = DEFAULT_SECTOR_SIZE,
> -	      alignment_offset;
> -int has_topology;
> -
> +unsigned long grain = DEFAULT_SECTOR_SIZE;
>  enum labeltype disklabel;	/* Current disklabel */
>  
>  static void __attribute__ ((__noreturn__)) usage(FILE *out)
> @@ -318,22 +308,22 @@ test_c(char **m, char *mesg) {
>  }
>  
>  static int
> -lba_is_aligned(sector_t lba)
> +lba_is_aligned(struct fdisk_context *cxt, sector_t lba)
>  {
> -	unsigned int granularity = max(phy_sector_size, min_io_size);
> -	sector_t offset = (lba * sector_size) & (granularity - 1);
> +	unsigned int granularity = max(cxt->phy_sector_size, cxt->min_io_size);

Shouldn't granularity rather be 'unsigned long' to match type of
sector_size and min_io_size to match type of sector_size and
min_io_size ?

> +	sector_t offset = (lba * cxt->sector_size) & (granularity - 1);
>  
> -	return !((granularity + alignment_offset - offset) & (granularity - 1));
> +	return !((granularity + cxt->alignment_offset - offset) & (granularity - 1));
>  }
>  
> -sector_t align_lba(sector_t lba, int direction)
> +sector_t align_lba(struct fdisk_context *cxt, sector_t lba, int direction)
>  {
>  	sector_t res;
>  
> -	if (lba_is_aligned(lba))
> +	if (lba_is_aligned(cxt, lba))
>  		res = lba;
>  	else {
> -		sector_t sects_in_phy = grain / sector_size;
> +		sector_t sects_in_phy = grain / cxt->sector_size;
>  
>  		if (lba < sector_offset)
>  			res = sector_offset;
> @@ -347,8 +337,8 @@ sector_t align_lba(sector_t lba, int direction)
>  		else /* ALIGN_NEAREST */
>  			res = ((lba + sects_in_phy / 2) / sects_in_phy) * sects_in_phy;
>  
> -		if (alignment_offset && !lba_is_aligned(res) &&
> -		    res > alignment_offset / sector_size) {
> +		if (cxt->alignment_offset && !lba_is_aligned(cxt, res) &&
> +		    res > cxt->alignment_offset / cxt->sector_size) {
>  			/*
>  			 * apply alignment_offset
>  			 *
> @@ -357,8 +347,8 @@ sector_t align_lba(sector_t lba, int direction)
>  			 * according the offset to be on the physical boundary.
>  			 */
>  			/* fprintf(stderr, "LBA: %llu apply alignment_offset\n", res); */
> -			res -= (max(phy_sector_size, min_io_size) -
> -					alignment_offset) / sector_size;
> +			res -= (max(cxt->phy_sector_size, cxt->min_io_size) -
> +					cxt->alignment_offset) / cxt->sector_size;
>  
>  			if (direction == ALIGN_UP && res < lba)
>  				res += sects_in_phy;
> @@ -408,31 +398,31 @@ void update_units(void)
>  		units_per_sector = 1;	/* in sectors */
>  }
>  
> -void warn_limits(void)
> +void warn_limits(struct fdisk_context *cxt)
>  {
>  	if (total_number_of_sectors > UINT_MAX && !nowarn) {
> -		unsigned long long bytes = total_number_of_sectors * sector_size;
> +		unsigned long long bytes = total_number_of_sectors * cxt->sector_size;
>  		int giga = bytes / 1000000000;
>  		int hectogiga = (giga + 50) / 100;
>  
>  		fprintf(stderr, _("\n"
>  "WARNING: The size of this disk is %d.%d TB (%llu bytes).\n"
>  "DOS partition table format can not be used on drives for volumes\n"
> -"larger than (%llu bytes) for %d-byte sectors. Use parted(1) and GUID \n"
> +"larger than (%llu bytes) for %ld-byte sectors. Use parted(1) and GUID \n"
>  "partition table format (GPT).\n\n"),
>  			hectogiga / 10, hectogiga % 10,
>  			bytes,
> -			(sector_t ) UINT_MAX * sector_size,
> -			sector_size);
> +			(sector_t ) UINT_MAX * cxt->sector_size,
> +			cxt->sector_size);
>  	}
>  }
>  
> -void warn_alignment(void)
> +void warn_alignment(struct fdisk_context *cxt)
>  {
>  	if (nowarn)
>  		return;
>  
> -	if (sector_size != phy_sector_size)
> +	if (cxt->sector_size != cxt->phy_sector_size)
>  		fprintf(stderr, _("\n"
>  "The device presents a logical sector size that is smaller than\n"
>  "the physical sector size. Aligning to a physical sector (or optimal\n"
> @@ -451,64 +441,6 @@ void warn_alignment(void)
>  }
>  
>  static void
> -get_topology(struct fdisk_context *cxt) {
> -	int arg;
> -#ifdef HAVE_LIBBLKID
> -	blkid_probe pr;
> -
> -	pr = blkid_new_probe();
> -	if (pr && blkid_probe_set_device(pr, cxt->dev_fd, 0, 0) == 0) {
> -		blkid_topology tp = blkid_probe_get_topology(pr);
> -
> -		if (tp) {
> -			min_io_size = blkid_topology_get_minimum_io_size(tp);
> -			io_size = blkid_topology_get_optimal_io_size(tp);
> -			phy_sector_size = blkid_topology_get_physical_sector_size(tp);
> -			alignment_offset = blkid_topology_get_alignment_offset(tp);
> -
> -			/* We assume that the device provides topology info if
> -			 * optimal_io_size is set or alignment_offset is set or
> -			 * minimum_io_size is not power of 2.
> -			 *
> -			 * See also update_sector_offset().
> -			 */
> -			if (io_size || alignment_offset ||
> -			    (min_io_size & (min_io_size - 1)))
> -				has_topology = 1;
> -			if (!io_size)
> -				/* optimal IO is optional, default to minimum IO */
> -				io_size = min_io_size;
> -		}
> -	}
> -	blkid_free_probe(pr);
> -#endif
> -
> -	if (user_set_sector_size)
> -		/* fdisk since 2.17 differentiate between logical and physical
> -		 * sectors size. For backward compatibility the
> -		 *    fdisk -b <sectorsize>
> -		 * changes both, logical and physical sector size.
> -		 */
> -		phy_sector_size = sector_size;
> -
> -	else if (blkdev_get_sector_size(cxt->dev_fd, &arg) == 0) {
> -		sector_size = arg;
> -
> -		if (!phy_sector_size)
> -			phy_sector_size = sector_size;
> -	}
> -
> -	if (!min_io_size)
> -		min_io_size = phy_sector_size;
> -	if (!io_size)
> -		io_size = min_io_size;
> -
> -	if (sector_size != DEFAULT_SECTOR_SIZE)
> -		printf(_("Note: sector size is %d (not %d)\n"),
> -		       sector_size, DEFAULT_SECTOR_SIZE);
> -}
> -
> -static void
>  get_partition_table_geometry(void) {
>  	unsigned char *bufp = MBRbuffer;
>  	struct partition *p;
> @@ -544,9 +476,9 @@ get_partition_table_geometry(void) {
>   * Sets LBA of the first partition
>   */
>  void
> -update_sector_offset(void)
> +update_sector_offset(struct fdisk_context *cxt)
>  {
> -	grain = io_size;
> +	grain = cxt->io_size;
>  
>  	if (dos_compatible_flag)
>  		sector_offset = sectors;	/* usually 63 sectors */
> @@ -564,29 +496,29 @@ update_sector_offset(void)
>  		 */
>  		sector_t x = 0;
>  
> -		if (has_topology) {
> -			if (alignment_offset)
> -				x = alignment_offset;
> -			else if (io_size > 2048 * 512)
> -				x = io_size;
> +		if (fdisk_dev_has_topology(cxt)) {
> +			if (cxt->alignment_offset)
> +				x = cxt->alignment_offset;
> +			else if (cxt->io_size > 2048 * 512)
> +				x = cxt->io_size;
>  		}
>  		/* default to 1MiB */
>  		if (!x)
>  			x = 2048 * 512;
>  
> -		sector_offset = x / sector_size;
> +		sector_offset = x / cxt->sector_size;
>  
>  		/* don't use huge offset on small devices */
>  		if (total_number_of_sectors <= sector_offset * 4)
> -			sector_offset = phy_sector_size / sector_size;
> +			sector_offset = cxt->phy_sector_size / cxt->sector_size;
>  
>  		/* use 1MiB grain always when possible */
>  		if (grain < 2048 * 512)
>  			grain = 2048 * 512;
>  
>  		/* don't use huge grain on small devices */
> -		if (total_number_of_sectors <= (grain * 4 / sector_size))
> -			grain = phy_sector_size;
> +		if (total_number_of_sectors <= (grain * 4 / cxt->sector_size))
> +			grain = cxt->phy_sector_size;
>  	}
>  }
>  
> @@ -595,7 +527,6 @@ get_geometry(struct fdisk_context *cxt, struct geom *g) {
>  	sector_t llcyls, nsects = 0;
>  	unsigned int kern_heads = 0, kern_sectors = 0;
>  
> -	get_topology(cxt);
>  	heads = cylinders = sectors = 0;
>  	pt_heads = pt_sectors = 0;
>  
> @@ -611,9 +542,9 @@ get_geometry(struct fdisk_context *cxt, struct geom *g) {
>  
>  	/* get number of 512-byte sectors, and convert it the real sectors */
>  	if (blkdev_get_sectors(cxt->dev_fd, &nsects) == 0)
> -		total_number_of_sectors = (nsects / (sector_size >> 9));
> +		total_number_of_sectors = (nsects / (cxt->sector_size >> 9));
>  
> -	update_sector_offset();
> +	update_sector_offset(cxt);
>  
>  	llcyls = total_number_of_sectors / (heads * sectors);
>  	cylinders = llcyls;
> @@ -676,7 +607,7 @@ static int get_boot(struct fdisk_context *cxt, int try_only) {
>  
>  	if (check_osf_label(cxt)) {
>  		/* intialize partitions for BSD as well */
> -		dos_init();
> +		dos_init(cxt);
>  		if (!valid_part_table_flag(MBRbuffer)) {
>  			disklabel = OSF_LABEL;
>  			return 0;
> @@ -695,7 +626,7 @@ static int get_boot(struct fdisk_context *cxt, int try_only) {
>  #ifdef __sparc__
>  		create_sunlabel(cxt);
>  #else
> -		create_doslabel();
> +		create_doslabel(cxt);
>  #endif
>  	}
>  	return 0;
> @@ -796,7 +727,8 @@ read_hex(struct systypes *sys)
>  }
>  
>  unsigned int
> -read_int_with_suffix(unsigned int low, unsigned int dflt, unsigned int high,
> +read_int_with_suffix(struct fdisk_context *cxt,
> +	unsigned int low, unsigned int dflt, unsigned int high,

We only need 'sector_size' from the context structure in this
function. Wouldn't it be cleaner if you pass only sector_size to the
function instead of full context? I don't think function for 'reading'
the number should need anything else from the context.

>  	 unsigned int base, char *mesg, int *is_suffix_used)
>  {
>  	unsigned int res;
> @@ -895,7 +827,7 @@ read_int_with_suffix(unsigned int low, unsigned int dflt, unsigned int high,
>  				unsigned long unit;
>  
>  				bytes = (unsigned long long) res * absolute;
> -				unit = sector_size * units_per_sector;
> +				unit = cxt->sector_size * units_per_sector;
>  				bytes += unit/2;	/* round */
>  				bytes /= unit;
>  				res = bytes;
> @@ -932,18 +864,19 @@ read_int_with_suffix(unsigned int low, unsigned int dflt, unsigned int high,
>   * There is no default if DFLT is not between LOW and HIGH.
>   */
>  unsigned int
> -read_int(unsigned int low, unsigned int dflt, unsigned int high,
> +read_int(struct fdisk_context *cxt, 

Dtto.

> +	 unsigned int low, unsigned int dflt, unsigned int high,
>  	 unsigned int base, char *mesg)
>  {
> -	return read_int_with_suffix(low, dflt, high, base, mesg, NULL);
> +	return read_int_with_suffix(cxt, low, dflt, high, base, mesg, NULL);
>  }
>  
>  int
> -get_partition_dflt(int warn, int max, int dflt) {
> +get_partition_dflt(struct fdisk_context *cxt, int warn, int max, int dflt) {
>  	struct pte *pe;
>  	int i;
>  
> -	i = read_int(1, dflt, max, 0, _("Partition number")) - 1;
> +	i = read_int(cxt, 1, dflt, max, 0, _("Partition number")) - 1;
>  	pe = &ptes[i];
>  
>  	if (warn) {
> @@ -961,14 +894,14 @@ get_partition_dflt(int warn, int max, int dflt) {
>  }
>  
>  int
> -get_partition(int warn, int max) {
> -	return get_partition_dflt(warn, max, 0);
> +get_partition(struct fdisk_context *cxt, int warn, int max) {
> +	return get_partition_dflt(cxt, warn, max, 0);
>  }
>  
>  /* User partition selection unless one partition only is available */
>  
>  static int
> -get_existing_partition(int warn, int max) {
> +get_existing_partition(struct fdisk_context *cxt, int warn, int max) {
>  	int pno = -1;
>  	int i;
>  
> @@ -995,7 +928,7 @@ get_existing_partition(int warn, int max) {
>  
>  not_implemented:
>  not_unique:
> -	return get_partition(warn, max);
> +	return get_partition(cxt, warn, max);
>  }
>  
>  const char *
> @@ -1031,18 +964,18 @@ toggle_active(int i) {
>  }
>  
>  static void
> -toggle_dos_compatibility_flag(void) {
> +toggle_dos_compatibility_flag(struct fdisk_context *cxt) {
>  	dos_compatible_flag = ~dos_compatible_flag;
>  	if (dos_compatible_flag)
>  		printf(_("DOS Compatibility flag is set (DEPRECATED!)\n"));
>  	else
>  		printf(_("DOS Compatibility flag is not set\n"));
>  
> -	update_sector_offset();
> +	update_sector_offset(cxt);
>  }
>  
>  static void
> -delete_partition(int i)
> +delete_partition(struct fdisk_context *cxt, int i)
>  {
>  	if (i < 0)
>  		return;
> @@ -1057,18 +990,18 @@ delete_partition(int i)
>  	else if (disklabel == SUN_LABEL)
>  		sun_delete_partition(i);
>  	else if (disklabel == SGI_LABEL)
> -		sgi_delete_partition(i);
> +		sgi_delete_partition(cxt, i);
>  
>  	printf(_("Partition %d is deleted\n"), i + 1);
>  }
>  
> -static void
> -change_sysid(void) {
> +static void change_sysid(struct fdisk_context *cxt)
> +{
>  	char *temp;
>  	int i, sys, origsys;
>  	struct partition *p;
>  
> -	i = get_existing_partition(0, partitions);
> +	i = get_existing_partition(cxt, 0, partitions);
>  
>  	if (i == -1)
>  		return;
> @@ -1204,16 +1137,16 @@ static void check_consistency(struct partition *p, int partition) {
>  }
>  
>  static void
> -check_alignment(sector_t lba, int partition)
> +check_alignment(struct fdisk_context *cxt, sector_t lba, int partition)
>  {
> -	if (!lba_is_aligned(lba))
> +	if (!lba_is_aligned(cxt, lba))
>  		printf(_("Partition %i does not start on physical sector boundary.\n"),
>  			partition + 1);
>  }
>  
>  static void
>  list_disk_geometry(struct fdisk_context *cxt) {
> -	unsigned long long bytes = total_number_of_sectors * sector_size;
> +	unsigned long long bytes = total_number_of_sectors * cxt->sector_size;
>  	long megabytes = bytes/1000000;
>  
>  	if (megabytes < 10000)
> @@ -1229,16 +1162,16 @@ list_disk_geometry(struct fdisk_context *cxt) {
>  	if (units_per_sector == 1)
>  		printf(_(", total %llu sectors"), total_number_of_sectors);
>  	printf("\n");
> -	printf(_("Units = %s of %d * %d = %d bytes\n"),
> +	printf(_("Units = %s of %d * %ld = %ld bytes\n"),
>  	       str_units(PLURAL),
> -	       units_per_sector, sector_size, units_per_sector * sector_size);
> +	       units_per_sector, cxt->sector_size, units_per_sector * cxt->sector_size);
>  
> -	printf(_("Sector size (logical/physical): %u bytes / %lu bytes\n"),
> -				sector_size, phy_sector_size);
> +	printf(_("Sector size (logical/physical): %lu bytes / %lu bytes\n"),
> +				cxt->sector_size, cxt->phy_sector_size);
>  	printf(_("I/O size (minimum/optimal): %lu bytes / %lu bytes\n"),
> -				min_io_size, io_size);
> -	if (alignment_offset)
> -		printf(_("Alignment offset: %lu bytes\n"), alignment_offset);
> +				cxt->min_io_size, cxt->io_size);
> +	if (cxt->alignment_offset)
> +		printf(_("Alignment offset: %lu bytes\n"), cxt->alignment_offset);
>  	if (disklabel == DOS_LABEL)
>  		dos_print_mbr_id();
>  	printf("\n");
> @@ -1428,12 +1361,12 @@ list_table(struct fdisk_context *cxt, int xtra) {
>  			unsigned int pblocks = psects;
>  			unsigned int podd = 0;
>  
> -			if (sector_size < 1024) {
> -				pblocks /= (1024 / sector_size);
> -				podd = psects % (1024 / sector_size);
> +			if (cxt->sector_size < 1024) {
> +				pblocks /= (1024 / cxt->sector_size);
> +				podd = psects % (1024 / cxt->sector_size);
>  			}
> -			if (sector_size > 1024)
> -				pblocks *= (sector_size / 1024);
> +			if (cxt->sector_size > 1024)
> +				pblocks *= (cxt->sector_size / 1024);
>                          printf(
>  			    "%s  %c %11lu %11lu %11lu%c  %2x  %s\n",
>  			partname(cxt->dev_path, i+1, w+2),
> @@ -1447,7 +1380,7 @@ list_table(struct fdisk_context *cxt, int xtra) {
>  /* type name */		(type = partition_type(p->sys_ind)) ?
>  			type : _("Unknown"));
>  			check_consistency(p, i);
> -			check_alignment(get_partition_start(pe), i);
> +			check_alignment(cxt, get_partition_start(pe), i);
>  		}
>  	}
>  
> @@ -1482,7 +1415,7 @@ x_list_table(struct fdisk_context *cxt, int extend) {
>  				(unsigned long) get_nr_sects(p), p->sys_ind);
>  			if (p->sys_ind) {
>  				check_consistency(p, i);
> -				check_alignment(get_partition_start(pe), i);
> +				check_alignment(cxt, get_partition_start(pe), i);
>  			}
>  		}
>  	}
> @@ -1533,7 +1466,7 @@ check(int n, unsigned int h, unsigned int s, unsigned int c,
>  }
>  
>  static void
> -verify(void) {
> +verify(struct fdisk_context *cxt) {
>  	int i, j;
>  	sector_t total = 1, n_sectors = total_number_of_sectors;
>  	unsigned long long first[partitions], last[partitions];
> @@ -1559,7 +1492,7 @@ verify(void) {
>  		p = pe->part_table;
>  		if (p->sys_ind && !IS_EXTENDED (p->sys_ind)) {
>  			check_consistency(p, i);
> -			check_alignment(get_partition_start(pe), i);
> +			check_alignment(cxt, get_partition_start(pe), i);
>  			if (get_partition_start(pe) < first[i])
>  				printf(_("Warning: bad start-of-data in "
>  					"partition %d\n"), i + 1);
> @@ -1603,30 +1536,31 @@ verify(void) {
>  		printf(_("Total allocated sectors %llu greater than the maximum"
>  			" %llu\n"), total, n_sectors);
>  	else if (total < n_sectors)
> -		printf(_("Remaining %lld unallocated %d-byte sectors\n"),
> -		       n_sectors - total, sector_size);
> +		printf(_("Remaining %lld unallocated %ld-byte sectors\n"),
> +		       n_sectors - total, cxt->sector_size);
>  }
>  
> -void print_partition_size(int num, sector_t start, sector_t stop, int sysid)
> +void print_partition_size(struct fdisk_context *cxt,
> +			  int num, sector_t start, sector_t stop, int sysid)
>  {
>  	char *str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE,
> -				     (stop - start + 1) * sector_size);
> +				     (stop - start + 1) * cxt->sector_size);
>  	printf(_("Partition %d of type %s and of size %s is set\n"), num, partition_type(sysid), str);
>  	free(str);
>  }
>  
> -static void new_partition(void)
> +static void new_partition(struct fdisk_context *cxt)
>  {
>  	if (warn_geometry())
>  		return;
>  
>  	if (disklabel == SUN_LABEL) {
> -		add_sun_partition(get_partition(0, partitions), LINUX_NATIVE);
> +		add_sun_partition(cxt, get_partition(cxt, 0, partitions), LINUX_NATIVE);
>  		return;
>  	}
>  
>  	if (disklabel == SGI_LABEL) {
> -		sgi_add_partition(get_partition(0, partitions), LINUX_NATIVE);
> +		sgi_add_partition(cxt, get_partition(cxt, 0, partitions), LINUX_NATIVE);
>  		return;
>  	}
>  
> @@ -1649,7 +1583,7 @@ static void new_partition(void)
>  	}
>  
>  	/* default to DOS/BSD */
> -	dos_new_partition();
> +	dos_new_partition(cxt);
>  }
>  
>  static void
> @@ -1719,10 +1653,10 @@ reread_partition_table(struct fdisk_context *cxt, int leave) {
>  
>  #define MAX_PER_LINE	16
>  static void
> -print_buffer(unsigned char pbuffer[]) {
> +print_buffer(struct fdisk_context *cxt, unsigned char pbuffer[]) {

Take something like 'unsigned long buflen' instead of 'full' context?

>  	unsigned int i, l;
>  
> -	for (i = 0, l = 0; i < sector_size; i++, l++) {
> +	for (i = 0, l = 0; i < cxt->sector_size; i++, l++) {
>  		if (l == 0)
>  			printf("0x%03X:", i);
>  		printf(" %02X", pbuffer[i]);
> @@ -1736,19 +1670,19 @@ print_buffer(unsigned char pbuffer[]) {
>  	printf("\n");
>  }
>  
> -static void
> -print_raw(char *dev) {
> +static void print_raw(struct fdisk_context *cxt)
> +{
>  	int i;
>  
> -	printf(_("Device: %s\n"), dev);
> +	printf(_("Device: %s\n"), cxt->dev_path);
>  	if (disklabel == SUN_LABEL || disklabel == SGI_LABEL)
> -		print_buffer(MBRbuffer);
> +		print_buffer(cxt, MBRbuffer);
>  	else for (i = 3; i < partitions; i++)
> -		print_buffer(ptes[i].sectorbuffer);
> +		     print_buffer(cxt, ptes[i].sectorbuffer);
>  }
>  
>  static void
> -move_begin(int i) {
> +move_begin(struct fdisk_context *cxt, int i) {
>  	struct pte *pe = &ptes[i];
>  	struct partition *p = pe->part_table;
>  	unsigned int new, free_start, curr_start, last;
> @@ -1785,7 +1719,7 @@ move_begin(int i) {
>  
>  	last = get_partition_start(pe) + get_nr_sects(p) - 1;
>  
> -	new = read_int(free_start, curr_start, last, free_start,
> +	new = read_int(cxt, free_start, curr_start, last, free_start,
>  		       _("New beginning of data")) - pe->offset;
>  
>  	if (new != get_nr_sects(p)) {
> @@ -1814,27 +1748,27 @@ expert_command_prompt(struct fdisk_context *cxt)
>  		switch (c) {
>  		case 'a':
>  			if (disklabel == SUN_LABEL)
> -				sun_set_alt_cyl();
> +				sun_set_alt_cyl(cxt);
>  			break;
>  		case 'b':
>  			if (disklabel == DOS_LABEL)
> -				move_begin(get_partition(0, partitions));
> +				move_begin(cxt, get_partition(cxt, 0, partitions));
>  			break;
>  		case 'c':
>  			user_cylinders = cylinders =
> -				read_int(1, cylinders, 1048576, 0,
> +				read_int(cxt, 1, cylinders, 1048576, 0,
>  					 _("Number of cylinders"));
>  			if (disklabel == SUN_LABEL)
>  				sun_set_ncyl(cylinders);
>  			break;
>  		case 'd':
> -			print_raw(cxt->dev_path);
> +			print_raw(cxt);
>  			break;
>  		case 'e':
>  			if (disklabel == SGI_LABEL)
>  				sgi_set_xcyl();
>  			else if (disklabel == SUN_LABEL)
> -				sun_set_xcyl();
> +				sun_set_xcyl(cxt);
>  			else
>  			if (disklabel == DOS_LABEL)
>  				x_list_table(cxt, 1);
> @@ -1847,19 +1781,19 @@ expert_command_prompt(struct fdisk_context *cxt)
>  			create_sgilabel(cxt);
>  			break;
>  		case 'h':
> -			user_heads = heads = read_int(1, heads, 256, 0,
> +			user_heads = heads = read_int(cxt, 1, heads, 256, 0,
>  					 _("Number of heads"));
>  			update_units();
>  			break;
>  		case 'i':
>  			if (disklabel == SUN_LABEL)
> -				sun_set_ilfact();
> +				sun_set_ilfact(cxt);
>  			else if (disklabel == DOS_LABEL)
>  				dos_set_mbr_id();
>  			break;
>  		case 'o':
>  			if (disklabel == SUN_LABEL)
> -				sun_set_rspeed();
> +				sun_set_rspeed(cxt);
>  			break;
>  		case 'p':
>  			if (disklabel == SUN_LABEL)
> @@ -1872,24 +1806,24 @@ expert_command_prompt(struct fdisk_context *cxt)
>  		case 'r':
>  			return;
>  		case 's':
> -			user_sectors = sectors = read_int(1, sectors, 63, 0,
> +			user_sectors = sectors = read_int(cxt, 1, sectors, 63, 0,
>  					   _("Number of sectors"));
>  			if (dos_compatible_flag)
>  				fprintf(stderr, _("Warning: setting "
>  					"sector offset for DOS "
>  					"compatiblity\n"));
> -			update_sector_offset();
> +			update_sector_offset(cxt);
>  			update_units();
>  			break;
>  		case 'v':
> -			verify();
> +			verify(cxt);
>  			break;
>  		case 'w':
>  			write_table(cxt);
>  			break;
>  		case 'y':
>  			if (disklabel == SUN_LABEL)
> -				sun_set_pcylcount();
> +				sun_set_pcylcount(cxt);
>  			break;
>  		default:
>  			print_menu(EXPERT_MENU);
> @@ -1918,15 +1852,15 @@ gpt_warning(char *dev)
>  }
>  
>  /* Print disk geometry and partition table of a specified device (-l option) */
> -
> -static void
> -print_partition_table_from_option(char *device)
> +static void print_partition_table_from_option(char *device, unsigned long sector_size)
>  {
>  	int gb;
>  
>  	struct fdisk_context *cxt = fdisk_new_context_from_filename(device, 1);	/* read-only */
>  	if (!cxt)
>  		err(EXIT_FAILURE, _("unable to open %s"), device);
> +	if (sector_size)  /* passed -b option, override autodiscovery */
> +		cxt->phy_sector_size = cxt->sector_size = sector_size;

Perhaps this user-provided sector_size together with a bit-flag
denoting that the user wants to override the sector_size should be
made part of the context? But that'd be material for separate patch,
IMHO.

[SNIP]
 
>  int fdisk_debug_mask;
>  
> +static unsigned long __get_sector_size(int fd)
> +{
> +	unsigned int sect_sz;

This introduces type mismatch - should be just int.

> +
> +	if (!blkdev_get_sector_size(fd, &sect_sz))
> +		return (unsigned long) sect_sz;
> +	return DEFAULT_SECTOR_SIZE;
> +}


Petr

-- 
Petr Uzel
IRC: ptr_uzl @ freenode

Attachment: pgpGfRyKvsZ68.pgp
Description: PGP signature


[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux