Skip to contentSkip to conversation

Enhanced df - followup

PATCH In review David Sterba 9 participants 35 messages Apr 29, 2014 – May 14, 2014 Btrfs file system

lore
Share & tools
lore

David Sterba

author
#0 lore raw

cover Enhanced df - followup

Hi,

this is a followup to the df (space usage) enhancements, Goffredo's patches are merged without changes, though the code needs updates.

But, given the patchset size, net benefit of the enriched output and longstanding need of the update, I've deliberately skipped the process of incremental rewiew/resend loop that should normally take place and implemented the changes that were agreed in previous discussions and did code cleanups along the way.

The basic layout of the command output has been kept, there are some tweaks or additions.

Changes:
* btrfs filesystem disk_usage - renamed to usage
* added a section of overall filesystem usage, that used to be in the 'fi df' output
* btrfs device disk_usage - renamed to usage
* the device size prints both blockdevie size and the size that's occupied by the filesystem
* device ID is printed
* btrfs filesystem df - reverted to the original output
* btrfs filesystem df - now prints the GlobalReserve info
* units print B for bytes
* the ioctls FS_INFO and DEV_INFO need root (patches sent to drop that requirement), so there's a warning printed instead
* no documentation for now

It's not finished, I've tried to scan all the previous iteration of the patches for the suggested UI tweaks and probably missed some, but IMO in a state that can go to git and be scheduled for a near release.

The major UI changes are there, we can tweak it further of course.

David Sterba (14):
btrfs-progs: read global reserve size from space infos
btrfs-progs: add original 'df' and rename 'disk_usage' to 'usage'
btrfs-progs: move device usage to cmds-device, more cleanups
btrfs-progs: check if we can't get info from ioctls due to permissions
btrfs-progs: zero out structures before calling ioctl
btrfs-progs: print B for bytes
btrfs-progs: Print more info about device sizes
btrfs-progs: compare unallocated space against the correct value
btrfs-progs: add section of overall filesystem usage
btrfs-progs: cleanup filesystem/device usage code
btrfs-progs: extend pretty printers with unit mode
btrfs-progs: replace df_pretty_sizes with pretty_size_mode
btrfs-progs: clean up return codes and paths
btrfs-progs: move global reserve to overall summary

 cmds-device.c        |  92 +++++++-
 cmds-fi-disk_usage.c | 631 +++++++++++++++++++--------------------------------
 cmds-fi-disk_usage.h |  35 ++-
 cmds-filesystem.c    |  55 ++++-
 ctree.h              |   6 +
 utils.c              |  78 +++++--
 utils.h              |  21 +-
 7 files changed, 482 insertions(+), 436 deletions(-)
Signature · 1 line
1.9.0
author
#1 lore raw

PATCH 1/14 btrfs-progs: read global reserve size from space infos

Kernels >= 3.15 export the global block reserve as a space info presented by 'btrfs fi df' but would display 'unknown' instead of some meaningful string.

Signed-off-by David Sterba <[email protected]>
---
 ctree.h | 6 ++++++
 utils.c | 7 ++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)
