[PATCH 0/2] __btrfs_drop_extents() BUG_ON reproducer

[PATCH 0/2] __btrfs_drop_extents() BUG_ON reproducer

[David Disseldorp]

This patch-set provides a reproducer for hitting the 3.14.0-rc1 BUG_ON()
at:
 692 int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
...
 839                 /*
 840                  *  | ---- range to drop ----- |
 841                  *      | -------- extent -------- |
 842                  */
 843                 if (start <= key.offset && end < extent_end) {
 844                         BUG_ON(extent_type == BTRFS_FILE_EXTENT_INLINE);
 845 
 846                         memcpy(&new_key, &key, sizeof(new_key));

The first patch adds a small cloner binary which is used by btrfs/035 to
dispatch BTRFS_IOC_CLONE_RANGE requests.

This workload resembles that of Samba's vfs_btrfs module, when a Windows
client restores a file from a shadow-copy (snapshot) using server-side
copy requests.

Feedback appreciated.

Cheers, David


 src/Makefile        |   2 +-
 src/cloner.c        | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/035     |  76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/035.out |   3 +++
 tests/btrfs/group   |   1 +
 5 files changed, 249 insertions(+), 1 deletion(-)

[PATCH 1/2] btrfs: add small program for clone testing

[David Disseldorp]

The cloner program is capable of cloning files using the BTRFS_IOC_CLONE
and BTRFS_IOC_CLONE_RANGE ioctls.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 src/Makefile |   2 +-
 src/cloner.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 src/cloner.c

diff --git a/src/Makefile b/src/Makefile
index 84c8297..6509f2d 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -18,7 +18,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	locktest unwritten_mmap bulkstat_unlink_test t_stripealign \
 	bulkstat_unlink_test_modified t_dir_offset t_futimens t_immutable \
 	stale_handle pwrite_mmap_blocked t_dir_offset2 seek_sanity_test \
-	seek_copy_test t_readdir_1 t_readdir_2 fsync-tester nsexec
+	seek_copy_test t_readdir_1 t_readdir_2 fsync-tester nsexec cloner
 
 SUBDIRS =
 
diff --git a/src/cloner.c b/src/cloner.c
new file mode 100644
index 0000000..59defbb
--- /dev/null
+++ b/src/cloner.c
@@ -0,0 +1,168 @@
+/*
+ *  Tiny program to perform file (range) clones using raw Btrfs ioctls.
+ *  It should only be needed until btrfs-progs has an xfs_io equivalent.
+ *
+ *  Copyright (C) 2014 SUSE Linux Products GmbH. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+struct btrfs_ioctl_clone_range_args {
+	int64_t src_fd;
+	uint64_t src_offset;
+	uint64_t src_length;
+	uint64_t dest_offset;
+};
+
+#define BTRFS_IOCTL_MAGIC 0x94
+#define BTRFS_IOC_CLONE       _IOW(BTRFS_IOCTL_MAGIC, 9, int)
+#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
+				   struct btrfs_ioctl_clone_range_args)
+
+static void
+usage(char *name, const char *msg)
+{
+	printf("Fatal: %s\n"
+	       "Usage:\n"
+	       "%s [options] <src_file> <dest_file>\n"
+	       "\tA full file clone (reflink) is performed by default, "
+	       "unless any of the following are specified:\n"
+	       "\t-s <offset>:	source file offset (default = 0)\n"
+	       "\t-d <offset>:	destination file offset (default = 0)\n"
+	       "\t-l <length>:	length of clone (default = 0)\n",
+	       msg, name);
+	_exit(1);
+}
+
+static int
+clone_file(int src_fd, int dst_fd)
+{
+	int ret = ioctl(dst_fd, BTRFS_IOC_CLONE, src_fd);
+	if (ret != 0)
+		ret = errno;
+	return ret;
+}
+
+static int
+clone_file_range(int src_fd, int dst_fd, uint64_t src_off, uint64_t dst_off,
+		 uint64_t len)
+{
+	struct btrfs_ioctl_clone_range_args cr_args;
+	int ret;
+
+	memset(&cr_args, 0, sizeof(cr_args));
+	cr_args.src_fd = src_fd;
+	cr_args.src_offset = src_off;
+	cr_args.src_length = len;
+	cr_args.dest_offset = dst_off;
+	ret = ioctl(dst_fd, BTRFS_IOC_CLONE_RANGE, &cr_args);
+	if (ret != 0)
+		ret = errno;
+	return ret;
+}
+
+int
+main(int argc, char **argv)
+{
+	bool full_file = true;
+	uint64_t src_off = 0;
+	uint64_t dst_off = 0;
+	uint64_t len = 0;
+	char *src_file;
+	int src_fd;
+	char *dst_file;
+	int dst_fd;
+	int ret;
+	int opt;
+
+	while ((opt = getopt(argc, argv, "s:d:l:")) != -1) {
+		switch (opt) {
+		case 's':
+			src_off = atoi(optarg);
+			full_file = false;
+			break;
+		case 'd':
+			dst_off = atoi(optarg);
+			full_file = false;
+			break;
+		case 'l':
+			len = atoi(optarg);
+			full_file = false;
+			break;
+		default:
+			usage(argv[0], "invalid argument");
+		}
+	}
+
+	/* should be exactly two args left */
+	if (optind != argc - 2)
+		usage(argv[0], "src_file and dst_file arguments are madatory");
+
+	src_file = (char *)strdup(argv[optind++]);
+	if (src_file == NULL) {
+		ret = ENOMEM;
+		goto err_out;
+	}
+	dst_file = (char *)strdup(argv[optind++]);
+	if (dst_file == NULL) {
+		ret = ENOMEM;
+		goto err_src_free;
+	}
+
+	src_fd = open(src_file, O_RDONLY);
+	if (src_fd == -1) {
+		ret = errno;
+		goto err_dst_free;
+	}
+	dst_fd = open(dst_file, O_CREAT | O_WRONLY, 0644);
+	if (dst_fd == -1) {
+		ret = errno;
+		goto err_src_close;
+	}
+
+	if (full_file) {
+		ret = clone_file(src_fd, dst_fd);
+	} else {
+		ret = clone_file_range(src_fd, dst_fd, src_off, dst_off, len);
+	}
+	if (ret != 0) {
+		printf("clone failed with %d\n", ret);
+		goto err_dst_close;
+	}
+
+	ret = 0;
+err_dst_close:
+	ret |= close(dst_fd);
+err_src_close:
+	ret |= close(src_fd);
+err_dst_free:
+	free(dst_file);
+err_src_free:
+	free(src_file);
+err_out:
+	return ret;
+}
-- 
1.8.4.5

