| Message ID | 20130201095947.GA32445@gmail.com (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
On Fri, Feb 01, 2013 at 09:59:49AM +0000, Audrius Butkevicius wrote: > Add '-b' and '--bytes' options to btrfs filesystem df and show, for easier > integration with scripts. This causes all sizes to be displayed in decimal > bytes instead of pretty-printed with human-readable suffices KB, MB, etc. Please, not this way. The right way to do this would be to modify the pretty-print function so that it can output bytes or SI suffixes (kB, MB) or binary suffixes (KiB, MiB). I'm fairly sure I posted a patch for this a couple of years ago. There was some discussion then about the appropriate command-line options to use. The options should also be applicable to fi show, and to anything else which outputs byte counts. Note also that this may well clash with Goffredo's new free-space display (which hasn't yet made it to David's integration tree, but shouldn't be far away). Hugo. > Signed-off-by: Audrius Butkevicius <audrius.butkevicius@elastichosts.com> > --- > cmds-filesystem.c | 93 +++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 68 insertions(+), 25 deletions(-) > > diff --git a/cmds-filesystem.c b/cmds-filesystem.c > index 507239a..9392209 100644 > --- a/cmds-filesystem.c > +++ b/cmds-filesystem.c > @@ -40,7 +40,7 @@ static const char * const filesystem_cmd_group_usage[] = { > }; > > static const char * const cmd_df_usage[] = { > - "btrfs filesystem df <path>", > + "btrfs filesystem df [-b|--bytes] <path>", > "Show space usage information for a mount point", > NULL > }; > @@ -53,11 +53,23 @@ static int cmd_df(int argc, char **argv) > int fd; > int e; > char *path; > + int start = 1; > + int rawbytes = 0; > + > + while (argc > start) { > + if (!strcmp(argv[start], "--bytes") || > + !strcmp(argv[start], "-b")) { > + rawbytes = 1; > + start += 1; > + } else { > + break; > + } > + } > > - if (check_argc_exact(argc, 2)) > + if (check_argc_exact(argc, start + 1)) > usage(cmd_df_usage); > > - path = argv[1]; > + path = argv[start]; > > fd = open_file_or_dir(path); > if (fd < 0) { > @@ -150,10 +162,18 @@ static int cmd_df(int argc, char **argv) > written += 8; > } > > - total_bytes = pretty_sizes(sargs->spaces[i].total_bytes); > - used_bytes = pretty_sizes(sargs->spaces[i].used_bytes); > - printf("%s: total=%s, used=%s\n", description, total_bytes, > - used_bytes); > + if (rawbytes) { > + printf("%s: total=%llu, used=%llu\n", description, > + (unsigned long long)sargs->spaces[i].total_bytes, > + (unsigned long long)sargs->spaces[i].used_bytes); > + } else { > + total_bytes = pretty_sizes(sargs->spaces[i].total_bytes); > + used_bytes = pretty_sizes(sargs->spaces[i].used_bytes); > + printf("%s: total=%s, used=%s\n", description, total_bytes, > + used_bytes); > + free(total_bytes); > + free(bytes_used); > + } > } > close(fd); > free(sargs); > @@ -182,7 +202,7 @@ static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search) > return 0; > } > > -static void print_one_uuid(struct btrfs_fs_devices *fs_devices) > +static void print_one_uuid(struct btrfs_fs_devices *fs_devices, int rawbytes) > { > char uuidbuf[37]; > struct list_head *cur; > @@ -199,25 +219,39 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices) > else > printf("Label: none "); > > - super_bytes_used = pretty_sizes(device->super_bytes_used); > - > total = device->total_devs; > - printf(" uuid: %s\n\tTotal devices %llu FS bytes used %s\n", uuidbuf, > - (unsigned long long)total, super_bytes_used); > > - free(super_bytes_used); > + if (rawbytes) { > + printf(" uuid: %s\n\tTotal devices %llu FS bytes used %llu\n", > + uuidbuf, (unsigned long long)total, > + (unsigned long long)device->super_bytes_used); > + } else { > + super_bytes_used = pretty_sizes(device->super_bytes_used); > + printf(" uuid: %s\n\tTotal devices %llu FS bytes used %s\n", > + uuidbuf, (unsigned long long)total, super_bytes_used); > + free(super_bytes_used); > + } > > list_for_each(cur, &fs_devices->devices) { > char *total_bytes; > char *bytes_used; > device = list_entry(cur, struct btrfs_device, dev_list); > - total_bytes = pretty_sizes(device->total_bytes); > - bytes_used = pretty_sizes(device->bytes_used); > - printf("\tdevid %4llu size %s used %s path %s\n", > - (unsigned long long)device->devid, > - total_bytes, bytes_used, device->name); > - free(total_bytes); > - free(bytes_used); > + if (rawbytes) { > + printf("\tdevid %4llu size %llu used %llu path %s\n", > + (unsigned long long)device->devid, > + (unsigned long long)device->total_bytes, > + (unsigned long long)device->bytes_used, > + device->name); > + } > + else { > + total_bytes = pretty_sizes(device->total_bytes); > + bytes_used = pretty_sizes(device->bytes_used); > + printf("\tdevid %4llu size %s used %s path %s\n", > + (unsigned long long)device->devid, > + total_bytes, bytes_used, device->name); > + free(total_bytes); > + free(bytes_used); > + } > devs_found++; > } > if (devs_found < total) { > @@ -227,7 +261,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices) > } > > static const char * const cmd_show_usage[] = { > - "btrfs filesystem show [--all-devices] [<uuid>|<label>]", > + "btrfs filesystem show [--all-devices] [-b|--bytes] [<uuid>|<label>]", > "Show the structure of a filesystem", > "If no argument is given, structure of all present filesystems is shown.", > NULL > @@ -240,12 +274,21 @@ static int cmd_show(int argc, char **argv) > struct list_head *cur_uuid; > char *search = 0; > int ret; > + int rawbytes = 0; > int checklist = 1; > int searchstart = 1; > > - if( argc > 1 && !strcmp(argv[1],"--all-devices")){ > - checklist = 0; > - searchstart += 1; > + while (argc > searchstart) { > + if (!strcmp(argv[searchstart], "--all-devices")) { > + checklist = 0; > + searchstart += 1; > + } else if (!strcmp(argv[searchstart], "--bytes") || > + !strcmp(argv[searchstart], "-b")) { > + rawbytes = 1; > + searchstart += 1; > + } else { > + break; > + } > } > > if (check_argc_max(argc, searchstart + 1)) > @@ -270,7 +313,7 @@ static int cmd_show(int argc, char **argv) > list); > if (search && uuid_search(fs_devices, search) == 0) > continue; > - print_one_uuid(fs_devices); > + print_one_uuid(fs_devices, rawbytes); > } > printf("%s\n", BTRFS_BUILD_VERSION); > return 0;
On 01/02/2013 10:30, Hugo Mills wrote: > On Fri, Feb 01, 2013 at 09:59:49AM +0000, Audrius Butkevicius wrote: >> Add '-b' and '--bytes' options to btrfs filesystem df and show, for easier >> integration with scripts. This causes all sizes to be displayed in decimal >> bytes instead of pretty-printed with human-readable suffices KB, MB, etc. > Please, not this way. > Hi Hugo, Just wanted to check which approach you'd prefer to see me adopt: 1. Providing an option which is handled near the entry point (prior going to commands), which would toggle a global flag to indicate the format. 2. An option in every function which uses pretty sizes. (Though -B seems to be used by scrub, -b is used by calc-size and mkfs utils, -u is used by subvolume list and so on, meaning the option might have to be different for different commands) 3. An environment variable BTRFS_UNITS, which when set to b[ytes], changes the behaviour of pretty printing. Avoids having to touch the multiple sets of ad-hoc option parsing code, but is perhaps a slightly non-standard interface. Thanks, Audrius. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20 February 2013 13:05, Audrius Butkevicius <audrius.butkevicius@elastichosts.com> wrote: > On 01/02/2013 10:30, Hugo Mills wrote: >> >> On Fri, Feb 01, 2013 at 09:59:49AM +0000, Audrius Butkevicius wrote: >>> >>> Add '-b' and '--bytes' options to btrfs filesystem df and show, for >>> easier >>> integration with scripts. This causes all sizes to be displayed in >>> decimal >>> bytes instead of pretty-printed with human-readable suffices KB, MB, etc. >> >> Please, not this way. >> > > Hi Hugo, > > Just wanted to check which approach you'd prefer to see me adopt: > > 1. Providing an option which is handled near the entry point (prior going to > commands), which would toggle a global flag to indicate the format. > > 2. An option in every function which uses pretty sizes. (Though -B seems to > be used by scrub, -b is used by calc-size and mkfs utils, -u is used by > subvolume list and so on, meaning the option might have to be different for > different commands) > > 3. An environment variable BTRFS_UNITS, which when set to b[ytes], changes > the behaviour of pretty printing. Avoids having to touch the multiple sets > of ad-hoc option parsing code, but is perhaps a slightly non-standard > interface. > > Thanks, > Audrius. Just for reference du and df use the following options: -b, --bytes (du only) -h, --human-readable -k (equivalent to --block-size=1K) --block-size=n[SUFFIX] (suffixes KB, MB, GB, ... mean 1000, 1000^2, 1000^3 ... and K, M, G, ... mean 1024, 1024^2, 1024^3, ...) Mike -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 20, 2013 at 02:15:52PM +0000, Mike Fleetwood wrote: > On 20 February 2013 13:05, Audrius Butkevicius > <audrius.butkevicius@elastichosts.com> wrote: > > On 01/02/2013 10:30, Hugo Mills wrote: > >> > >> On Fri, Feb 01, 2013 at 09:59:49AM +0000, Audrius Butkevicius wrote: > >>> > >>> Add '-b' and '--bytes' options to btrfs filesystem df and show, for > >>> easier > >>> integration with scripts. This causes all sizes to be displayed in > >>> decimal > >>> bytes instead of pretty-printed with human-readable suffices KB, MB, etc. > >> > >> Please, not this way. > >> > > > > Hi Hugo, > > > > Just wanted to check which approach you'd prefer to see me adopt: > > > > 1. Providing an option which is handled near the entry point (prior going to > > commands), which would toggle a global flag to indicate the format. > > > > 2. An option in every function which uses pretty sizes. (Though -B seems to > > be used by scrub, -b is used by calc-size and mkfs utils, -u is used by > > subvolume list and so on, meaning the option might have to be different for > > different commands) Option 1 has an icky global (but one that's pretty much read-only, so it's not all that bad). Option 2 pushes a piece of information through a whole load of functions which really don't have to know about it themselves. Either way, it's not particularly elegant, but either way would work. Unless someone else (Chris? Dave?) has any particular feelings either way, I'd probably go for option 1. > > 3. An environment variable BTRFS_UNITS, which when set to b[ytes], changes > > the behaviour of pretty printing. Avoids having to touch the multiple sets > > of ad-hoc option parsing code, but is perhaps a slightly non-standard > > interface. Yuck. Don't do that. > Just for reference du and df use the following options: > -b, --bytes (du only) > -h, --human-readable and -H, --si likewise, but use powers of 1000 not 1024 > -k (equivalent to --block-size=1K) > --block-size=n[SUFFIX] (suffixes KB, MB, GB, ... mean 1000, > 1000^2, 1000^3 ... and K, M, G, ... mean 1024, 1024^2, 1024^3, ...) ls has --block-size, -k, -h, --human-readable and --si, but not -H (which overlaps with another option) or -b. I'm not too fussed about -k being there. -h and -H and their long forms are the ones I'd definitely want to see. --block-size is useful to give consistent numbers. I'd _like_ to see "bytes" as the default option (which also means that we don't need to use up -b), but that changes the behaviour of the tool (for anyone who was parsing the output of it in scripts), so we probably shouldn't do that. OTOH, we're going to be changing the output of the tool anyway with this change, so maybe it's worth doing it all at once... Hugo.
On Wed, Feb 20, 2013 at 02:54:09PM +0000, Hugo Mills wrote: > Option 1 has an icky global (but one that's pretty much read-only, > so it's not all that bad). Option 2 pushes a piece of information > through a whole load of functions which really don't have to know > about it themselves. > > Either way, it's not particularly elegant, but either way would > work. Unless someone else (Chris? Dave?) has any particular feelings > either way, I'd probably go for option 1. I prefer option 1 as well. > I'd _like_ to see "bytes" as the default option (which also means > that we don't need to use up -b), but that changes the behaviour of > the tool (for anyone who was parsing the output of it in scripts), so > we probably shouldn't do that. OTOH, we're going to be changing the > output of the tool anyway with this change, so maybe it's worth doing > it all at once... For brief overview of the filesytem, the values with suffixes are much easier to grasp. This is what I use in scripts: # perl sub tobytes($$) { my ($a,$suff)=@_; $a*=1024*1024*1024*1024 if($suff eq "TiB"); $a*=1024*1024*1024 if($suff eq "GiB"); $a*=1024*1024 if($suff eq "MiB"); $a*=1024 if($suff eq "KiB"); return $a; } so it'll use raw value in case of an unrecognized suffix (forget for now that it does not recognize the 1000x variants). david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 507239a..9392209 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -40,7 +40,7 @@ static const char * const filesystem_cmd_group_usage[] = { }; static const char * const cmd_df_usage[] = { - "btrfs filesystem df <path>", + "btrfs filesystem df [-b|--bytes] <path>", "Show space usage information for a mount point", NULL }; @@ -53,11 +53,23 @@ static int cmd_df(int argc, char **argv) int fd; int e; char *path; + int start = 1; + int rawbytes = 0; + + while (argc > start) { + if (!strcmp(argv[start], "--bytes") || + !strcmp(argv[start], "-b")) { + rawbytes = 1; + start += 1; + } else { + break; + } + } - if (check_argc_exact(argc, 2)) + if (check_argc_exact(argc, start + 1)) usage(cmd_df_usage); - path = argv[1]; + path = argv[start]; fd = open_file_or_dir(path); if (fd < 0) { @@ -150,10 +162,18 @@ static int cmd_df(int argc, char **argv) written += 8; } - total_bytes = pretty_sizes(sargs->spaces[i].total_bytes); - used_bytes = pretty_sizes(sargs->spaces[i].used_bytes); - printf("%s: total=%s, used=%s\n", description, total_bytes, - used_bytes); + if (rawbytes) { + printf("%s: total=%llu, used=%llu\n", description, + (unsigned long long)sargs->spaces[i].total_bytes, + (unsigned long long)sargs->spaces[i].used_bytes); + } else { + total_bytes = pretty_sizes(sargs->spaces[i].total_bytes); + used_bytes = pretty_sizes(sargs->spaces[i].used_bytes); + printf("%s: total=%s, used=%s\n", description, total_bytes, + used_bytes); + free(total_bytes); + free(bytes_used); + } } close(fd); free(sargs); @@ -182,7 +202,7 @@ static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search) return 0; } -static void print_one_uuid(struct btrfs_fs_devices *fs_devices) +static void print_one_uuid(struct btrfs_fs_devices *fs_devices, int rawbytes) { char uuidbuf[37]; struct list_head *cur; @@ -199,25 +219,39 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices) else printf("Label: none "); - super_bytes_used = pretty_sizes(device->super_bytes_used); - total = device->total_devs; - printf(" uuid: %s\n\tTotal devices %llu FS bytes used %s\n", uuidbuf, - (unsigned long long)total, super_bytes_used); - free(super_bytes_used); + if (rawbytes) { + printf(" uuid: %s\n\tTotal devices %llu FS bytes used %llu\n", + uuidbuf, (unsigned long long)total, + (unsigned long long)device->super_bytes_used); + } else { + super_bytes_used = pretty_sizes(device->super_bytes_used); + printf(" uuid: %s\n\tTotal devices %llu FS bytes used %s\n", + uuidbuf, (unsigned long long)total, super_bytes_used); + free(super_bytes_used); + } list_for_each(cur, &fs_devices->devices) { char *total_bytes; char *bytes_used; device = list_entry(cur, struct btrfs_device, dev_list); - total_bytes = pretty_sizes(device->total_bytes); - bytes_used = pretty_sizes(device->bytes_used); - printf("\tdevid %4llu size %s used %s path %s\n", - (unsigned long long)device->devid, - total_bytes, bytes_used, device->name); - free(total_bytes); - free(bytes_used); + if (rawbytes) { + printf("\tdevid %4llu size %llu used %llu path %s\n", + (unsigned long long)device->devid, + (unsigned long long)device->total_bytes, + (unsigned long long)device->bytes_used, + device->name); + } + else { + total_bytes = pretty_sizes(device->total_bytes); + bytes_used = pretty_sizes(device->bytes_used); + printf("\tdevid %4llu size %s used %s path %s\n", + (unsigned long long)device->devid, + total_bytes, bytes_used, device->name); + free(total_bytes); + free(bytes_used); + } devs_found++; } if (devs_found < total) { @@ -227,7 +261,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices) } static const char * const cmd_show_usage[] = { - "btrfs filesystem show [--all-devices] [<uuid>|<label>]", + "btrfs filesystem show [--all-devices] [-b|--bytes] [<uuid>|<label>]", "Show the structure of a filesystem", "If no argument is given, structure of all present filesystems is shown.", NULL @@ -240,12 +274,21 @@ static int cmd_show(int argc, char **argv) struct list_head *cur_uuid; char *search = 0; int ret; + int rawbytes = 0; int checklist = 1; int searchstart = 1; - if( argc > 1 && !strcmp(argv[1],"--all-devices")){ - checklist = 0; - searchstart += 1; + while (argc > searchstart) { + if (!strcmp(argv[searchstart], "--all-devices")) { + checklist = 0; + searchstart += 1; + } else if (!strcmp(argv[searchstart], "--bytes") || + !strcmp(argv[searchstart], "-b")) { + rawbytes = 1; + searchstart += 1; + } else { + break; + } } if (check_argc_max(argc, searchstart + 1)) @@ -270,7 +313,7 @@ static int cmd_show(int argc, char **argv) list); if (search && uuid_search(fs_devices, search) == 0) continue; - print_one_uuid(fs_devices); + print_one_uuid(fs_devices, rawbytes); } printf("%s\n", BTRFS_BUILD_VERSION); return 0;
Add '-b' and '--bytes' options to btrfs filesystem df and show, for easier integration with scripts. This causes all sizes to be displayed in decimal bytes instead of pretty-printed with human-readable suffices KB, MB, etc. Signed-off-by: Audrius Butkevicius <audrius.butkevicius@elastichosts.com> --- cmds-filesystem.c | 93 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 25 deletions(-)