diff mbox

[RFC,v2] btrfs-progs: Add recursive defrag using -r option

Message ID CAC4gV=40wHT3UcSbHsgei8zqcRwCT7-X0g7Vxzp7A_1nyNkOYw@mail.gmail.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Frank Holton Sept. 17, 2013, 3:30 p.m. UTC
Thanks for that hint to use ftw. I've updated the code to use it and
tried to make sure
that I caught all of the styling errors.

Since the ftw callback doesn't take any additional options I had to add several
global variables to pass the fancy_ioctl and range parameters. Should
I replace all
of the uses of those variables with the globals or just copy into the
globals like I did in
the code below.

It does not attempt to defrag directories anymore in the recursive
mode, however, the
non recursive mode will still attempt to defrag directories. I figured
since that only works
when you run as root that it is acceptable for now.

Thanks again for looking over it.

- Frank

---
 cmds-filesystem.c |  119 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 96 insertions(+), 23 deletions(-)

  int fd;
@@ -349,7 +403,8 @@ static int cmd_defrag(int argc, char **argv)
  u64 len = (u64)-1;
  u32 thresh = 0;
  int i;
- int errors = 0;
+ int recursive = 0;
+ global_errors = 0;
  int ret = 0;
  int verbose = 0;
  int fancy_ioctl = 0;
@@ -359,7 +414,7 @@ static int cmd_defrag(int argc, char **argv)

  optind = 1;
  while(1) {
- int c = getopt(argc, argv, "vc::fs:l:t:");
+ int c = getopt(argc, argv, "vrc::fs:l:t:");
  if (c < 0)
  break;

@@ -389,6 +444,9 @@ static int cmd_defrag(int argc, char **argv)
  thresh = parse_size(optarg);
  fancy_ioctl = 1;
  break;
+ case 'r':
+ recursive = 1;
+ break;
  default:
  usage(cmd_defrag_usage);
  }
@@ -407,47 +465,62 @@ static int cmd_defrag(int argc, char **argv)
  }
  if (flush)
  range.flags |= BTRFS_DEFRAG_RANGE_START_IO;