ctree.h +6 -0
index 8ac17619b9dc..5c43fc5f5f6e 100644
@@ -862,6 +862,12 @@ struct btrfs_csum_item {
 /* used in struct btrfs_balance_args fields */
 #define BTRFS_AVAIL_ALLOC_BIT_SINGLE	(1ULL << 48)
 
+/*
+ * GLOBAL_RSV does not exist as a on-disk block group type and is used
+ * internally for exporting info about global block reserve from space infos
+ */
+#define BTRFS_SPACE_INFO_GLOBAL_RSV    (1ULL << 49)
+
 #define BTRFS_QGROUP_STATUS_OFF			0
 #define BTRFS_QGROUP_STATUS_ON			1
 #define BTRFS_QGROUP_STATUS_SCANNING		2
utils.c +6 -1
index f2ab416c28b2..ca150404ea6f 100644
@@ -2240,7 +2240,10 @@ u64 get_partition_size(char *dev)
  */
 const char *group_type_str(u64 flag)
 {
-	switch (flag & BTRFS_BLOCK_GROUP_TYPE_MASK) {
+	u64 mask = BTRFS_BLOCK_GROUP_TYPE_MASK |
+		BTRFS_SPACE_INFO_GLOBAL_RSV;
+
+	switch (flag & mask) {
 	case BTRFS_BLOCK_GROUP_DATA:
 		return "Data";
 	case BTRFS_BLOCK_GROUP_SYSTEM:

@@ -2249,6 +2252,8 @@ const char *group_type_str(u64 flag)
 		return "Metadata";
 	case BTRFS_BLOCK_GROUP_DATA|BTRFS_BLOCK_GROUP_METADATA:
 		return "Data+Metadata";
+	case BTRFS_SPACE_INFO_GLOBAL_RSV:
+		return "GlobalReserve";
 	default:
 		return "unknown";
 	}
Signature · 1 line
1.9.0
author
#2 lore raw

PATCH 2/14 btrfs-progs: add original 'df' and rename 'disk_usage' to 'usage'

In reply to David Sterba

Add back the original output of the 'btrfs fi df' command for backward compatibility. The rich output is moved from 'disk_usage' to 'usage'.

Agreed in http://www.spinics.net/lists/linux-btrfs/msg31698.html

Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c | 85 ++++++++++------------------------------------------
 cmds-fi-disk_usage.h |  7 ++---
 cmds-filesystem.c    | 55 ++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 76 deletions(-)
cmds-fi-disk_usage.c +16 -69
index 2bd591db0f8a..400863d9764f 100644
@@ -328,6 +328,8 @@ static struct btrfs_ioctl_space_args *load_space_info(int fd, char *path)
 	return sargs;
 }
 
+/* Not used, keep for later */
+#if 0
 /*
  * This function computes the space occuped by a *single* RAID5/RAID6 chunk.
  * The computation is performed on the basis of the number of stripes

@@ -465,62 +467,7 @@ exit:
 
 	return ret;
 }
-
-const char * const cmd_filesystem_df_usage[] = {
-	"btrfs filesystem df [-b] <path> [<path>..]",
-	"Show space usage information for a mount point(s).",
-	"",
-	"-b\tSet byte as unit",
-	NULL
-};
-
-int cmd_filesystem_df(int argc, char **argv)
-{
-
-	int	flags = DF_HUMAN_UNIT;
-	int	i, more_than_one = 0;
-
-	optind = 1;
-	while (1) {
-		char	c = getopt(argc, argv, "b");
-		if (c < 0)
-			break;
-
-		switch (c) {
-		case 'b':
-			flags &= ~DF_HUMAN_UNIT;
-			break;
-		default:
-			usage(cmd_filesystem_df_usage);
-		}
-	}
-
-	if (check_argc_min(argc - optind, 1))
-		usage(cmd_filesystem_df_usage);
-
-	for (i = optind; i < argc ; i++) {
-		int r, fd;
-		DIR *dirstream = NULL;
-		if (more_than_one)
-			printf("\n");
-
-		fd = open_file_or_dir(argv[i], &dirstream);
-		if (fd < 0) {
-			fprintf(stderr, "ERROR: can't access to '%s'\n",
-				argv[1]);
-			return 12;
-		}
-		r = _cmd_disk_free(fd, argv[i], flags);
-		close_file_or_dir(fd, dirstream);
-
-		if (r)
-			return r;
-		more_than_one = 1;
-
-	}
-
-	return 0;
-}
+#endif
 
 /*
  *  Helper to sort the disk_info structure

@@ -612,10 +559,10 @@ static u64 calc_chunk_size(struct chunk_info *ci)
 }
 
 /*
- *  This function print the results of the command btrfs fi disk-usage
+ *  This function print the results of the command "btrfs fi usage"
  *  in tabular format
  */
-static void _cmd_filesystem_disk_usage_tabular(int mode,
+static void _cmd_filesystem_usage_tabular(int mode,
 					struct btrfs_ioctl_space_args *sargs,
 					struct chunk_info *chunks_info_ptr,
 					int chunks_info_count,

@@ -795,10 +742,10 @@ static void print_chunk_disks(u64 chunk_type,
 }
 
 /*
- *  This function print the results of the command btrfs fi disk-usage
+ *  This function print the results of the command "btrfs fi usage"
  *  in linear format
  */
-static void _cmd_filesystem_disk_usage_linear(int mode,
+static void _cmd_filesystem_usage_linear(int mode,
 					struct btrfs_ioctl_space_args *sargs,
 					struct chunk_info *info_ptr,
 					int info_count,

@@ -839,7 +786,7 @@ static void _cmd_filesystem_disk_usage_linear(int mode,
 
 }
 
-static int _cmd_filesystem_disk_usage(int fd, char *path, int mode, int tabular)
+static int _cmd_filesystem_usage(int fd, char *path, int mode, int tabular)
 {
 	struct btrfs_ioctl_space_args *sargs = 0;
 	int info_count = 0;

@@ -860,11 +807,11 @@ static int _cmd_filesystem_disk_usage(int fd, char *path, int mode, int tabular)
 	}
 
 	if (tabular)
-		_cmd_filesystem_disk_usage_tabular(mode, sargs,
+		_cmd_filesystem_usage_tabular(mode, sargs,
 					info_ptr, info_count,
 					disks_info_ptr, disks_info_count);
 	else
-		_cmd_filesystem_disk_usage_linear(mode, sargs,
+		_cmd_filesystem_usage_linear(mode, sargs,
 					info_ptr, info_count,
 					disks_info_ptr, disks_info_count);
 

@@ -880,8 +827,8 @@ exit:
 	return ret;
 }
 
-const char * const cmd_filesystem_disk_usage_usage[] = {
-	"btrfs filesystem disk-usage [-b][-t] <path> [<path>..]",
+const char * const cmd_filesystem_usage_usage[] = {
+	"btrfs filesystem usage [-b][-t] <path> [<path>..]",
 	"Show in which disk the chunks are allocated.",
 	"",
 	"-b\tSet byte as unit",

@@ -889,7 +836,7 @@ const char * const cmd_filesystem_disk_usage_usage[] = {
 	NULL
 };
 
-int cmd_filesystem_disk_usage(int argc, char **argv)
+int cmd_filesystem_usage(int argc, char **argv)
 {
 
 	int	flags =	DF_HUMAN_UNIT;

@@ -909,12 +856,12 @@ int cmd_filesystem_disk_usage(int argc, char **argv)
 			tabular = 1;
 			break;
 		default:
-			usage(cmd_filesystem_disk_usage_usage);
+			usage(cmd_filesystem_usage_usage);
 		}
 	}
 
 	if (check_argc_min(argc - optind, 1))
-		usage(cmd_filesystem_disk_usage_usage);
+		usage(cmd_filesystem_usage_usage);
 
 	for (i = optind; i < argc ; i++) {
 		int r, fd;

@@ -928,7 +875,7 @@ int cmd_filesystem_disk_usage(int argc, char **argv)
 				argv[1]);
 			return 12;
 		}
-		r = _cmd_filesystem_disk_usage(fd, argv[i], flags, tabular);
+		r = _cmd_filesystem_usage(fd, argv[i], flags, tabular);
 		close_file_or_dir(fd, dirstream);
 
 		if (r)
cmds-fi-disk_usage.h +2 -5
index c315004cd806..95cf4aabefb4 100644
@@ -19,11 +19,8 @@
 #ifndef __CMDS_FI_DISK_USAGE__
 #define __CMDS_FI_DISK_USAGE__
 
-extern const char * const cmd_filesystem_df_usage[];
-int cmd_filesystem_df(int argc, char **argv);
-
-extern const char * const cmd_filesystem_disk_usage_usage[];
-int cmd_filesystem_disk_usage(int argc, char **argv);
+extern const char * const cmd_filesystem_usage_usage[];
+int cmd_filesystem_usage(int argc, char **argv);
 
 extern const char * const cmd_device_disk_usage_usage[];
 int cmd_device_disk_usage(int argc, char **argv);
cmds-filesystem.c +53 -2

73 lines not on this page · raw message

Signature · 1 line
1.9.0
author
#3 lore raw

PATCH 3/14 btrfs-progs: move device usage to cmds-device, more cleanups

In reply to David Sterba

Move the command definitions where they belong, keep common 'usage' functions in cmds-fi-disk_usage.c and add exports.

Rename structures containing 'disk' to 'device'.

Fix whitespace in the modified code.

Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  96 +++++++++++++++++-
 cmds-fi-disk_usage.c | 281 ++++++++++++---------------------------------------
 cmds-fi-disk_usage.h |  30 +++++-
 3 files changed, 188 insertions(+), 219 deletions(-)
cmds-device.c +94 -2
index a9b4a3859390..7a9d808b36dd 100644
@@ -423,6 +423,98 @@ out:
 	return err;
 }
 
+const char * const cmd_device_usage_usage[] = {
+	"btrfs device usage [-b] <path> [<path>..]",
+	"Show which chunks are in a device.",
+	"",
+	"-b\tSet byte as unit",
+	NULL
+};
+
+static int _cmd_device_usage(int fd, char *path, int mode)
+{
+	int i;
+	int ret = 0;
+	int info_count = 0;
+	struct chunk_info *info_ptr = 0;
+	struct device_info *device_info_ptr = 0;
+	int device_info_count = 0;
+
+	if (load_chunk_info(fd, &info_ptr, &info_count) ||
+	    load_device_info(fd, &device_info_ptr, &device_info_count)) {
+		ret = -1;
+		goto exit;
+	}
+
+	for (i = 0; i < device_info_count; i++) {
+		printf("%s\t%10s\n", device_info_ptr[i].path,
+			df_pretty_sizes(device_info_ptr[i].size, mode));
+
+		print_device_chunks(fd, device_info_ptr[i].devid,
+				device_info_ptr[i].size,
+				info_ptr, info_count,
+				mode);
+		printf("\n");
+	}
+
+exit:
+	if (device_info_ptr)
+		free(device_info_ptr);
+	if (info_ptr)
+		free(info_ptr);
+
+	return ret;
+}
+
+int cmd_device_usage(int argc, char **argv)
+{
+
+	int	flags = DF_HUMAN_UNIT;
+	int	i, more_than_one = 0;
+
+	optind = 1;
+	while (1) {
+		char	c = getopt(argc, argv, "b");
+
+		if (c < 0)
+			break;
+
+		switch (c) {
+		case 'b':
+			flags &= ~DF_HUMAN_UNIT;
+			break;
+		default:
+			usage(cmd_device_usage_usage);
+		}
+	}
+
+	if (check_argc_min(argc - optind, 1))
+		usage(cmd_device_usage_usage);
+
+	for (i = optind; i < argc ; i++) {
+		int r, fd;
+		DIR	*dirstream = NULL;
+		if (more_than_one)
+			printf("\n");
+
+		fd = open_file_or_dir(argv[i], &dirstream);
+		if (fd < 0) {
+			fprintf(stderr, "ERROR: can't access to '%s'\n",
+				argv[1]);
+			return 12;
+		}
+		r = _cmd_device_usage(fd, argv[i], flags);
+		close_file_or_dir(fd, dirstream);
+
+		if (r)
+			return r;
+		more_than_one = 1;
+
+	}
+
+	return 0;
+}
+
 const struct cmd_group device_cmd_group = {
 	device_cmd_group_usage, NULL, {
 		{ "add", cmd_add_dev, cmd_add_dev_usage, NULL, 0 },

@@ -430,8 +522,8 @@ const struct cmd_group device_cmd_group = {
 		{ "scan", cmd_scan_dev, cmd_scan_dev_usage, NULL, 0 },
 		{ "ready", cmd_ready_dev, cmd_ready_dev_usage, NULL, 0 },
 		{ "stats", cmd_dev_stats, cmd_dev_stats_usage, NULL, 0 },
-		{ "disk-usage", cmd_device_disk_usage,
-			cmd_device_disk_usage_usage, NULL, 0 },
+		{ "usage", cmd_device_usage,
+			cmd_device_usage_usage, NULL, 0 },
 		NULL_CMD_STRUCT
 	}
 };
cmds-fi-disk_usage.c +66 -215

505 lines not on this page · raw message

cmds-fi-disk_usage.h +28 -2
index 95cf4aabefb4..787b4eb56acf 100644
@@ -19,10 +19,36 @@
 #ifndef __CMDS_FI_DISK_USAGE__
 #define __CMDS_FI_DISK_USAGE__
 
+#define DF_HUMAN_UNIT		(1<<0)
+
 extern const char * const cmd_filesystem_usage_usage[];
 int cmd_filesystem_usage(int argc, char **argv);
 
-extern const char * const cmd_device_disk_usage_usage[];
-int cmd_device_disk_usage(int argc, char **argv);
+struct device_info {
+	u64	devid;
+	char	path[BTRFS_DEVICE_PATH_NAME_MAX];
+	u64	size;
+};
+
+/*
+ * To store the size information about the chunks:
+ * the chunks info are grouped by the tuple (type, devid, num_stripes),
+ * i.e. if two chunks are of the same type (RAID1, DUP...), are on the
+ * same disk, have the same stripes then their sizes are grouped
+ */
+struct chunk_info {
+	u64	type;
+	u64	size;
+	u64	devid;
+	u64	num_stripes;
+};
+
+int load_device_info(int fd, struct device_info **device_info_ptr,
+		int *device_info_count);
+int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count);
+char *df_pretty_sizes(u64 size, int mode);
+void print_device_chunks(int fd, u64 devid, u64 total_size,
+		struct chunk_info *chunks_info_ptr,
+		int chunks_info_count, int mode);
 
 #endif
Signature · 1 line
1.9.0
author
#4 lore raw

PATCH 4/14 btrfs-progs: check if we can't get info from ioctls due to permissions

In reply to David Sterba

The TREE_SEARCH ioctl is root-only, FS_INFO will be available for non-root users with an updated kernel, let the user know.

Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
cmds-fi-disk_usage.c +10 -0
index 17202de1b6e8..64c85acafd51 100644
@@ -171,6 +171,12 @@ int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count)
 	while (1) {
 		ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
 		e = errno;
+		if (ret == -EPERM) {
+			fprintf(stderr,
+				"ERROR: can't read detailed chunk info from ioctl(TREE_SEARCH), run as root\n");
+			return 0;
+		}
+
 		if (ret < 0) {
 			fprintf(stderr,
 				"ERROR: can't perform the search - %s\n",

@@ -461,6 +467,10 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 	*device_info_ptr = 0;
 
 	ret = ioctl(fd, BTRFS_IOC_FS_INFO, &fi_args);
+	if (ret == -EPERM) {
+		fprintf(stderr, "ERROR: can't get filesystem info from ioctl(FS_INFO), run as root\n");
+		return -1;
+	}
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: cannot get filesystem info\n");
 		return -1;
Signature · 1 line
1.9.0
author
#5 lore raw

PATCH 5/14 btrfs-progs: zero out structures before calling ioctl

In reply to David Sterba
Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
cmds-fi-disk_usage.c +3 -3
index 64c85acafd51..067c60078710 100644
@@ -248,7 +248,7 @@ static struct btrfs_ioctl_space_args *load_space_info(int fd, char *path)
 	struct btrfs_ioctl_space_args *sargs = 0, *sargs_orig = 0;
 	int e, ret, count;
 
-	sargs_orig = sargs = malloc(sizeof(struct btrfs_ioctl_space_args));
+	sargs_orig = sargs = calloc(1, sizeof(struct btrfs_ioctl_space_args));
 	if (!sargs) {
 		fprintf(stderr, "ERROR: not enough memory\n");
 		return NULL;

@@ -476,15 +476,15 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 		return -1;
 	}
 
-	info = malloc(sizeof(struct device_info) * fi_args.num_devices);
+	info = calloc(fi_args.num_devices, sizeof(struct device_info));
 	if (!info) {
 		fprintf(stderr, "ERROR: not enough memory\n");
 		return -1;
 	}
 
 	for (i = 0, ndevs = 0 ; i <= fi_args.max_id ; i++) {
-
 		BUG_ON(ndevs >= fi_args.num_devices);
+		memset(&dev_info, 0, sizeof(dev_info));
 		ret = get_device_info(fd, i, &dev_info);
 
 		if (ret == -ENODEV)
Signature · 1 line
1.9.0
author
#6 lore raw

PATCH 6/14 btrfs-progs: print B for bytes

In reply to David Sterba

This arguably helps parsers.

Signed-off-by David Sterba <[email protected]>
---
 utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
utils.c +1 -1
index ca150404ea6f..159abf8bd0e4 100644
@@ -1252,7 +1252,7 @@ out:
 	return ret;
 }
 
-static char *size_strs[] = { "", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"};
+static char *size_strs[] = { "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"};
 int pretty_size_snprintf(u64 size, char *str, size_t str_bytes)
 {
 	int num_divs = 0;
Signature · 1 line
1.9.0
author
#7 lore raw

PATCH 7/14 btrfs-progs: Print more info about device sizes

In reply to David Sterba

The entire device size may not be available to the filesystem, eg. if it's modified via resize. Print this information if it can be obtained from the DEV_INFO ioctl.

Print the device ID on the same line as the device name and move size to the next line.

Sample:
/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB
Data,RAID10: 512.00MiB
Metadata,RAID10: 512.00MiB
System,RAID10: 4.00MiB
Unallocated: 9.00GiB

Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  6 +++---
 cmds-fi-disk_usage.c | 13 ++++++++++++-
 cmds-fi-disk_usage.h |  6 +++++-
 3 files changed, 20 insertions(+), 5 deletions(-)
cmds-device.c +3 -3
index 7a9d808b36dd..519725f83e8c 100644
@@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode)
 	}
 
 	for (i = 0; i < device_info_count; i++) {
-		printf("%s\t%10s\n", device_info_ptr[i].path,
-			df_pretty_sizes(device_info_ptr[i].size, mode));
-
+		printf("%s, ID: %llu\n", device_info_ptr[i].path,
+				device_info_ptr[i].devid);
+		print_device_sizes(fd, &device_info_ptr[i], mode);
 		print_device_chunks(fd, device_info_ptr[i].devid,
 				device_info_ptr[i].size,
 				info_ptr, info_count,
cmds-fi-disk_usage.c +12 -1
index 067c60078710..ddb064cc4c66 100644
@@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 
 		info[ndevs].devid = dev_info.devid;
 		strcpy(info[ndevs].path, (char *)dev_info.path);
-		info[ndevs].size = get_partition_size((char *)dev_info.path);
+		info[ndevs].device_size = get_partition_size((char *)dev_info.path);
+		info[ndevs].size = dev_info.total_size;
 		++ndevs;
 	}
 

@@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size,
 	printf("   Unallocated: %*s%10s\n",
 		(int)(20 - strlen("Unallocated")), "",
 		df_pretty_sizes(total_size - allocated, mode));
+}
 
+void print_device_sizes(int fd, struct device_info *devinfo, int mode)
+{
+	printf("   Device size: %*s%10s\n",
+		(int)(20 - strlen("Device size")), "",
+		df_pretty_sizes(devinfo->device_size, mode));
+	printf("   FS occuppied:%*s%10s\n",
+		(int)(20 - strlen("FS occupied")), "",
+		df_pretty_sizes(devinfo->size, mode));
+	}
 }
cmds-fi-disk_usage.h +5 -1
index 787b4eb56acf..79cc2a115bc5 100644
@@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv);
 struct device_info {
 	u64	devid;
 	char	path[BTRFS_DEVICE_PATH_NAME_MAX];
-	u64	size;
+	/* Size of the block device */
+	u64	device_size;
+	/* Size that's occupied by the filesystem, can be changed via resize */
+	u64     size;
 };
 
 /*

@@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode);
 void print_device_chunks(int fd, u64 devid, u64 total_size,
 		struct chunk_info *chunks_info_ptr,
 		int chunks_info_count, int mode);
+void print_device_sizes(int fd, struct device_info *devinfo, int mode);
 
 #endif
Signature · 1 line
1.9.0
#8 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 11 lines · David Sterba

The entire device size may not be available to the filesystem, eg. if it's modified via resize. Print this information if it can be obtained from the DEV_INFO ioctl.

Print the device ID on the same line as the device name and move size to the next line.

Sample:
/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB

Spelling mistake. s/occuppied/occupied/.

Quoted 45 lines
   Data,RAID10:           512.00MiB
   Metadata,RAID10:       512.00MiB
   System,RAID10:           4.00MiB
   Unallocated:             9.00GiB
Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  6 +++---
 cmds-fi-disk_usage.c | 13 ++++++++++++-
 cmds-fi-disk_usage.h |  6 +++++-
 3 files changed, 20 insertions(+), 5 deletions(-)
cmds-device.c +3 -3
index 7a9d808b36dd..519725f83e8c 100644
@@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode)
        }
 
        for (i = 0; i < device_info_count; i++) {
-               printf("%s\t%10s\n", device_info_ptr[i].path,
-                       df_pretty_sizes(device_info_ptr[i].size, mode));
-
+               printf("%s, ID: %llu\n", device_info_ptr[i].path,
+                               device_info_ptr[i].devid);
+               print_device_sizes(fd, &device_info_ptr[i], mode);
                print_device_chunks(fd, device_info_ptr[i].devid,
                                device_info_ptr[i].size,
                                info_ptr, info_count,
cmds-fi-disk_usage.c +2 -1
index 067c60078710..ddb064cc4c66 100644
@@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 
                info[ndevs].devid = dev_info.devid;
                strcpy(info[ndevs].path, (char *)dev_info.path);
-               info[ndevs].size = get_partition_size((char *)dev_info.path);
+               info[ndevs].device_size = get_partition_size((char *)dev_info.path);
+               info[ndevs].size = dev_info.total_size;
                ++ndevs;
        }
 

@@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size,
        printf("   Unallocated: %*s%10s\n",

cmds-fi-disk_usage.c void print_device_chunks(int fd, u64 devid, u64 total_size, · line 890

Quoted 10 lines
cmds-fi-disk_usage.c +7 -0
                (int)(20 - strlen("Unallocated")), "",
                df_pretty_sizes(total_size - allocated, mode));
+}
 
+void print_device_sizes(int fd, struct device_info *devinfo, int mode)
+{
+       printf("   Device size: %*s%10s\n",
+               (int)(20 - strlen("Device size")), "",
+               df_pretty_sizes(devinfo->device_size, mode));
+       printf("   FS occuppied:%*s%10s\n",

Here too. s/occuppied/occupied/.

Quoted 28 lines
cmds-fi-disk_usage.c +3 -0
+               (int)(20 - strlen("FS occupied")), "",
+               df_pretty_sizes(devinfo->size, mode));
+       }
 }
cmds-fi-disk_usage.h +5 -2
index 787b4eb56acf..79cc2a115bc5 100644
@@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv);
 struct device_info {
        u64     devid;
        char    path[BTRFS_DEVICE_PATH_NAME_MAX];
-       u64     size;
+       /* Size of the block device */
+       u64     device_size;
+       /* Size that's occupied by the filesystem, can be changed via resize */
+       u64     size;
 };
 
 /*

@@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode);
 void print_device_chunks(int fd, u64 devid, u64 total_size,
                struct chunk_info *chunks_info_ptr,
                int chunks_info_count, int mode);
+void print_device_sizes(int fd, struct device_info *devinfo, int mode);
 
 #endif
--
Quoted 1 line

1.9.0

Same spelling mistake (occuppied) also occurs in the following patches too:
[PATCH 08/14] btrfs-progs: compare unallocated space against the correct value [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode

Thanks,
Mike

#9 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

I David
thanks, to take care of these enhancements.

Quoted 8 lines · Mike Fleetwood
Quoted 7 lines · David Sterba

The entire device size may not be available to the filesystem, eg. if it's modified via resize. Print this information if it can be obtained from the DEV_INFO ioctl.

Print the device ID on the same line as the device name and move size to the next line.

Quoted 6 lines
Quoted 4 lines

Sample:
/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB

Spelling mistake. s/occuppied/occupied/.

I found a bit unclear the "FS occupied" terms.
Can I suggest "Resized to:" instead of "FS occupied:", and to show it only when the two values differ ? In fact this value has a meaning only if a filesystem is resized.

BR
G.Baroncelli

Quoted 100 lines
Quoted 46 lines
   Data,RAID10:           512.00MiB
   Metadata,RAID10:       512.00MiB
   System,RAID10:           4.00MiB
   Unallocated:             9.00GiB
Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  6 +++---
 cmds-fi-disk_usage.c | 13 ++++++++++++-
 cmds-fi-disk_usage.h |  6 +++++-
 3 files changed, 20 insertions(+), 5 deletions(-)
cmds-device.c +3 -3
index 7a9d808b36dd..519725f83e8c 100644
@@ -447,9 +447,9 @@ static int _cmd_device_usage(int fd, char *path, int mode)
        }
 
        for (i = 0; i < device_info_count; i++) {
-               printf("%s\t%10s\n", device_info_ptr[i].path,
-                       df_pretty_sizes(device_info_ptr[i].size, mode));
-
+               printf("%s, ID: %llu\n", device_info_ptr[i].path,
+                               device_info_ptr[i].devid);
+               print_device_sizes(fd, &device_info_ptr[i], mode);
                print_device_chunks(fd, device_info_ptr[i].devid,
                                device_info_ptr[i].size,
                                info_ptr, info_count,
cmds-fi-disk_usage.c +2 -1
index 067c60078710..ddb064cc4c66 100644
@@ -499,7 +499,8 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 
                info[ndevs].devid = dev_info.devid;
                strcpy(info[ndevs].path, (char *)dev_info.path);
-               info[ndevs].size = get_partition_size((char *)dev_info.path);
+               info[ndevs].device_size = get_partition_size((char *)dev_info.path);
+               info[ndevs].size = dev_info.total_size;
                ++ndevs;
        }
 

@@ -879,5 +880,15 @@ void print_device_chunks(int fd, u64 devid, u64 total_size,
        printf("   Unallocated: %*s%10s\n",

cmds-fi-disk_usage.c void print_device_chunks(int fd, u64 devid, u64 total_size, · line 890

Quoted 10 lines
cmds-fi-disk_usage.c +7 -0
                (int)(20 - strlen("Unallocated")), "",
                df_pretty_sizes(total_size - allocated, mode));
+}
 
+void print_device_sizes(int fd, struct device_info *devinfo, int mode)
+{
+       printf("   Device size: %*s%10s\n",
+               (int)(20 - strlen("Device size")), "",
+               df_pretty_sizes(devinfo->device_size, mode));
+       printf("   FS occuppied:%*s%10s\n",

Here too. s/occuppied/occupied/.

Quoted 28 lines
cmds-fi-disk_usage.c +3 -0
+               (int)(20 - strlen("FS occupied")), "",
+               df_pretty_sizes(devinfo->size, mode));
+       }
 }
cmds-fi-disk_usage.h +5 -2
index 787b4eb56acf..79cc2a115bc5 100644
@@ -27,7 +27,10 @@ int cmd_filesystem_usage(int argc, char **argv);
 struct device_info {
        u64     devid;
        char    path[BTRFS_DEVICE_PATH_NAME_MAX];
-       u64     size;
+       /* Size of the block device */
+       u64     device_size;
+       /* Size that's occupied by the filesystem, can be changed via resize */
+       u64     size;
 };
 
 /*

@@ -50,5 +53,6 @@ char *df_pretty_sizes(u64 size, int mode);
 void print_device_chunks(int fd, u64 devid, u64 total_size,
                struct chunk_info *chunks_info_ptr,
                int chunks_info_count, int mode);
+void print_device_sizes(int fd, struct device_info *devinfo, int mode);
 
 #endif
--
Quoted 1 line

1.9.0

Same spelling mistake (occuppied) also occurs in the following patches too:
[PATCH 08/14] btrfs-progs: compare unallocated space against the correct value [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
author
#10 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 8 lines · Goffredo Baroncelli
Quoted 6 lines
Quoted 4 lines

Sample:
/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB

Spelling mistake. s/occuppied/occupied/.

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

Here's what we have already:

- used
- allocated
- reserved
- total

'occupied' seemed like a good choice to me, though it may be not obvious at first. The rationale behind is to denote space that the filesystem can potentially use on the device - in one word.

Quoted 3 lines

Can I suggest "Resized to:" instead of "FS occupied:", and to show it only when the two values differ ? In fact this value has a meaning only if a filesystem is resized.

'Resized to' sounds good to me and makes sense in the context of the output:

/dev/sda6, ID: 2
Device size: 10.00GiB
Resized to: 5.00GiB
Data,RAID10: 512.00MiB
Metadata,RAID10: 512.00MiB
System,RAID10: 4.00MiB
Unallocated: 9.00GiB

The next point, display only if it's different from size. This makes calculating the filesystem size conditional and if eg. I filter the output I can't simply do 'grep -i occupied'.

The points against that are that this makes the output longer and the resized devices are probably not the most common scenario.

So as a compromise, we can add an option to enable more verbose output.

#11 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 6 lines · David Sterba
Quoted 5 lines · Goffredo Baroncelli
Quoted 5 lines
Quoted 4 lines

Sample:
/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Spelling mistake. s/occuppied/occupied/.

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

'occupied' seemed like a good choice to me, though it may be not obvious

The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization.

/dev/sda6, ID: 2
Device size: 10.00GiB
Filesystem size: 5.00GiB

and perhaps only show that "Device size" line if it differs from the filesystem size by more than a block.

Frank

#12 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 8 lines · Frank Kingswood
Quoted 7 lines · David Sterba
Quoted 2 lines · Goffredo Baroncelli

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

Quoted 8 lines
Quoted 1 line

'occupied' seemed like a good choice to me, though it may be not obvious

The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization.

/dev/sda6, ID: 2
Device size: 10.00GiB
Filesystem size: 5.00GiB

FS size was what I was about to suggest, before I saw your reply.

It makes more sense to me than 'Occupied' and seems cleaner than 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV sizes, too.

Signature · 1 line
David Taylor
#13 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 11 lines · David Taylor
Quoted 10 lines · Frank Kingswood
Quoted 8 lines · David Sterba
Quoted 2 lines · Goffredo Baroncelli

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

'occupied' seemed like a good choice to me, though it may be not obvious

Quoted 8 lines
Quoted 6 lines

The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization.

/dev/sda6, ID: 2
Device size: 10.00GiB
Filesystem size: 5.00GiB

FS size was what I was about to suggest, before I saw your reply.

Pay attention that this value is not the Filesystem size,
but to the maximum space the of THE DEVICE the filesystem is allowed to use.

The filesystem size (the space available or the sum of the space available and the one occupied) is based on this value but it should be very different (think about a RAID 1 on three device of different sizes....)

Quoted 5 lines

It makes more sense to me than 'Occupied' and seems cleaner than 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV sizes, too.

Signature · 2 lines
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
author
#14 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

Quoted 15 lines · Goffredo Baroncelli
Quoted 14 lines · David Taylor
Quoted 13 lines · Frank Kingswood
Quoted 8 lines · David Sterba
Quoted 2 lines · Goffredo Baroncelli

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

'occupied' seemed like a good choice to me, though it may be not obvious

The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

/dev/sda6, ID: 2
Device size: 10.00GiB
Filesystem size: 5.00GiB

FS size was what I was about to suggest, before I saw your reply.

Pay attention that this value is not the Filesystem size,
but to the maximum space the of THE DEVICE the filesystem is allowed to use.

I agree that plain 'Filesystem size' could be misleading, using the same term that has an established meaning can cause misuderstandings in bugreports.

author
#15 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

In reply to David Taylor
Quoted 3 lines · David Taylor

It makes more sense to me than 'Occupied' and seems cleaner than 'Resized To'. It sort of mirrors how LVM describes PV / VG / LV sizes, too.

Do you have a concrete example how we could map the btrfs sizes based on LVM?

author
#16 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

In reply to Frank Kingswood
Quoted 13 lines · Frank Kingswood
Quoted 9 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines

Device size: 10.00GiB
FS occuppied: 5.00GiB

I found a bit unclear the "FS occupied" terms.

We're running out of terms to describe and distinguish the space that the filesystem uses.

'occupied' seemed like a good choice to me, though it may be not obvious

The space that the filesystem uses in total seems to me is called the "size". It has nothing to do with utilization.

Quoted 6 lines

/dev/sda6, ID: 2
Device size: 10.00GiB
Filesystem size: 5.00GiB

and perhaps only show that "Device size" line if it differs from the filesystem size by more than a block.

I have noticed that the help text of 'btrfs fi resize' already uses 'occupy':

$ btrfs fi resize --help
usage: btrfs filesystem resize [devid:][+/-]<newsize>[gkm]|[devid:]max <path>

Resize a filesystem

If 'max' is passed, the filesystem will occupy all available space on the device 'devid'.

First appeared in

commit 35401ac1902544637ac26634e139f04ae31c2cbf

Author: Goffredo Baroncelli <[email protected]>
Date:   Thu Mar 11 22:32:50 2010 +0100

I've been thinking about all the proposals and I'm not feeling comfortable to pick the winner yet.

So, I'll hide the entry from the output for now so the patchset can be pushed next step upstream but does not introduce something that's visible in UI but not agreed on.

author
#17 lore raw

Re: [PATCH 07/14] btrfs-progs: Print more info about device sizes

In reply to Mike Fleetwood
Quoted 15 lines · Mike Fleetwood
Quoted 3 lines

/dev/sda7, ID: 3
Device size: 10.00GiB
FS occuppied: 5.00GiB

Spelling mistake. s/occuppied/occupied/.

cmds-fi-disk_usage.c void print_device_chunks(int fd, u64 devid, u64 total_size, · line 890

Quoted 6 lines
cmds-fi-disk_usage.c +6 -0
+void print_device_sizes(int fd, struct device_info *devinfo, int mode)
+{
+       printf("   Device size: %*s%10s\n",
+               (int)(20 - strlen("Device size")), "",
+               df_pretty_sizes(devinfo->device_size, mode));
+       printf("   FS occuppied:%*s%10s\n",

Here too. s/occuppied/occupied/.

Quoted 8 lines

cmds-fi-disk_usage.c void print_device_chunks(int fd, u64 devid, u64 total_size, · line 894

Quoted 4 lines
cmds-fi-disk_usage.c +3 -0
+               (int)(20 - strlen("FS occupied")), "",
+               df_pretty_sizes(devinfo->size, mode));
+       }
 }

Same spelling mistake (occuppied) also occurs in the following patches too:
[PATCH 08/14] btrfs-progs: compare unallocated space against the correct value [PATCH 12/14] btrfs-progs: replace df_pretty_sizes with pretty_size_mode

Thanks for catching it.

author
#18 lore raw

PATCH 8/14 btrfs-progs: compare unallocated space against the correct value

In reply to David Sterba

The device may not be fully occupied by the filesystem, the value of Unallocated should not be calculated against the device size but the size provided by DEV_INFO.

Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        | 6 ++----
 cmds-fi-disk_usage.c | 9 ++++-----
 cmds-fi-disk_usage.h | 2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)
cmds-device.c +2 -4
index 519725f83e8c..d7889f93d343 100644
@@ -450,10 +450,8 @@ static int _cmd_device_usage(int fd, char *path, int mode)
 		printf("%s, ID: %llu\n", device_info_ptr[i].path,
 				device_info_ptr[i].devid);
 		print_device_sizes(fd, &device_info_ptr[i], mode);
-		print_device_chunks(fd, device_info_ptr[i].devid,
-				device_info_ptr[i].size,
-				info_ptr, info_count,
-				mode);
+		print_device_chunks(fd, &device_info_ptr[i],
+				info_ptr, info_count, mode);
 		printf("\n");
 	}
cmds-fi-disk_usage.c +4 -5
index ddb064cc4c66..7d0334086dbd 100644
@@ -500,7 +500,7 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 		info[ndevs].devid = dev_info.devid;
 		strcpy(info[ndevs].path, (char *)dev_info.path);
 		info[ndevs].device_size = get_partition_size((char *)dev_info.path);
-		info[ndevs].size = dev_info.total_size;
+		info[ndevs].size = dev_info.total_bytes;
 		++ndevs;
 	}
 

@@ -847,7 +847,7 @@ int cmd_filesystem_usage(int argc, char **argv)
 	return 0;
 }
 
-void print_device_chunks(int fd, u64 devid, u64 total_size,
+void print_device_chunks(int fd, struct device_info *devinfo,
 		struct chunk_info *chunks_info_ptr,
 		int chunks_info_count, int mode)
 {

@@ -860,7 +860,7 @@ void print_device_chunks(int fd, u64 devid, u64 total_size,
 		u64 flags;
 		u64 size;
 
-		if (chunks_info_ptr[i].devid != devid)
+		if (chunks_info_ptr[i].devid != devinfo->devid)
 			continue;
 
 		flags = chunks_info_ptr[i].type;

@@ -879,7 +879,7 @@ void print_device_chunks(int fd, u64 devid, u64 total_size,
 	}
 	printf("   Unallocated: %*s%10s\n",
 		(int)(20 - strlen("Unallocated")), "",
-		df_pretty_sizes(total_size - allocated, mode));
+		df_pretty_sizes(devinfo->size - allocated, mode));
 }
 
 void print_device_sizes(int fd, struct device_info *devinfo, int mode)

@@ -890,5 +890,4 @@ void print_device_sizes(int fd, struct device_info *devinfo, int mode)
 	printf("   FS occuppied:%*s%10s\n",
 		(int)(20 - strlen("FS occupied")), "",
 		df_pretty_sizes(devinfo->size, mode));
-	}
 }
cmds-fi-disk_usage.h +1 -1
index 79cc2a115bc5..dbc2a10f31eb 100644
@@ -50,7 +50,7 @@ int load_device_info(int fd, struct device_info **device_info_ptr,
 		int *device_info_count);
 int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count);
 char *df_pretty_sizes(u64 size, int mode);
-void print_device_chunks(int fd, u64 devid, u64 total_size,
+void print_device_chunks(int fd, struct device_info *devinfo,
 		struct chunk_info *chunks_info_ptr,
 		int chunks_info_count, int mode);
 void print_device_sizes(int fd, struct device_info *devinfo, int mode);
Signature · 1 line
1.9.0
author
#19 lore raw

PATCH 9/14 btrfs-progs: add section of overall filesystem usage

In reply to David Sterba

The 'fi usage' lacks an overall report, this used to be in the enhanced df command. Add it back.

Sample:
Overall:
Device size: 35.00GiB
Device allocated: 8.07GiB
Device unallocated: 26.93GiB
Used: 1.12MiB
Free (Estimated): 17.57GiB (Max: 30.98GiB, min: 17.52GiB)
Data to device ratio: 50 %
...

Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
cmds-fi-disk_usage.c +13 -12
index 7d0334086dbd..d76fa3552136 100644
@@ -302,8 +302,6 @@ static struct btrfs_ioctl_space_args *load_space_info(int fd, char *path)
 	return sargs;
 }
 
-/* Not used, keep for later */
-#if 0
 /*
  * This function computes the space occuped by a *single* RAID5/RAID6 chunk.
  * The computation is performed on the basis of the number of stripes

@@ -331,7 +329,6 @@ static int get_raid56_used(int fd, u64 *raid5_used, u64 *raid6_used)
 	free(info_ptr);
 
 	return 0;
-
 }
 
 static int _cmd_disk_free(int fd, char *path, int mode)

@@ -416,22 +413,24 @@ static int _cmd_disk_free(int fd, char *path, int mode)
 	else
 		width = 18;
 
-	printf("Disk size:\t\t%*s\n", width,
+	printf("Overall:\n");
+
+	printf("    Device size:\t\t%*s\n", width,
 		df_pretty_sizes(total_disk, mode));
-	printf("Disk allocated:\t\t%*s\n", width,
+	printf("    Device allocated:\t\t%*s\n", width,
 		df_pretty_sizes(total_chunks, mode));
-	printf("Disk unallocated:\t%*s\n", width,
+	printf("    Device unallocated:\t\t%*s\n", width,
 		df_pretty_sizes(total_disk-total_chunks, mode));
-	printf("Used:\t\t\t%*s\n", width,
+	printf("    Used:\t\t\t%*s\n", width,
 		df_pretty_sizes(total_used, mode));
-	printf("Free (Estimated):\t%*s\t(",
+	printf("    Free (Estimated):\t\t%*s\t(",
 		width,
 		df_pretty_sizes((u64)(K*total_disk-total_used), mode));
 	printf("Max: %s, ",
 		df_pretty_sizes(total_disk-total_chunks+total_free, mode));
 	printf("min: %s)\n",
 		df_pretty_sizes((total_disk-total_chunks)/2+total_free, mode));
-	printf("Data to disk ratio:\t%*.0f %%\n",
+	printf("    Data to device ratio:\t%*.0f %%\n",
 		width-2, K*100);
 
 exit:

@@ -441,7 +440,6 @@ exit:
 
 	return ret;
 }
-#endif
 
 /*
  *  Helper to sort the device_info structure

@@ -826,8 +824,6 @@ int cmd_filesystem_usage(int argc, char **argv)
 	for (i = optind; i < argc ; i++) {
 		int r, fd;
 		DIR	*dirstream = NULL;
-		if (more_than_one)
-			printf("\n");
 
 		fd = open_file_or_dir(argv[i], &dirstream);
 		if (fd < 0) {

@@ -835,6 +831,11 @@ int cmd_filesystem_usage(int argc, char **argv)
 				argv[1]);
 			return 12;
 		}
+		if (more_than_one)
+			printf("\n");
+
+		r = _cmd_disk_free(fd, argv[i], flags);
+		printf("\n");
 		r = _cmd_filesystem_usage(fd, argv[i], flags, tabular);
 		close_file_or_dir(fd, dirstream);
Signature · 1 line
1.9.0
author
#20 lore raw

PATCH 10/14 btrfs-progs: cleanup filesystem/device usage code

In reply to David Sterba

The main point of this is to load the device and chunk infos at one place and pass down to the printers. The EPERM is handled separately, in case kernel does not give us all the information about chunks or devices, but we want to warn and print at least something.

For non-root users, 'filesystem usage' prints only the overall stats and warns about RAID5/6.

The sole cleanup changes affect mostly the modified code and the related functions, should be reasonably small.

Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  31 +++++-----
 cmds-fi-disk_usage.c | 163 +++++++++++++++++++++++++++------------------------
 cmds-fi-disk_usage.h |   5 +-
 3 files changed, 105 insertions(+), 94 deletions(-)
cmds-device.c +15 -16
index d7889f93d343..29eeaf6bbd0f 100644
@@ -435,31 +435,29 @@ static int _cmd_device_usage(int fd, char *path, int mode)
 {
 	int i;
 	int ret = 0;
-	int info_count = 0;
-	struct chunk_info *info_ptr = 0;
-	struct device_info *device_info_ptr = 0;
-	int device_info_count = 0;
+	struct chunk_info *chunkinfo = NULL;
+	struct device_info *devinfo = NULL;
+	int chunkcount = 0;
+	int devcount = 0;
 
-	if (load_chunk_info(fd, &info_ptr, &info_count) ||
-	    load_device_info(fd, &device_info_ptr, &device_info_count)) {
+	ret = load_chunk_and_device_info(fd, &chunkinfo, &chunkcount, &devinfo,
+			&devcount);
+	if (ret) {
 		ret = -1;
 		goto exit;
 	}
 
-	for (i = 0; i < device_info_count; i++) {
-		printf("%s, ID: %llu\n", device_info_ptr[i].path,
-				device_info_ptr[i].devid);
-		print_device_sizes(fd, &device_info_ptr[i], mode);
-		print_device_chunks(fd, &device_info_ptr[i],
-				info_ptr, info_count, mode);
+	for (i = 0; i < devcount; i++) {
+		printf("%s, ID: %llu\n", devinfo[i].path, devinfo[i].devid);
+		print_device_sizes(fd, &devinfo[i], mode);
+		print_device_chunks(fd, &devinfo[i], chunkinfo, chunkcount,
+				mode);
 		printf("\n");
 	}
 
 exit:
-	if (device_info_ptr)
-		free(device_info_ptr);
-	if (info_ptr)
-		free(info_ptr);
+	free(devinfo);
+	free(chunkinfo);
 
 	return ret;
 }

@@ -501,6 +499,7 @@ int cmd_device_usage(int argc, char **argv)
 				argv[1]);
 			return 12;
 		}
+
 		r = _cmd_device_usage(fd, argv[i], flags);
 		close_file_or_dir(fd, dirstream);
cmds-fi-disk_usage.c +88 -75

257 lines not on this page · raw message

cmds-fi-disk_usage.h +2 -3
index dbc2a10f31eb..0779defc71db 100644
@@ -46,9 +46,8 @@ struct chunk_info {
 	u64	num_stripes;
 };
 
-int load_device_info(int fd, struct device_info **device_info_ptr,
-		int *device_info_count);
-int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count);
+int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
+		int *chunkcount, struct device_info **devinfo, int *devcount);
 char *df_pretty_sizes(u64 size, int mode);
 void print_device_chunks(int fd, struct device_info *devinfo,
 		struct chunk_info *chunks_info_ptr,
Signature · 1 line
1.9.0
author
#21 lore raw

PATCH 11/14 btrfs-progs: extend pretty printers with unit mode

In reply to David Sterba

The functionality of pretty unit printing was duplicated by df_pretty_sizes, merge it with pretty_size and enhance the interface with more suffix mode. Raw, binary or decimal.

Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c |  9 ++-----
 utils.c              | 71 ++++++++++++++++++++++++++++++++++++----------------
 utils.h              | 21 +++++++++++-----
 3 files changed, 66 insertions(+), 35 deletions(-)
cmds-fi-disk_usage.c +2 -7
index e472833c646d..c823223ee8a8 100644
@@ -33,18 +33,13 @@
 
 /*
  * Pretty print the size
- * PAY ATTENTION: it return a statically buffer
  */
 char *df_pretty_sizes(u64 size, int mode)
 {
-	static char buf[30];
-
 	if (mode & DF_HUMAN_UNIT)
-		(void)pretty_size_snprintf(size, buf, sizeof(buf));
+		return pretty_size_mode(size, UNITS_HUMAN);
 	else
-		sprintf(buf, "%llu", size);
-
-	return buf;
+		return pretty_size_mode(size, UNITS_RAW);
 }
 
 /*
utils.c +49 -22

84 lines not on this page · raw message

utils.h +15 -6
index 2d08e0b2a3a4..3aea8b47d9e7 100644
@@ -39,6 +39,14 @@
 
 #define BTRFS_UUID_UNPARSED_SIZE	37
 
+/*
+ * Output mode of byte units
+ */
+#define UNITS_RAW			(1)
+#define UNITS_BINARY			(2)
+#define UNITS_DECIMAL			(3)
+#define UNITS_HUMAN			UNITS_BINARY
+
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize, u64 features);

@@ -59,12 +67,13 @@ 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);
-#define pretty_size(size) 						\
-	({								\
-		static __thread char _str[24];				\
-		(void)pretty_size_snprintf((size), _str, sizeof(_str));	\
-		_str;							\
+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)
+#define pretty_size_mode(size, mode) 					      \
+	({								      \
+		static __thread char _str[32];				      \
+		(void)pretty_size_snprintf((size), _str, sizeof(_str), mode); \
+		_str;							      \
 	})
 
 int get_mountpt(char *dev, char *mntpt, size_t size);
Signature · 1 line
1.9.0
author
#22 lore raw

PATCH 12/14 btrfs-progs: replace df_pretty_sizes with pretty_size_mode

In reply to David Sterba
Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        |  8 +++----
 cmds-fi-disk_usage.c | 63 ++++++++++++++++++++++------------------------------
 cmds-fi-disk_usage.h |  3 ---
 3 files changed, 30 insertions(+), 44 deletions(-)
cmds-device.c +4 -4
index 29eeaf6bbd0f..b2773f249cba 100644
@@ -465,19 +465,19 @@ exit:
 int cmd_device_usage(int argc, char **argv)
 {
 
-	int	flags = DF_HUMAN_UNIT;
+	int mode = UNITS_HUMAN;
 	int	i, more_than_one = 0;
 
 	optind = 1;
 	while (1) {
-		char	c = getopt(argc, argv, "b");
+		int c = getopt(argc, argv, "b");
 
 		if (c < 0)
 			break;
 
 		switch (c) {
 		case 'b':
-			flags &= ~DF_HUMAN_UNIT;
+			mode = UNITS_RAW;
 			break;
 		default:
 			usage(cmd_device_usage_usage);

@@ -500,7 +500,7 @@ int cmd_device_usage(int argc, char **argv)
 			return 12;
 		}
 
-		r = _cmd_device_usage(fd, argv[i], flags);
+		r = _cmd_device_usage(fd, argv[i], mode);
 		close_file_or_dir(fd, dirstream);
 
 		if (r)
cmds-fi-disk_usage.c +26 -37

173 lines not on this page · raw message

cmds-fi-disk_usage.h +0 -3
index 0779defc71db..8a0c60f011e4 100644
@@ -19,8 +19,6 @@
 #ifndef __CMDS_FI_DISK_USAGE__
 #define __CMDS_FI_DISK_USAGE__
 
-#define DF_HUMAN_UNIT		(1<<0)
-
 extern const char * const cmd_filesystem_usage_usage[];
 int cmd_filesystem_usage(int argc, char **argv);
 

@@ -48,7 +46,6 @@ struct chunk_info {
 
 int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
 		int *chunkcount, struct device_info **devinfo, int *devcount);
-char *df_pretty_sizes(u64 size, int mode);
 void print_device_chunks(int fd, struct device_info *devinfo,
 		struct chunk_info *chunks_info_ptr,
 		int chunks_info_count, int mode);
Signature · 1 line
1.9.0
author
#23 lore raw

PATCH 13/14 btrfs-progs: clean up return codes and paths

In reply to David Sterba

Use the common patterns with one return statement at the end, pass down error

Signed-off-by David Sterba <[email protected]>
---
 cmds-device.c        | 27 +++++++++++++--------------
 cmds-fi-disk_usage.c | 39 +++++++++++++++++++++------------------
 2 files changed, 34 insertions(+), 32 deletions(-)
cmds-device.c +13 -14
index b2773f249cba..41a5a5c6ca8e 100644
@@ -442,10 +442,8 @@ static int _cmd_device_usage(int fd, char *path, int mode)
 
 	ret = load_chunk_and_device_info(fd, &chunkinfo, &chunkcount, &devinfo,
 			&devcount);
-	if (ret) {
-		ret = -1;
-		goto exit;
-	}
+	if (ret)
+		goto out;
 
 	for (i = 0; i < devcount; i++) {
 		printf("%s, ID: %llu\n", devinfo[i].path, devinfo[i].devid);

@@ -455,7 +453,7 @@ static int _cmd_device_usage(int fd, char *path, int mode)
 		printf("\n");
 	}
 
-exit:
+out:
 	free(devinfo);
 	free(chunkinfo);
 

@@ -466,6 +464,7 @@ int cmd_device_usage(int argc, char **argv)
 {
 
 	int mode = UNITS_HUMAN;
+	int ret = 0;
 	int	i, more_than_one = 0;
 
 	optind = 1;

@@ -488,28 +487,28 @@ int cmd_device_usage(int argc, char **argv)
 		usage(cmd_device_usage_usage);
 
 	for (i = optind; i < argc ; i++) {
-		int r, fd;
+		int fd;
 		DIR	*dirstream = NULL;
 		if (more_than_one)
 			printf("\n");
 
 		fd = open_file_or_dir(argv[i], &dirstream);
 		if (fd < 0) {
-			fprintf(stderr, "ERROR: can't access to '%s'\n",
+			fprintf(stderr, "ERROR: can't access '%s'\n",
 				argv[1]);
-			return 12;
+			ret = 1;
+			goto out;
 		}
 
-		r = _cmd_device_usage(fd, argv[i], mode);
+		ret = _cmd_device_usage(fd, argv[i], mode);
 		close_file_or_dir(fd, dirstream);
 
-		if (r)
-			return r;
+		if (ret)
+			goto out;
 		more_than_one = 1;
-
 	}
-
-	return 0;
+out:
+	return !!ret;
 }
 
 const struct cmd_group device_cmd_group = {
cmds-fi-disk_usage.c +21 -18
index 632ded0d21ab..d0b061ec7eb0 100644
@@ -67,7 +67,7 @@ static int add_info_to_list(struct chunk_info **info_ptr,
 
 			if (!res) {
 				fprintf(stderr, "ERROR: not enough memory\n");
-				return -1;
+				return -ENOMEM;
 			}
 
 			*info_ptr = res;

@@ -162,7 +162,7 @@ static int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count
 			fprintf(stderr,
 				"ERROR: can't perform the search - %s\n",
 				strerror(e));
-			return -99;
+			return ret;
 		}
 		/* the ioctl returns the number of item it found in nr_items */
 

@@ -178,10 +178,11 @@ static int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count
 			off += sizeof(*sh);
 			item = (struct btrfs_chunk *)(args.buf + off);
 
-			if (add_info_to_list(info_ptr, info_count, item)) {
+			ret = add_info_to_list(info_ptr, info_count, item);
+			if (ret) {
 				*info_ptr = 0;
 				free(*info_ptr);
-				return -100;
+				return ret;
 			}
 
 			off += sh->len;

@@ -319,8 +320,9 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 	double K;
 	u64 raid5_used, raid6_used;
 
-	if ((sargs = load_space_info(fd, path)) == NULL) {
-		ret = -1;
+	sargs = load_space_info(fd, path);
+	if (!sargs) {
+		ret = 1;
 		goto exit;
 	}
 

@@ -331,7 +333,7 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 			"ERROR: couldn't get space info on '%s' - %s\n",
 			path, strerror(e));
 
-		ret = 19;
+		ret = 1;
 		goto exit;
 	}
 	get_raid56_used(fd, chunkinfo, chunkcount, &raid5_used, &raid6_used);

@@ -439,13 +441,13 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 		return ret;
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: cannot get filesystem info\n");
-		return -1;
+		return ret;
 	}
 
 	info = calloc(fi_args.num_devices, sizeof(struct device_info));
 	if (!info) {
 		fprintf(stderr, "ERROR: not enough memory\n");
-		return -1;
+		return ret;
 	}
 
 	for (i = 0, ndevs = 0 ; i <= fi_args.max_id ; i++) {

@@ -460,7 +462,7 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 			    "ERROR: cannot get info about device devid=%d\n",
 			    i);
 			free(info);
-			return -1;
+			return ret;
 		}
 
 		info[ndevs].devid = dev_info.devid;

@@ -749,7 +751,7 @@ static int print_filesystem_usage_by_chunk(int fd,
 	sargs = load_space_info(fd, path);
 	if (!sargs) {
 		ret = 1;
-		goto exit;
+		goto out;
 	}
 
 	if (tabular)

@@ -759,9 +761,8 @@ static int print_filesystem_usage_by_chunk(int fd,
 		_cmd_filesystem_usage_linear(mode, sargs, chunkinfo,
 				chunkcount, devinfo, devcount);
 
-exit:
 	free(sargs);
-
+out:
 	return ret;
 }
 

@@ -777,6 +778,7 @@ const char * const cmd_filesystem_usage_usage[] = {
 int cmd_filesystem_usage(int argc, char **argv)
 {
 	int mode = UNITS_HUMAN;
+	int ret = 0;
 	int	i, more_than_one = 0;
 	int	tabular = 0;
 

@@ -803,7 +805,6 @@ int cmd_filesystem_usage(int argc, char **argv)
 		usage(cmd_filesystem_usage_usage);
 
 	for (i = optind; i < argc; i++) {
-		int ret;
 		int fd;
 		DIR *dirstream = NULL;
 		struct chunk_info *chunkinfo = NULL;

@@ -813,9 +814,10 @@ int cmd_filesystem_usage(int argc, char **argv)
 
 		fd = open_file_or_dir(argv[i], &dirstream);
 		if (fd < 0) {
-			fprintf(stderr, "ERROR: can't access to '%s'\n",
+			fprintf(stderr, "ERROR: can't access '%s'\n",
 				argv[1]);
-			return 12;
+			ret = 1;
+			goto out;
 		}
 		if (more_than_one)
 			printf("\n");

@@ -838,11 +840,12 @@ cleanup:
 		free(devinfo);
 
 		if (ret)
-			return ret;
+			goto out;
 		more_than_one = 1;
 	}
 
-	return 0;
+out:
+	return !!ret;
 }
 
 void print_device_chunks(int fd, struct device_info *devinfo,
Signature · 1 line
1.9.0
author
#24 lore raw

PATCH 14/14 btrfs-progs: move global reserve to overall summary

In reply to David Sterba

It looks confusing among the chunks, it is not in fact a chunk type.

Sample:
Overall:
Device size: 35.00GiB
Device allocated: 8.07GiB
Device unallocated: 26.93GiB
Used: 1.12MiB
Free (Estimated): 17.57GiB (Max: 30.98GiB, min: 17.52GiB)
Data to device ratio: 50 %
Global reserve: 16.00MiB (used: 0.00B)
...

Signed-off-by David Sterba <[email protected]>
---
 cmds-fi-disk_usage.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)
cmds-fi-disk_usage.c +20 -2
index d0b061ec7eb0..d8222d4d094d 100644
@@ -319,6 +319,8 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 	u64 total_free;		/* logical space un-used */
 	double K;
 	u64 raid5_used, raid6_used;
+	u64 global_reserve;
+	u64 global_reserve_used;
 
 	sargs = load_space_info(fd, path);
 	if (!sargs) {

@@ -341,6 +343,8 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 	total_chunks = 0;
 	total_used = 0;
 	total_free = 0;
+	global_reserve = 0;
+	global_reserve_used = 0;
 
 	for (i = 0; i < sargs->total_spaces; i++) {
 		float ratio = 1;

@@ -366,6 +370,11 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 		else
 			ratio = 1;
 
+		if (flags & BTRFS_SPACE_INFO_GLOBAL_RSV) {
+			global_reserve = sargs->spaces[i].total_bytes;
+			global_reserve_used = sargs->spaces[i].used_bytes;
+		}
+
 		allocated = sargs->spaces[i].total_bytes * ratio;
 
 		total_chunks += allocated;

@@ -404,6 +413,9 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 		pretty_size_mode((total_disk-total_chunks) / 2 + total_free, mode));
 	printf("    Data to device ratio:\t%*.0f %%\n",
 		width - 2, K * 100);
+	printf("    Global reserve:\t\t%*s\t(used: %s)\n", width,
+		pretty_size_mode(global_reserve, mode),
+		pretty_size_mode(global_reserve_used, mode));
 
 exit:
 

@@ -553,8 +565,11 @@ static void _cmd_filesystem_usage_tabular(int mode,
 	/* header */
 	for (i = 0; i < sargs->total_spaces; i++) {
 		const char *description;
-
 		u64 flags = sargs->spaces[i].flags;
+
+		if (flags & BTRFS_SPACE_INFO_GLOBAL_RSV)
+			continue;
+
 		description = group_type_str(flags);
 
 		table_printf(matrix, 1+i, 0, "<%s", description);

@@ -715,8 +730,11 @@ static void _cmd_filesystem_usage_linear(int mode,
 	for (i = 0; i < sargs->total_spaces; i++) {
 		const char *description;
 		const char *r_mode;
-
 		u64 flags = sargs->spaces[i].flags;
+
+		if (flags & BTRFS_SPACE_INFO_GLOBAL_RSV)
+			continue;
+
 		description= group_type_str(flags);
 		r_mode = group_profile_str(flags);
Signature · 1 line
1.9.0
#25 lore raw
In reply to David Sterba

David Sterba posted on Tue, 29 Apr 2014 17:56:47 +0200 as excerpted:

Quoted 2 lines

Changes:
* btrfs filesystem disk_usage - renamed to usage

Hopefully this isn't rehashing an old discussion, but...

Simpler is good, and getting rid of the _ is good, but...

To users familiar with Unix/POSIX/Linux CLI, "usage" (as --usage) is most often seen as a rather less common and generally briefer form of --help, usually with the distinction being that --help may be a screen or more of output, while --usage is much shorter, perhaps a single line.

While I had seen it listed in manpages and --help output often enough, once I actually went looking for real examples I had a hard time finding some, but eventually found some in the grub (grub2) command set.

$ grub-bios-setup --usage
Usage: grub-bios-setup [-afsv?V] [-b FILE] [-c FILE] [-d DIR] [-m FILE]
            [--allow-floppy] [--boot-image=FILE] [--core-image=FILE]
            [--directory=DIR] [--force] [--device-map=FILE] [--no-rs-
codes]
            [--skip-fs-probe] [--verbose] [--help] [--usage] [--version]
            DEVICE

(For brevity, just printing the line-count for the following few example grub-* commands, not the full output.)

$ grub-bios-setup --help | wc -l
30
$ grub-probe --usage | wc -l
3
$ grub-probe --help | wc -l
23
$ grub-editenv --usage | wc -l
2
$ grub-editenv --help | wc -l
23

While --usage does normally appear with the usual long-option double-dash and the proposal for btrfs does not, I'd still consider a simple "btrfs filesystem usage" confusing at best, expecting it to print out a short help/usage for the "btrfs filesystem" command, either due to not understanding the token and thus printing command usage, or understanding it as a request to print command usage, NOT the actual filesystem usage.

OTOH, for CLI veterans anyway, standard old "du" (btrfs fi du) should be as immediately understood as "df".

Given that confusion, I'd suggest either simply making it du, or making it "device_usage" but with "du" a documented alias printing exactly the same output, thus giving folks an easy way to avoid the _.

Signature · 3 lines
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman
#26 lore raw
Quoted 8 lines · Duncan

David Sterba posted on Tue, 29 Apr 2014 17:56:47 +0200 as excerpted:

Quoted 2 lines

Changes:
* btrfs filesystem disk_usage - renamed to usage

Hopefully this isn't rehashing an old discussion, but...

Simpler is good, and getting rid of the _ is good, but...

(...)

Quoted 3 lines

Given that confusion, I'd suggest either simply making it du, or making it "device_usage" but with "du" a documented alias printing exactly the same output, thus giving folks an easy way to avoid the _.

It's also fine to just use disk-usage instead of disk_usage.

Marc

Signature · 4 lines
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/                         | PGP 1024R/763BE901
#27 lore raw
Quoted 8 lines · Marc MERLIN
Quoted 7 lines · Duncan

David Sterba posted on Tue, 29 Apr 2014 17:56:47 +0200 as excerpted:

Quoted 2 lines

Changes:
* btrfs filesystem disk_usage - renamed to usage

Hopefully this isn't rehashing an old discussion, but...

Quoted 7 lines
Quoted 1 line

Simpler is good, and getting rid of the _ is good, but...

(...)

Quoted 3 lines

Given that confusion, I'd suggest either simply making it du, or making it "device_usage" but with "du" a documented alias printing exactly the same output, thus giving folks an easy way to avoid the _.

It's also fine to just use disk-usage instead of disk_usage.

I think the point was to have the same term for both filesystem and device categories, which IMHO makes a lot of sense and is less to remember. Categorical polymorphism: it's a good thing. :)

-h

#28 lore raw

Holger Hoffstätte posted on Tue, 29 Apr 2014 17:33:09 +0000 as excerpted:

Quoted 6 lines
Quoted 6 lines · Duncan
Quoted 5 lines

David Sterba posted on Tue, 29 Apr 2014 17:56:47 +0200 as excerpted:

Quoted 2 lines

Changes:
* btrfs filesystem disk_usage - renamed to usage

Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

[On the CLI, "usage" is often used to print a briefer "help", and thus shouldn't be used for an actual command.]

I think the point was to have the same term for both filesystem and device categories, which IMHO makes a lot of sense and is less to remember. Categorical polymorphism: it's a good thing. :)

Good idea to use the same term.

Bad idea (IMO) to make that term "usage", by itself.

=:^)

Signature · 3 lines
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman
#29 lore raw

[keeping dropped CC although not customary on this list]

Am Mittwoch, 30. April 2014, 00:42:44 schrieb Duncan:

Quoted 13 lines

Holger Hoffstätte posted on Tue, 29 Apr 2014 17:33:09 +0000 as excerpted:

Quoted 11 lines
Quoted 7 lines · Duncan
Quoted 6 lines

David Sterba posted on Tue, 29 Apr 2014 17:56:47 +0200 as excerpted:

Quoted 2 lines

Changes:
* btrfs filesystem disk_usage - renamed to usage

[On the CLI, "usage" is often used to print a briefer "help", and thus shouldn't be used for an actual command.]

I think the point was to have the same term for both filesystem and device categories, which IMHO makes a lot of sense and is less to remember. Categorical polymorphism: it's a good thing. :)

Quoted 5 lines

Good idea to use the same term.

Bad idea (IMO) to make that term "usage", by itself.

=:^)

Hmmm, I disagree. What it basically displays is usage. So the term "usage" is just about right.

And it isn´t an option either.

Additionally I never ever used the option "--usage". I didn´t even know it existed.

But if you have a different equally short sugestion, I am all ears.

Anyway, I´d personally wouldn´t block the patch going in due to this.

Thanks,

Signature · 2 lines
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7
author
#30 lore raw
Quoted 21 lines · Martin Steigerwald

[keeping dropped CC although not customary on this list]

Am Mittwoch, 30. April 2014, 00:42:44 schrieb Duncan:

Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line

* btrfs filesystem disk_usage - renamed to usage

[On the CLI, "usage" is often used to print a briefer "help", and thus shouldn't be used for an actual command.]

I think the point was to have the same term for both filesystem and device categories, which IMHO makes a lot of sense and is less to remember. Categorical polymorphism: it's a good thing. :)

Quoted 5 lines

Good idea to use the same term.

Bad idea (IMO) to make that term "usage", by itself.

=:^)

Hmmm, I disagree. What it basically displays is usage. So the term "usage" is just about right.

Quoted 8 lines

And it isn´t an option either.

Additionally I never ever used the option "--usage". I didn´t even know it existed.

But if you have a different equally short sugestion, I am all ears.

Anyway, I´d personally wouldn´t block the patch going in due to this.

I agree with all your points.

For the reference, the term 'usage' has been agreed in the previous iteration.

[1] http://www.spinics.net/lists/linux-btrfs/msg31698.html
[2] http://www.spinics.net/lists/linux-btrfs/msg31694.html

author
#31 lore raw
In reply to Duncan
Quoted 4 lines · Duncan

To users familiar with Unix/POSIX/Linux CLI, "usage" (as --usage) is most often seen as a rather less common and generally briefer form of --help, usually with the distinction being that --help may be a screen or more of output, while --usage is much shorter, perhaps a single line.

I did a quick check of ~20 randomly chosen binaries in /usr/bin and none of them had --usage among options, unlike --help.

Quoted 3 lines

While I had seen it listed in manpages and --help output often enough, once I actually went looking for real examples I had a hard time finding some, but eventually found some in the grub (grub2) command set.

We can put usage examples to the man pages, following git here.

Quoted 6 lines

OTOH, for CLI veterans anyway, standard old "du" (btrfs fi du) should be as immediately understood as "df".

Given that confusion, I'd suggest either simply making it du, or making it "device_usage" but with "du" a documented alias printing exactly the same output, thus giving folks an easy way to avoid the _.

Though adding a command alias is trivial, I'd rather not do that right now. Let's see if we can live with 'btrfs fi us'. I worked for me during developing the patch series.

#32 lore raw

David Sterba posted on Wed, 30 Apr 2014 15:01:34 +0200 as excerpted:

Quoted 7 lines · Duncan
Quoted 3 lines

To users familiar with Unix/POSIX/Linux CLI, "usage" (as --usage) is most often seen as a rather less common and generally briefer form of --help

I did a quick check of ~20 randomly chosen binaries in /usr/bin and none of them had --usage among options, unlike --help.

Yes, it wasn't nearly as common as I had the impression it was (I would have guessed about a third coverage, now it looks like a tenth at best), when I went to check, as well. But I did find some.

Quoted 3 lines

Though adding a command alias is trivial, I'd rather not do that right now. Let's see if we can live with 'btrfs fi us'. I worked for me during developing the patch series.

Actually, that suggests a nice, short, non-conflicting alternative, "use". =:^)

"Use" doesn't conflict with other (not-so) common usage like "usage", doesn't have the hard to type implications of a compound device_usage or the like, either, and is nice and short on its own. =:^)

Regardless, I don't feel particularly strongly about it, and as both you and I found, the other use of "usage" is less common than I had at first thought. It might be slightly confusing here, that's all. As others don't seem to have that problem I'm now happy either way and the existing patches use usage, so leaving them as they are is fine by me. =:^)

Signature · 3 lines
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman
#33 lore raw
In reply to David Sterba
Quoted 11 lines · David Sterba

Changes:
* btrfs filesystem disk_usage - renamed to usage
* added a section of overall filesystem usage, that used to be in the 'fi df' output
* btrfs device disk_usage - renamed to usage
* the device size prints both blockdevie size and the size that's occupied by the filesystem
* device ID is printed

 cmds-fi-disk_usage.c | 631 +++++++++++++++++++--------------------------------
 cmds-fi-disk_usage.h |  35 ++-

After renaming the commands btrfs ... disk_usage to btrfs ... usage should the source files also be renamed to cmds-fi-usage.[ch]?

Thanks,
Mike

author
#34 lore raw
Quoted 10 lines · Mike Fleetwood
Quoted 8 lines · David Sterba

Changes:
* btrfs filesystem disk_usage - renamed to usage
* added a section of overall filesystem usage, that used to be in the 'fi df' output
* btrfs device disk_usage - renamed to usage
* the device size prints both blockdevie size and the size that's occupied by the filesystem
* device ID is printed

Quoted 5 lines
Quoted 2 lines
 cmds-fi-disk_usage.c | 631 +++++++++++++++++++--------------------------------
 cmds-fi-disk_usage.h |  35 ++-

After renaming the commands btrfs ... disk_usage to btrfs ... usage should the source files also be renamed to cmds-fi-usage.[ch]?

I should have mentioned that in the cover letter why they've been left as-is - to keep the amount of unrelated cleanup changes low. The files contain helpers for loading/calculating the space information etc and will be moved to a separate file, possibly usable from libbtrfs.

This can be done independently of the 'enhanced df' changes.

Thanks.