* [PATCH] duperemove: test presence of dedupe ioctl
@ 2016-12-09 17:56 Darrick J. Wong
2016-12-14 10:44 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-09 17:56 UTC (permalink / raw)
To: mfasheh; +Cc: xfs, linux-btrfs
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;
+};
+
+/*
+ * 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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] duperemove: test presence of dedupe ioctl
2016-12-09 17:56 [PATCH] duperemove: test presence of dedupe ioctl Darrick J. Wong
@ 2016-12-14 10:44 ` Christoph Hellwig
2016-12-14 18:38 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-14 10:44 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: mfasheh, xfs, linux-btrfs
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] duperemove: test presence of dedupe ioctl
2016-12-14 10:44 ` Christoph Hellwig
@ 2016-12-14 18:38 ` Darrick J. Wong
2016-12-14 19:26 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-14 18:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: mfasheh, xfs, linux-btrfs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] duperemove: test presence of dedupe ioctl
2016-12-14 18:38 ` Darrick J. Wong
@ 2016-12-14 19:26 ` Christoph Hellwig
2016-12-16 1:20 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-14 19:26 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, mfasheh, xfs, linux-btrfs
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 };
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] duperemove: test presence of dedupe ioctl
2016-12-14 19:26 ` Christoph Hellwig
@ 2016-12-16 1:20 ` Darrick J. Wong
2016-12-16 7:53 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2016-12-16 1:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: mfasheh, xfs, linux-btrfs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] duperemove: test presence of dedupe ioctl
2016-12-16 1:20 ` Darrick J. Wong
@ 2016-12-16 7:53 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2016-12-16 7:53 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, mfasheh, xfs, linux-btrfs
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 :)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-16 7:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 17:56 [PATCH] duperemove: test presence of dedupe ioctl Darrick J. Wong
2016-12-14 10:44 ` Christoph Hellwig
2016-12-14 18:38 ` Darrick J. Wong
2016-12-14 19:26 ` Christoph Hellwig
2016-12-16 1:20 ` Darrick J. Wong
2016-12-16 7:53 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.