+
+ memcpy(&global_range, &range, sizeof(range));
+ global_verbose = verbose;
+ global_fancy_ioctl = fancy_ioctl;

  for (i = optind; i < argc; i++) {
- if (verbose)
- printf("%s\n", argv[i]);
  fd = open_file_or_dir(argv[i]);
  if (fd < 0) {
- fprintf(stderr, "failed to open %s\n", argv[i]);
+ fprintf(stderr, "ERROR: failed to open %s\n", argv[i]);
  perror("open:");
- errors++;
+ global_errors++;
  continue;
  }
- if (!fancy_ioctl) {
- ret = ioctl(fd, BTRFS_IOC_DEFRAG, NULL);
- e=errno;
- } else {
- ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, &range);
- if (ret && errno == ENOTTY) {
- fprintf(stderr, "ERROR: defrag range ioctl not "
- "supported in this kernel, please try "
- "without any options.\n");
- errors++;
- close(fd);
- break;
+ if (recursive) {
+ struct stat st;
+ fstat(fd, &st);
+ if (S_ISDIR(st.st_mode)) {
+ ret = ftw(argv[i], defrag_callback, 10);
+ if (ret == ENOTTY)
+ exit(1);
+ /* errors are handled in the callback */
+ ret = 0;
+ } else {
+ if (verbose)
+ printf("%s\n", argv[i]);
+ ret = do_defrag(fd, fancy_ioctl, &range);
+ e = errno;
  }
+ } else {
+ if (verbose)
+ printf("%s\n", argv[i]);
+ ret = do_defrag(fd, fancy_ioctl, &range);
  e = errno;
  }
+ close(fd);
+ if (ret && e == ENOTTY) {
+ fprintf(stderr, "ERROR: defrag range ioctl not "
+ "supported in this kernel, please try "
+ "without any options.\n");
+ global_errors++;
+ break;
+ }
  if (ret) {
  fprintf(stderr, "ERROR: defrag failed on %s - %s\n",
  argv[i], strerror(e));
- errors++;
+ global_errors++;
  }
- close(fd);
  }
  if (verbose)
  printf("%s\n", BTRFS_BUILD_VERSION);
- if (errors) {
- fprintf(stderr, "total %d failures\n", errors);
+ if (global_errors) {
+ fprintf(stderr, "total %d failures\n", global_errors);
  exit(1);
  }

- return errors;
+ return 0;
 }

 static const char * const cmd_resize_usage[] = {

Comments

David Sterba Sept. 23, 2013, 5:35 p.m. UTC | #1
On Tue, Sep 17, 2013 at 11:30:52AM -0400, Frank Holton wrote:
> Thanks for that hint to use ftw. I've updated the code to use it and
> tried to make sure
> that I caught all of the styling errors.

Dunno what caused that, but the whitespace is completely messed up and
squasthed into a single space everywhere.

> Since the ftw callback doesn't take any additional options I had to add several
> global variables to pass the fancy_ioctl and range parameters. Should
> I replace all
> of the uses of those variables with the globals or just copy into the
> globals like I did in
> the code below.

Eww a callback without user data, that's kind of underdesigned. I don't
see a better workaround than the gobals.

> It does not attempt to defrag directories anymore in the recursive
> mode, however, the
> non recursive mode will still attempt to defrag directories. I figured
> since that only works
> when you run as root that it is acceptable for now.

Agreed.

> +static int global_fancy_ioctl;
> +static struct btrfs_ioctl_defrag_range_args global_range;
> +static int global_verbose;
> +static int global_errors;

For now, please prefix all of them as defrag_ so they do not get reused
like generic variables.

> @@ -349,7 +403,8 @@ static int cmd_defrag(int argc, char **argv)
>   u64 len = (u64)-1;
>   u32 thresh = 0;
>   int i;
> - int errors = 0;
> + int recursive = 0;
> + global_errors = 0;

move this out of the declaration block

>   int ret = 0;
>   int verbose = 0;
>   int fancy_ioctl = 0;

The rest looks ok, but I'll take another look when you send the next
version where the indentatino is fixed.

thanks
--
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 mbox

Patch

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index f41a72a..9635066 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -22,6 +22,8 @@ 
 #include <errno.h>
 #include <uuid/uuid.h>
 #include <ctype.h>
+#include <fcntl.h>
+#include <ftw.h>

 #include "kerncompat.h"
 #include "ctree.h"
@@ -333,6 +335,7 @@  static const char * const cmd_defrag_usage[] = {
  "Defragment a file or a directory",
  "",
  "-v             be verbose",
+ "-r             defragment files recursively",
  "-c[zlib,lzo]   compress the file while defragmenting",
  "-f             flush data to disk immediately after defragmenting",
  "-s start       defragment only from byte onward",
@@ -341,6 +344,57 @@  static const char * const cmd_defrag_usage[] = {
  NULL
 };

+static int do_defrag(int fd, int fancy_ioctl,
+ struct btrfs_ioctl_defrag_range_args *range)
+{
+ int ret;
+
+ if (!fancy_ioctl) {
+ ret = ioctl(fd, BTRFS_IOC_DEFRAG, NULL);
+ } else {
+ ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, range);
+ }
+ return ret;
+}
+
+static int global_fancy_ioctl;
+static struct btrfs_ioctl_defrag_range_args global_range;
+static int global_verbose;
+static int global_errors;
+static int defrag_callback(const char *fpath, const struct stat *sb,
int typeflag)
+{
+ int ret = 0;
+ int e = 0;
+ int fd = 0;
+
+ if (typeflag == FTW_F) {
+ if (global_verbose)
+ printf("%s\n", fpath);
+ fd = open(fpath,O_RDWR);
+ e = errno;
+ if (fd < 0)
+ goto error;
+ ret = do_defrag(fd, global_fancy_ioctl, &global_range);
+ e = errno;
+ close(fd);
+ if (ret && e == ENOTTY) {
+ fprintf(stderr, "ERROR: defrag range ioctl not "
+ "supported in this kernel, please try "
+ "without any options.\n");
+ global_errors++;
+ return ENOTTY;
+ }
+ if (ret)
+ goto error;
+ }
+ return 0;
+
+error:
+ fprintf(stderr, "ERROR: defrag failed on %s - %s\n", fpath, strerror(e));
+ global_errors++;
+ return 0;
+}
+
 static int cmd_defrag(int argc, char **argv)
 {