[PATCH 2/2] btrfs/035: add new clone overwrite regression test

[David Disseldorp]

This test uses the newly added cloner binary to dispatch full file and
range specific clone (reflink) requests.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 tests/btrfs/035     | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/035.out |  3 +++
 tests/btrfs/group   |  1 +
 3 files changed, 80 insertions(+)
 create mode 100755 tests/btrfs/035
 create mode 100644 tests/btrfs/035.out

diff --git a/tests/btrfs/035 b/tests/btrfs/035
new file mode 100755
index 0000000..03c2cd3
--- /dev/null
+++ b/tests/btrfs/035
@@ -0,0 +1,76 @@
+#!/bin/bash
+# FS QA Test No. btrfs/035
+#
+# Regression test for overwriting clones
+#
+#-----------------------------------------------------------------------
+# Copyright (C) 2014 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+CLONER_PROG=$here/src/cloner
+
+src_str="aaaaaaaaaa"
+
+echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"
+
+$CLONER_PROG $SCRATCH_MNT/src  $SCRATCH_MNT/src.clone1
+
+src_str="bbbbbbbbbbcccccccccc"
+
+echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"
+
+$CLONER_PROG $SCRATCH_MNT/src $SCRATCH_MNT/src.clone2
+
+snap_src_sz=`ls -lah $SCRATCH_MNT/src.clone1 | awk '{print $5}'`
+echo "attempting ioctl (src.clone1 src)"
+$CLONER_PROG -s 0 -d 0 -l ${snap_src_sz} \
+	$SCRATCH_MNT/src.clone1 $SCRATCH_MNT/src || _fail "ioctl failed"
+
+snap_src_sz=`ls -lah $SCRATCH_MNT/src.clone2 | awk '{print $5}'`
+echo "attempting ioctl (src.clone2 src)"
+$CLONER_PROG -s 0 -d 0 -l ${snap_src_sz} \
+	$SCRATCH_MNT/src.clone2 $SCRATCH_MNT/src || _fail "ioctl failed"
+
+status=0 ; exit
diff --git a/tests/btrfs/035.out b/tests/btrfs/035.out
new file mode 100644
index 0000000..f86cadf
--- /dev/null
+++ b/tests/btrfs/035.out
@@ -0,0 +1,3 @@
+QA output created by 035
+attempting ioctl (src.clone1 src)
+attempting ioctl (src.clone2 src)
diff --git a/tests/btrfs/group b/tests/btrfs/group
index f9f062f..bee57cb 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -37,3 +37,4 @@
 032 auto quick
 033 auto quick
 034 auto quick
+035 auto quick
-- 
1.8.4.5

Re: [PATCH 1/2] btrfs: add small program for clone testing

[Dave Chinner]

On Wed, Feb 05, 2014 at 12:16:48PM +0100, David Disseldorp wrote:
> The cloner program is capable of cloning files using the BTRFS_IOC_CLONE
> and BTRFS_IOC_CLONE_RANGE ioctls.
> 
> Signed-off-by: David Disseldorp <ddiss@suse.de>

