Re: [UI RFC][PATCH] btrfs-progs: add options to tune units for fi df output

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

 



On Mon, Sep 15, 2014 at 05:09:52PM +0200, David Sterba wrote:
> The size unit format is a longstanding annoyance. This patch is based on
> the work of Nils and Alexandre and enhances the options. It's possible
> to select raw bytes, SI-based or IEC-based compact units (human
> frientdly) or a fixed base from kilobytes to terabytes. The default is
> compact human readable IEC-based, no change to current version.
> 
> CC: Nils Steinger <nst@xxxxxxxxxx>
> CC: Alexandre Oliva <oliva@xxxxxxx>
> Signed-off-by: David Sterba <dsterba@xxxxxxx>

   Looks good to me. One _tiny_ nit: For the kilo-/kibi- prefix, IEC
is KiB (upper case), SI is kB (lower case). Other than that, the UI
looks pretty comfortable to me.

Reviewed-by: Hugo Mills <hugo@xxxxxxxxxxxxx>

> ---
> 
> I tried to make the command line UI rich enough to address current and future
> needs, I'm open to tweaks, rewording etc.
> 
> The patch is based on current snapshot of integration branch that will be the
> base of 3.17 release and contains the 'enhanced df' patches, branch dev/units.
> 
>  Documentation/btrfs-filesystem.txt |  25 ++++++++-
>  cmds-filesystem.c                  | 111 ++++++++++++++++++++++++++++---------
>  utils.c                            |  48 ++++++++++++----
>  utils.h                            |  30 +++++++---
>  4 files changed, 168 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/btrfs-filesystem.txt b/Documentation/btrfs-filesystem.txt
> index c9c0b006a0b0..7ac105ff350e 100644
> --- a/Documentation/btrfs-filesystem.txt
> +++ b/Documentation/btrfs-filesystem.txt
> @@ -17,8 +17,31 @@ resizing, defragment.
>  
>  SUBCOMMAND
>  ----------
> -*df* <path> [<path>...]::
> +*df* [options] <path>::
>  Show space usage information for a mount point.
> ++
> +`Options`
> ++
> +-b|--raw::::
> +raw numbers in bytes, without the 'B' suffix
> +-h::::
> +print human friendly numbers, base 1024, this is the default
> +-H::::
> +print human friendly numbers, base 1000
> +--iec::::
> +select the 1024 base for the following options, according to the IEC standard
> +--si::::
> +select the 1000 base for the following options, according to the SI standard
> +-k|--kbytes::::
> +show sizes in KiB, or KB with --si
> +-m|--mbytes::::
> +show sizes in MiB, or MB with --si
> +-g|--gbytes::::
> +show sizes in GiB, or GB with --si
> +-t|--tbytes::::
> +show sizes in TiB, or TB with --si
> +
> +If conflicting options are passed, the last one takes precedence.
>  
>  *show* [--mounted|--all-devices|<path>|<uuid>|<device>|<label>]::
>  Show the btrfs filesystem with some additional info.
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 89b897496256..68876957cbab 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -114,12 +114,21 @@ static const char * const filesystem_cmd_group_usage[] = {
>  };
>  
>  static const char * const cmd_filesystem_df_usage[] = {
> -       "btrfs filesystem df <path>",
> +       "btrfs filesystem df [options] <path>",
>         "Show space usage information for a mount point",
> +	"-b|--raw           raw numbers in bytes",
> +	"-h                 human friendly numbers, base 1024 (default)",
> +	"-H                 human friendly numbers, base 1000",
> +	"--iec              use 1024 as a base (Kib, MiB, GiB, ...)",
> +	"--si               use 1000 as a base (KB, MB, GB, ...)",
> +	"-k|--kbytes        show sizes in KiB, or KB with --si",
> +	"-m|--mbytes        show sizes in MiB, or MB with --si",
> +	"-g|--gbytes        show sizes in GiB, or GB with --si",
> +	"-t|--tbytes        show sizes in TiB, or TB with --si",
>         NULL
>  };
>  
> -static void print_df(struct btrfs_ioctl_space_args *sargs)
> +static void print_df(struct btrfs_ioctl_space_args *sargs, int unit_mode)
>  {
>         u64 i;
>         struct btrfs_ioctl_space_info *sp = sargs->spaces;
> @@ -128,8 +137,8 @@ static void print_df(struct btrfs_ioctl_space_args *sargs)
>                 printf("%s, %s: total=%s, used=%s\n",
>                         group_type_str(sp->flags),
>                         group_profile_str(sp->flags),
> -                       pretty_size(sp->total_bytes),
> -                       pretty_size(sp->used_bytes));
> +                       pretty_size_mode(sp->total_bytes, unit_mode),
> +                       pretty_size_mode(sp->used_bytes, unit_mode));
>         }
>  }
>  
> @@ -183,33 +192,83 @@ static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
>  
>  static int cmd_filesystem_df(int argc, char **argv)
>  {
> -       struct btrfs_ioctl_space_args *sargs = NULL;
> -       int ret;
> -       int fd;
> -       char *path;
> -       DIR *dirstream = NULL;
> +	struct btrfs_ioctl_space_args *sargs = NULL;
> +	int ret;
> +	int fd;
> +	char *path;
> +	DIR *dirstream = NULL;
> +	unsigned unit_mode = UNITS_DEFAULT;
>  
> -       if (check_argc_exact(argc, 2))
> -               usage(cmd_filesystem_df_usage);
> +	optind = 1;
> +	while (1) {
> +		int long_index;
> +		static const struct option long_options[] = {
> +			{ "raw", no_argument, NULL, 'b'},
> +			{ "kbytes", no_argument, NULL, 'k'},
> +			{ "mbytes", no_argument, NULL, 'm'},
> +			{ "gbytes", no_argument, NULL, 'g'},
> +			{ "tbytes", no_argument, NULL, 't'},
> +			{ "si", no_argument, NULL, 256},
> +			{ "iec", no_argument, NULL, 257},
> +		};
> +		int c = getopt_long(argc, argv, "bhHkmgt", long_options,
> +					&long_index);
> +		if (c < 0)
> +			break;
> +		switch (c) {
> +		case 'b':
> +			unit_mode = UNITS_RAW;
> +			break;
> +		case 'k':
> +			units_set_base(&unit_mode, UNITS_KBYTES);
> +			break;
> +		case 'm':
> +			units_set_base(&unit_mode, UNITS_MBYTES);
> +			break;
> +		case 'g':
> +			units_set_base(&unit_mode, UNITS_GBYTES);
> +			break;
> +		case 't':
> +			units_set_base(&unit_mode, UNITS_TBYTES);
> +			break;
> +		case 'h':
> +			unit_mode = UNITS_HUMAN_BINARY;
> +			break;
> +		case 'H':
> +			unit_mode = UNITS_HUMAN_DECIMAL;
> +			break;
> +		case 256:
> +			units_set_mode(&unit_mode, UNITS_DECIMAL);
> +			break;
> +		case 257:
> +			units_set_mode(&unit_mode, UNITS_BINARY);
> +			break;
> +		default:
> +			usage(cmd_filesystem_df_usage);
> +		}
> +	}
>  
> -       path = argv[1];
> +	if (check_argc_max(argc, optind + 1))
> +		usage(cmd_filesystem_df_usage);
>  
> -       fd = open_file_or_dir(path, &dirstream);
> -       if (fd < 0) {
> -               fprintf(stderr, "ERROR: can't access '%s'\n", path);
> -               return 1;
> -       }
> -       ret = get_df(fd, &sargs);
> +	path = argv[optind];
>  
> -       if (!ret && sargs) {
> -               print_df(sargs);
> -               free(sargs);
> -       } else {
> -               fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
> -       }
> +	fd = open_file_or_dir(path, &dirstream);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: can't access '%s'\n", path);
> +		return 1;
> +	}
> +	ret = get_df(fd, &sargs);
>  
> -       close_file_or_dir(fd, dirstream);
> -       return !!ret;
> +	if (ret == 0) {
> +		print_df(sargs, unit_mode);
> +		free(sargs);
> +	} else {
> +		fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
> +	}
> +
> +	close_file_or_dir(fd, dirstream);
> +	return !!ret;
>  }
>  
>  static int match_search_item_kernel(__u8 *fsid, char *mnt, char *label,
> diff --git a/utils.c b/utils.c
> index 9ab1480ede43..5db05010e839 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1383,33 +1383,36 @@ static const char* unit_suffix_binary[] =
>  static const char* unit_suffix_decimal[] =
>  	{ "B", "KB", "MB", "GB", "TB", "PB", "EB"};
>  
> -int pretty_size_snprintf(u64 size, char *str, size_t str_size, int unit_mode)
> +int pretty_size_snprintf(u64 size, char *str, size_t str_size, unsigned unit_mode)
>  {
>  	int num_divs;
>  	float fraction;
> -	int base = 0;
> +	u64 base = 0;
> +	int mult = 0;
>  	const char** suffix = NULL;
>  	u64 last_size;
>  
>  	if (str_size == 0)
>  		return 0;
>  
> -	if (unit_mode == UNITS_RAW) {
> +	if ((unit_mode & ~UNITS_MODE_MASK) == UNITS_RAW) {
>  		snprintf(str, str_size, "%llu", size);
>  		return 0;
>  	}
>  
> -	if (unit_mode == UNITS_BINARY) {
> +	if ((unit_mode & ~UNITS_MODE_MASK) == UNITS_BINARY) {
>  		base = 1024;
> +		mult = 1024;
>  		suffix = unit_suffix_binary;
> -	} else if (unit_mode == UNITS_DECIMAL) {
> +	} else if ((unit_mode & ~UNITS_MODE_MASK) == UNITS_DECIMAL) {
>  		base = 1000;
> +		mult = 1000;
>  		suffix = unit_suffix_decimal;
>  	}
>  
>  	/* Unknown mode */
>  	if (!base) {
> -		fprintf(stderr, "INTERNAL ERROR: unknown unit base, mode %d",
> +		fprintf(stderr, "INTERNAL ERROR: unknown unit base, mode %d\n",
>  				unit_mode);
>  		assert(0);
>  		return -1;
> @@ -1417,11 +1420,22 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_size, int unit_mode)
>  
>  	num_divs = 0;
>  	last_size = size;
> -
> -	while (size >= base) {
> -		last_size = size;
> -		size /= base;
> -		num_divs++;
> +	switch (unit_mode & UNITS_MODE_MASK) {
> +	case UNITS_TBYTES: base *= mult; num_divs++;
> +	case UNITS_GBYTES: base *= mult; num_divs++;
> +	case UNITS_MBYTES: base *= mult; num_divs++;
> +	case UNITS_KBYTES: num_divs++;
> +			   break;
> +	case UNITS_BYTES:
> +			   base = 1;
> +			   num_divs = 0;
> +			   break;
> +	default:
> +		while (size >= mult) {
> +			last_size = size;
> +			size /= mult;
> +			num_divs++;
> +		}
>  	}
>  
>  	if (num_divs >= ARRAY_SIZE(unit_suffix_binary)) {
> @@ -2573,3 +2587,15 @@ const char *group_profile_str(u64 flag)
>  	}
>  }
>  
> +void units_set_mode(unsigned *units, unsigned mode)
> +{
> +	unsigned base = *units & UNITS_MODE_MASK;
> +
> +	*units = base | mode;
> +}
> +void units_set_base(unsigned *units, unsigned base)
> +{
> +	unsigned mode = *units & ~UNITS_MODE_MASK;
> +
> +	*units = base | mode;
> +}
> diff --git a/utils.h b/utils.h
> index 1eed0c1606aa..8156f13126af 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -50,12 +50,26 @@ void fixup_argv0(char **argv, const char *token);
>  void set_argv0(char **argv);
>  
>  /*
> - * Output mode of byte units
> + * Output modes of size
>   */
> -#define UNITS_RAW			(1)
> -#define UNITS_BINARY			(2)
> -#define UNITS_DECIMAL			(3)
> -#define UNITS_HUMAN			UNITS_BINARY
> +#define UNITS_RESERVED			(0)
> +#define UNITS_BYTES			(1)
> +#define UNITS_KBYTES			(2)
> +#define UNITS_MBYTES			(3)
> +#define UNITS_GBYTES			(4)
> +#define UNITS_TBYTES			(5)
> +#define UNITS_RAW			(1U << UNITS_MODE_SHIFT)
> +#define UNITS_BINARY			(2U << UNITS_MODE_SHIFT)
> +#define UNITS_DECIMAL			(3U << UNITS_MODE_SHIFT)
> +#define UNITS_MODE_MASK			((1U << UNITS_MODE_SHIFT) - 1)
> +#define UNITS_MODE_SHIFT		(8)
> +#define UNITS_HUMAN_BINARY		(UNITS_BINARY)
> +#define UNITS_HUMAN_DECIMAL		(UNITS_DECIMAL)
> +#define UNITS_HUMAN			(UNITS_HUMAN_BINARY)
> +#define UNITS_DEFAULT			(UNITS_HUMAN)
> +
> +void units_set_mode(unsigned *units, unsigned mode);
> +void units_set_base(unsigned *units, unsigned base);
>  
>  int make_btrfs(int fd, const char *device, const char *label,
>  	       char *fs_uuid, u64 blocks[6], u64 num_bytes, u32 nodesize,
> @@ -79,12 +93,12 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
>  int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
>  				 int super_offset);
>  
> -int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, int unit_mode);
> -#define pretty_size(size) 	pretty_size_mode(size, UNITS_BINARY)
> +int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, unsigned unit_mode);
> +#define pretty_size(size) 	pretty_size_mode(size, UNITS_DEFAULT)
>  #define pretty_size_mode(size, mode) 					      \
>  	({								      \
>  		static __thread char _str[32];				      \
> -		(void)pretty_size_snprintf((size), _str, sizeof(_str), mode); \
> +		(void)pretty_size_snprintf((size), _str, sizeof(_str), (mode)); \
>  		_str;							      \
>  	})
>  

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
           --- There are three mistaikes in this sentance. ---           

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux