| Message ID | 20161209175645.GB16813@birch.djwong.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Dec 09, 2016 at 09:56:45AM -0800, Darrick J. Wong wrote: > Since a zero-length dedupe operation is guaranteed to succeed, use that > to test whether or not this filesystem supports dedupe. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > file_scan.c | 47 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 37 insertions(+), 10 deletions(-) > > diff --git a/file_scan.c b/file_scan.c > index 617f166..a34453e 100644 > --- a/file_scan.c > +++ b/file_scan.c > @@ -45,11 +45,7 @@ > #include "file_scan.h" > #include "dbfile.h" > #include "util.h" > - > -/* This is not in linux/magic.h */ > -#ifndef XFS_SB_MAGIC > -#define XFS_SB_MAGIC 0x58465342 /* 'XFSB' */ > -#endif > +#include "btrfs-ioctl.h" > > static char path[PATH_MAX] = { 0, }; > static char *pathp = path; > @@ -189,6 +185,39 @@ static int walk_dir(const char *name) > return ret; > } > > +struct fake_btrfs_ioctl_same_args { > + struct btrfs_ioctl_same_args args; > + struct btrfs_ioctl_same_extent_info info; > +}; Why does this need a fake structure here? -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 14, 2016 at 02:44:36AM -0800, Christoph Hellwig wrote: > On Fri, Dec 09, 2016 at 09:56:45AM -0800, Darrick J. Wong wrote: > > Since a zero-length dedupe operation is guaranteed to succeed, use that > > to test whether or not this filesystem supports dedupe. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > file_scan.c | 47 +++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 37 insertions(+), 10 deletions(-) > > > > diff --git a/file_scan.c b/file_scan.c > > index 617f166..a34453e 100644 > > --- a/file_scan.c > > +++ b/file_scan.c > > @@ -45,11 +45,7 @@ > > #include "file_scan.h" > > #include "dbfile.h" > > #include "util.h" > > - > > -/* This is not in linux/magic.h */ > > -#ifndef XFS_SB_MAGIC > > -#define XFS_SB_MAGIC 0x58465342 /* 'XFSB' */ > > -#endif > > +#include "btrfs-ioctl.h" > > > > static char path[PATH_MAX] = { 0, }; > > static char *pathp = path; > > @@ -189,6 +185,39 @@ static int walk_dir(const char *name) > > return ret; > > } > > > > +struct fake_btrfs_ioctl_same_args { > > + struct btrfs_ioctl_same_args args; > > + struct btrfs_ioctl_same_extent_info info; > > +}; > > Why does this need a fake structure here? In order to test the ioctl we have to fill out at least one btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific dedupe_range handler that we've verified that the fs is capable of dedupe and that the fs is willing to try to satisfy the request. We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)... --D > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 14, 2016 at 10:38:45AM -0800, Darrick J. Wong wrote: > > > +struct fake_btrfs_ioctl_same_args { > > > + struct btrfs_ioctl_same_args args; > > > + struct btrfs_ioctl_same_extent_info info; > > > +}; > > > > Why does this need a fake structure here? > > In order to test the ioctl we have to fill out at least one > btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific > dedupe_range handler that we've verified that the fs is capable of dedupe and > that the fs is willing to try to satisfy the request. Oh, got it, it's just the fake that tripped me up. > We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)... Either that, or more simply just don't give the structure a name by just declaring it locally on the stack: struct { struct btrfs_ioctl_same_args args; struct btrfs_ioctl_same_extent_info info; } sa = { 0 }; -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 14, 2016 at 11:26:07AM -0800, Christoph Hellwig wrote: > On Wed, Dec 14, 2016 at 10:38:45AM -0800, Darrick J. Wong wrote: > > > > +struct fake_btrfs_ioctl_same_args { > > > > + struct btrfs_ioctl_same_args args; > > > > + struct btrfs_ioctl_same_extent_info info; > > > > +}; > > > > > > Why does this need a fake structure here? > > > > In order to test the ioctl we have to fill out at least one > > btrfs_ioctl_same_extent_info so that we get far enough into the fs-specific > > dedupe_range handler that we've verified that the fs is capable of dedupe and > > that the fs is willing to try to satisfy the request. > > Oh, got it, it's just the fake that tripped me up. > > > We could just malloc sizeof(_same_args) + sizeof(_same_extent_info)... > > Either that, or more simply just don't give the structure a name > by just declaring it locally on the stack: > > struct { > struct btrfs_ioctl_same_args args; > struct btrfs_ioctl_same_extent_info info; > } sa = { 0 }; Fair enough, no need to pollute the namespace. --D > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 15, 2016 at 05:20:14PM -0800, Darrick J. Wong wrote:
> Fair enough, no need to pollute the namespace.
Or confuse poor readers with the 'fake' name :)
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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/file_scan.c b/file_scan.c index 617f166..a34453e 100644 --- a/file_scan.c +++ b/file_scan.c @@ -45,11 +45,7 @@ #include "file_scan.h" #include "dbfile.h" #include "util.h" - -/* This is not in linux/magic.h */ -#ifndef XFS_SB_MAGIC -#define XFS_SB_MAGIC 0x58465342 /* 'XFSB' */ -#endif +#include "btrfs-ioctl.h" static char path[PATH_MAX] = { 0, }; static char *pathp = path; @@ -189,6 +185,39 @@ static int walk_dir(const char *name) return ret; } +struct fake_btrfs_ioctl_same_args { + struct btrfs_ioctl_same_args args; + struct btrfs_ioctl_same_extent_info info; +}; + +/* + * A zero-length dedupe between two files should always succeed, + * so we can use this to test the presence of dedupe functionality. + */ +static bool check_ioctl_works(int fd) +{ + struct fake_btrfs_ioctl_same_args sa = {0}; + struct stat sb; + static int cached = -1; + int ret; + + if (cached >= 0) + return cached != 0; + + ret = fstat(fd, &sb); + if (ret) + return false; + + sa.args.dest_count = 1; + sa.args.length = 0; + sa.info.fd = fd; + sa.info.logical_offset = 0; + errno = 0; + ret = btrfs_extent_same(fd, &sa.args); + cached = !ret && !errno && !sa.info.status; + return cached != 0; +} + static int __add_file(const char *name, struct stat *st, struct filerec **ret_file) { @@ -235,12 +264,10 @@ static int __add_file(const char *name, struct stat *st, goto out; } - if (run_dedupe && - ((fs.f_type != BTRFS_SUPER_MAGIC && - fs.f_type != XFS_SB_MAGIC))) { + if (run_dedupe && !check_ioctl_works(fd)) { close(fd); - fprintf(stderr, "\"%s\": Can only dedupe files on btrfs or xfs " - "(experimental)\n", name); + fprintf(stderr, "\"%s\": dedupe ioctl not supported on this " + "filesystem.\n", name); return ENOSYS; }
Since a zero-length dedupe operation is guaranteed to succeed, use that to test whether or not this filesystem supports dedupe. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- file_scan.c | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html