Hi Dave - long time since I've seen your head pop up around here ;)

A few comments below.

> +struct btrfs_ioctl_clone_range_args {
> +	int64_t src_fd;
> +	uint64_t src_offset;
> +	uint64_t src_length;
> +	uint64_t dest_offset;
> +};
> +
> +#define BTRFS_IOCTL_MAGIC 0x94
> +#define BTRFS_IOC_CLONE       _IOW(BTRFS_IOCTL_MAGIC, 9, int)
> +#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
> +				   struct btrfs_ioctl_clone_range_args)

Is there some published header file that these belong to? i.e.
somewhere in the include/linux/uapi/ kernel directory? Normally the
way to handle this sort of thing is by autoconf - if the header file
exists, then we include it, otherwise we use the manual definitions.
This just means that if the public api ever changes, we'll pick it
up automatically in future...

> +int
> +main(int argc, char **argv)
> +{
> +	bool full_file = true;
> +	uint64_t src_off = 0;
> +	uint64_t dst_off = 0;
> +	uint64_t len = 0;
> +	char *src_file;
> +	int src_fd;
> +	char *dst_file;
> +	int dst_fd;
> +	int ret;
> +	int opt;
> +
> +	while ((opt = getopt(argc, argv, "s:d:l:")) != -1) {
> +		switch (opt) {
> +		case 's':
> +			src_off = atoi(optarg);

atoi() only returns 32 bit numbers. You probably should use
strtoull() as the offset parameters are 64 bit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] btrfs/035: add new clone overwrite regression test

[Dave Chinner]

On Wed, Feb 05, 2014 at 12:16:49PM +0100, David Disseldorp wrote:
> This test uses the newly added cloner binary to dispatch full file and
> range specific clone (reflink) requests.

A couple of small nits:

> +CLONER_PROG=$here/src/cloner

Need to test that the binary was build and is present.

> +
> +src_str="aaaaaaaaaa"
> +
> +echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"

No need for the "|| _fail ..." in any part of this test. Failures
will be caught in the output and hence cause golden output
mismatches.

Letting the test run even after a failure exercises the filesystem
in interesting ways, so it's worthwhile ignoring failures in the
test and letting the harness pick up the failures through error
messages.

> +$CLONER_PROG $SCRATCH_MNT/src  $SCRATCH_MNT/src.clone1
> +
> +src_str="bbbbbbbbbbcccccccccc"
> +
> +echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"
> +
> +$CLONER_PROG $SCRATCH_MNT/src $SCRATCH_MNT/src.clone2
> +
> +snap_src_sz=`ls -lah $SCRATCH_MNT/src.clone1 | awk '{print $5}'`
> +echo "attempting ioctl (src.clone1 src)"
> +$CLONER_PROG -s 0 -d 0 -l ${snap_src_sz} \
> +	$SCRATCH_MNT/src.clone1 $SCRATCH_MNT/src || _fail "ioctl failed"

And to do that here, you probably need to add perror() output to
the cloner program when it detects an error. i.e. let it give you
the exact error that was detected, rather than lumping them all into
a catchall here...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/2] btrfs: add small program for clone testing

[David Disseldorp]

Hi Dave,

On Thu, 6 Feb 2014 10:09:36 +1100, Dave Chinner wrote:

> On Wed, Feb 05, 2014 at 12:16:48PM +0100, David Disseldorp wrote:
> > The cloner program is capable of cloning files using the BTRFS_IOC_CLONE
> > and BTRFS_IOC_CLONE_RANGE ioctls.
> > 
> > Signed-off-by: David Disseldorp <ddiss@suse.de>
> 
> Hi Dave - long time since I've seen your head pop up around here ;)

Indeed, it's been a while. Thanks for the review :)

> 
> A few comments below.
> 
> > +struct btrfs_ioctl_clone_range_args {
> > +	int64_t src_fd;
> > +	uint64_t src_offset;
> > +	uint64_t src_length;
> > +	uint64_t dest_offset;
> > +};
> > +
> > +#define BTRFS_IOCTL_MAGIC 0x94
> > +#define BTRFS_IOC_CLONE       _IOW(BTRFS_IOCTL_MAGIC, 9, int)
> > +#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
> > +				   struct btrfs_ioctl_clone_range_args)
> 
> Is there some published header file that these belong to? i.e.
> somewhere in the include/linux/uapi/ kernel directory? Normally the
> way to handle this sort of thing is by autoconf - if the header file
> exists, then we include it, otherwise we use the manual definitions.
> This just means that if the public api ever changes, we'll pick it
> up automatically in future...

I'd wanted to avoid the addition of another xfsqa prereq, but I guess
it'll work with the fall-back.
I'll add the autoconf logic to the next round, along with changes
addressing your other remarks.

Cheers, David