All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-02 23:52 ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-02 23:52 UTC (permalink / raw)
  To: david, eguan; +Cc: linux-btrfs, fstests, xfs, Christoph Hellwig

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see the patch
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 src/Makefile                          |    3 -
 src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 +++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 5 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..5aef79c
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,186 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_SZ		1024
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle,
+	bool			root_space)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	if (root_space)
+		flags |= ATTR_ROOT;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+/* Iterate a range of inodes. */
+void
+find_inode(
+	struct xfs_handle	*fshandle,
+	int			fd,
+	ino_t			ino)
+{
+	struct xfs_fsop_bulkreq	bulkreq;
+	struct xfs_bstat	*bstatbuf;
+	struct xfs_bstat	*p;
+	struct xfs_bstat	*endp;
+	struct xfs_handle	handle;
+	__u64			first_ino = ino & ~63;
+	__s32			buflenout = 0;
+	int			error;
+
+	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
+	if (!bstatbuf) {
+		perror("bulkstat malloc");
+		return;
+	}
+
+	bulkreq.lastip = (__u64 *)&first_ino;
+	bulkreq.icount  = BSTATBUF_SZ;
+	bulkreq.ubuffer = (void *)bstatbuf;
+	bulkreq.ocount  = &buflenout;
+
+	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
+			sizeof(handle.ha_fid.fid_len);
+	handle.ha_fid.fid_pad = 0;
+	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
+		if (buflenout == 0)
+			break;
+		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
+			if (p->bs_ino > ino) {
+				fprintf(stderr,
+					"Expected ino %llu, got %llu.\n",
+					(unsigned long long)ino, p->bs_ino);
+				goto out;
+			}
+
+			handle.ha_fid.fid_gen = p->bs_gen;
+			handle.ha_fid.fid_ino = p->bs_ino;
+
+			read_handle_xattrs(&handle, false);
+			read_handle_xattrs(&handle, true);
+			goto out;
+		}
+	}
+
+	if (error)
+		perror("bulkstat");
+out:
+	free(bstatbuf);
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct stat		sb;
+	int			fd;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("opening file");
+		return 2;
+	}
+
+	error = fstat(fd, &sb);
+	if (error) {
+		perror("fstat file");
+		return 2;
+	}
+
+	find_inode(fshandle, fd, sb.st_ino);
+
+	close(fd);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..ae12e74 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-02 23:52 ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-02 23:52 UTC (permalink / raw)
  To: david, eguan; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see the patch
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 src/Makefile                          |    3 -
 src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 +++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 5 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..5aef79c
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,186 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_SZ		1024
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle,
+	bool			root_space)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	if (root_space)
+		flags |= ATTR_ROOT;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+/* Iterate a range of inodes. */
+void
+find_inode(
+	struct xfs_handle	*fshandle,
+	int			fd,
+	ino_t			ino)
+{
+	struct xfs_fsop_bulkreq	bulkreq;
+	struct xfs_bstat	*bstatbuf;
+	struct xfs_bstat	*p;
+	struct xfs_bstat	*endp;
+	struct xfs_handle	handle;
+	__u64			first_ino = ino & ~63;
+	__s32			buflenout = 0;
+	int			error;
+
+	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
+	if (!bstatbuf) {
+		perror("bulkstat malloc");
+		return;
+	}
+
+	bulkreq.lastip = (__u64 *)&first_ino;
+	bulkreq.icount  = BSTATBUF_SZ;
+	bulkreq.ubuffer = (void *)bstatbuf;
+	bulkreq.ocount  = &buflenout;
+
+	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
+			sizeof(handle.ha_fid.fid_len);
+	handle.ha_fid.fid_pad = 0;
+	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
+		if (buflenout == 0)
+			break;
+		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
+			if (p->bs_ino > ino) {
+				fprintf(stderr,
+					"Expected ino %llu, got %llu.\n",
+					(unsigned long long)ino, p->bs_ino);
+				goto out;
+			}
+
+			handle.ha_fid.fid_gen = p->bs_gen;
+			handle.ha_fid.fid_ino = p->bs_ino;
+
+			read_handle_xattrs(&handle, false);
+			read_handle_xattrs(&handle, true);
+			goto out;
+		}
+	}
+
+	if (error)
+		perror("bulkstat");
+out:
+	free(bstatbuf);
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct stat		sb;
+	int			fd;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("opening file");
+		return 2;
+	}
+
+	error = fstat(fd, &sb);
+	if (error) {
+		perror("fstat file");
+		return 2;
+	}
+
+	find_inode(fshandle, fd, sb.st_ino);
+
+	close(fd);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..ae12e74 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] xfs: test attr_list_by_handle cursor iteration
  2016-08-02 23:52 ` Darrick J. Wong
@ 2016-08-03  4:47   ` Eryu Guan
  -1 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-03  4:47 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: david, linux-btrfs, fstests, xfs, Christoph Hellwig

On Tue, Aug 02, 2016 at 04:52:51PM -0700, Darrick J. Wong wrote:
> Apparently the XFS attr_list_by_handle ioctl has never actually copied
> the cursor contents back to user space, which means that iteration has
> never worked.  Add a test case for this and see the patch
> "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  src/Makefile                          |    3 -
>  src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++

New test program needs an entry in .gitignore

>  tests/xfs/700                         |   64 +++++++++++
>  tests/xfs/700.out                     |    5 +
>  tests/xfs/group                       |    1 
>  5 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 src/attr-list-by-handle-cursor-test.c
>  create mode 100755 tests/xfs/700
>  create mode 100644 tests/xfs/700.out
> 
> diff --git a/src/Makefile b/src/Makefile
> index 1bf318b..ae06d50 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	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 cloner \
> -	renameat2 t_getcwd e4compact test-nextquota punch-alternating
> +	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
> +	attr-list-by-handle-cursor-test
>  
>  SUBDIRS =
>  
> diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
> new file mode 100644
> index 0000000..5aef79c
> --- /dev/null
> +++ b/src/attr-list-by-handle-cursor-test.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright (C) 2016 Oracle.  All Rights Reserved.
> + *
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * 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 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.
> + */
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <attr/attributes.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <asm/types.h>
> +#include <xfs/xfs.h>
> +#include <xfs/handle.h>
> +
> +#define ATTRBUFSZ		1024
> +#define BSTATBUF_SZ		1024
> +
> +/* Read all the extended attributes of a file handle. */
> +void
> +read_handle_xattrs(
> +	struct xfs_handle	*handle,
> +	bool			root_space)

On RHEL6, stdbool.h is needed for bool. Or simply change it to int?

    [CC]    attr-list-by-handle-cursor-test
attr-list-by-handle-cursor-test.c:38: error: expected declaration specifiers or '...' before 'bool'
attr-list-by-handle-cursor-test.c: In function 'read_handle_xattrs':
attr-list-by-handle-cursor-test.c:49: error: 'root_space' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:49: error: (Each undeclared identifier is reported only once
attr-list-by-handle-cursor-test.c:49: error: for each function it appears in.)
attr-list-by-handle-cursor-test.c: In function 'find_inode':
attr-list-by-handle-cursor-test.c:135: error: 'false' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:135: error: too many arguments to function 'read_handle_xattrs'
attr-list-by-handle-cursor-test.c:136: error: 'true' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:136: error: too many arguments to function 'read_handle_xattrs'
gmake[2]: *** [attr-list-by-handle-cursor-test] Error 1
gmake[1]: *** [src] Error 2
make: *** [default] Error 2

> +{
> +	struct attrlist_cursor	cur;
> +	char			attrbuf[ATTRBUFSZ];
> +	char			*firstname = NULL;
> +	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
> +	struct attrlist_ent	*ent;
> +	int			i;
> +	int			flags = 0;
> +	int			error;
> +
> +	if (root_space)
> +		flags |= ATTR_ROOT;
> +
> +	memset(&cur, 0, sizeof(cur));
> +	while ((error = attr_list_by_handle(handle, sizeof(*handle),
> +					    attrbuf, ATTRBUFSZ, flags,
> +					    &cur)) == 0) {
> +		for (i = 0; i < attrlist->al_count; i++) {

I applied the patch "xfs: in _attrlist_by_handle, copy the cursor back
to userspace" on top of 4.7-rc7 kernel and did some tests, in some cases
test failed because nothing was printed by the test program, e.g.

# diff -u tests/xfs/700.out /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad
--- tests/xfs/700.out   2016-08-03 11:32:02.502000000 +0800
+++ /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad        2016-08-03 11:34:57.018000000 +0800
@@ -2,4 +2,3 @@
 Format and mount
 Stuff file with xattrs
 Run test program
-Test passes.

Debug code shows that this is because attrlist->al_count is 0 in such
cases, so it doesn't go into the for loop.

Failed test configs are:

2k block size XFS with CRC enabled
1k block size XFS with CRC enabled
512 block size XFS

> +			ent = ATTR_ENTRY(attrlist, i);
> +
> +			if (i != 0)
> +				continue;
> +
> +			if (firstname == NULL) {
> +				firstname = malloc(ent->a_valuelen);
> +				memcpy(firstname, ent->a_name, ent->a_valuelen);
> +			} else {
> +				if (memcmp(firstname, ent->a_name,
> +					   ent->a_valuelen) == 0)
> +					fprintf(stderr,
> +						"Saw duplicate xattr \"%s\", buggy XFS?\n",
> +						ent->a_name);
> +				else
> +					fprintf(stderr,
> +						"Test passes.\n");
> +				goto out;
> +			}
> +		}
> +
> +		if (!attrlist->al_more)
> +			break;
> +	}
> +
> +out:
> +	if (firstname)
> +		free(firstname);
> +	if (error)
> +		perror("attr_list_by_handle");
> +	return;
> +}
> +
> +/* Iterate a range of inodes. */
> +void
> +find_inode(
> +	struct xfs_handle	*fshandle,
> +	int			fd,
> +	ino_t			ino)
> +{
> +	struct xfs_fsop_bulkreq	bulkreq;
> +	struct xfs_bstat	*bstatbuf;
> +	struct xfs_bstat	*p;
> +	struct xfs_bstat	*endp;
> +	struct xfs_handle	handle;
> +	__u64			first_ino = ino & ~63;
> +	__s32			buflenout = 0;
> +	int			error;
> +
> +	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
> +	if (!bstatbuf) {
> +		perror("bulkstat malloc");
> +		return;
> +	}
> +
> +	bulkreq.lastip = (__u64 *)&first_ino;
> +	bulkreq.icount  = BSTATBUF_SZ;
> +	bulkreq.ubuffer = (void *)bstatbuf;
> +	bulkreq.ocount  = &buflenout;
> +
> +	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
> +	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
> +			sizeof(handle.ha_fid.fid_len);
> +	handle.ha_fid.fid_pad = 0;
> +	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
> +		if (buflenout == 0)
> +			break;
> +		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
> +			if (p->bs_ino > ino) {
> +				fprintf(stderr,
> +					"Expected ino %llu, got %llu.\n",
> +					(unsigned long long)ino, p->bs_ino);
> +				goto out;
> +			}
> +
> +			handle.ha_fid.fid_gen = p->bs_gen;
> +			handle.ha_fid.fid_ino = p->bs_ino;
> +
> +			read_handle_xattrs(&handle, false);
> +			read_handle_xattrs(&handle, true);
> +			goto out;
> +		}
> +	}
> +
> +	if (error)
> +		perror("bulkstat");
> +out:
> +	free(bstatbuf);
> +	return;
> +}
> +
> +int main(
> +	int			argc,
> +	char			*argv[])
> +{
> +	struct xfs_handle	*fshandle;
> +	size_t			fshandle_len;
> +	struct stat		sb;
> +	int			fd;
> +	int			error;
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "Usage: %s filename\n", argv[0]);
> +		return 1;
> +	}
> +
> +	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
> +	if (error) {
> +		perror("getting fshandle");
> +		return 2;
> +	}
> +
> +	fd = open(argv[1], O_RDONLY);
> +	if (fd < 0) {
> +		perror("opening file");
> +		return 2;
> +	}
> +
> +	error = fstat(fd, &sb);
> +	if (error) {
> +		perror("fstat file");
> +		return 2;
> +	}
> +
> +	find_inode(fshandle, fd, sb.st_ino);
> +
> +	close(fd);
> +	free_handle(fshandle, fshandle_len);
> +	return 0;
> +}
> diff --git a/tests/xfs/700 b/tests/xfs/700
> new file mode 100755
> index 0000000..4a5680d
> --- /dev/null
> +++ b/tests/xfs/700
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# FS QA Test No. 700
> +#
> +# Check that attr_list_by_handle copies the cursor back to userspace.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2016, Oracle and/or its affiliates.  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!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +. ./common/populate
> +
> +# real QA test starts here
> +_supported_os Linux
> +_require_scratch
> +_require_test_program "attr-list-by-handle-cursor-test"
> +
> +rm -f "$seqres.full"
> +
> +echo "Format and mount"
> +_scratch_mkfs > "$seqres.full" 2>&1
> +_scratch_mount
> +
> +echo "Stuff file with xattrs"
> +mkdir $SCRATCH_MNT/foo
> +__populate_create_attr $SCRATCH_MNT/foo 100
> +
> +echo "Run test program"
> +./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/700.out b/tests/xfs/700.out
> new file mode 100644
> index 0000000..493a68a
> --- /dev/null
> +++ b/tests/xfs/700.out
> @@ -0,0 +1,5 @@
> +QA output created by 700
> +Format and mount
> +Stuff file with xattrs
> +Run test program
> +Test passes.
> diff --git a/tests/xfs/group b/tests/xfs/group
> index ff0efa5..ae12e74 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -307,3 +307,4 @@
>  325 auto quick clone
>  326 auto quick clone
>  327 auto quick clone
> +700 auto quick

Also add ioctl group?

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-03  4:47   ` Eryu Guan
  0 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-03  4:47 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

On Tue, Aug 02, 2016 at 04:52:51PM -0700, Darrick J. Wong wrote:
> Apparently the XFS attr_list_by_handle ioctl has never actually copied
> the cursor contents back to user space, which means that iteration has
> never worked.  Add a test case for this and see the patch
> "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  src/Makefile                          |    3 -
>  src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++

New test program needs an entry in .gitignore

>  tests/xfs/700                         |   64 +++++++++++
>  tests/xfs/700.out                     |    5 +
>  tests/xfs/group                       |    1 
>  5 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 src/attr-list-by-handle-cursor-test.c
>  create mode 100755 tests/xfs/700
>  create mode 100644 tests/xfs/700.out
> 
> diff --git a/src/Makefile b/src/Makefile
> index 1bf318b..ae06d50 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	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 cloner \
> -	renameat2 t_getcwd e4compact test-nextquota punch-alternating
> +	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
> +	attr-list-by-handle-cursor-test
>  
>  SUBDIRS =
>  
> diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
> new file mode 100644
> index 0000000..5aef79c
> --- /dev/null
> +++ b/src/attr-list-by-handle-cursor-test.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright (C) 2016 Oracle.  All Rights Reserved.
> + *
> + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> + *
> + * 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 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.
> + */
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <attr/attributes.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <asm/types.h>
> +#include <xfs/xfs.h>
> +#include <xfs/handle.h>
> +
> +#define ATTRBUFSZ		1024
> +#define BSTATBUF_SZ		1024
> +
> +/* Read all the extended attributes of a file handle. */
> +void
> +read_handle_xattrs(
> +	struct xfs_handle	*handle,
> +	bool			root_space)

On RHEL6, stdbool.h is needed for bool. Or simply change it to int?

    [CC]    attr-list-by-handle-cursor-test
attr-list-by-handle-cursor-test.c:38: error: expected declaration specifiers or '...' before 'bool'
attr-list-by-handle-cursor-test.c: In function 'read_handle_xattrs':
attr-list-by-handle-cursor-test.c:49: error: 'root_space' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:49: error: (Each undeclared identifier is reported only once
attr-list-by-handle-cursor-test.c:49: error: for each function it appears in.)
attr-list-by-handle-cursor-test.c: In function 'find_inode':
attr-list-by-handle-cursor-test.c:135: error: 'false' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:135: error: too many arguments to function 'read_handle_xattrs'
attr-list-by-handle-cursor-test.c:136: error: 'true' undeclared (first use in this function)
attr-list-by-handle-cursor-test.c:136: error: too many arguments to function 'read_handle_xattrs'
gmake[2]: *** [attr-list-by-handle-cursor-test] Error 1
gmake[1]: *** [src] Error 2
make: *** [default] Error 2

> +{
> +	struct attrlist_cursor	cur;
> +	char			attrbuf[ATTRBUFSZ];
> +	char			*firstname = NULL;
> +	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
> +	struct attrlist_ent	*ent;
> +	int			i;
> +	int			flags = 0;
> +	int			error;
> +
> +	if (root_space)
> +		flags |= ATTR_ROOT;
> +
> +	memset(&cur, 0, sizeof(cur));
> +	while ((error = attr_list_by_handle(handle, sizeof(*handle),
> +					    attrbuf, ATTRBUFSZ, flags,
> +					    &cur)) == 0) {
> +		for (i = 0; i < attrlist->al_count; i++) {

I applied the patch "xfs: in _attrlist_by_handle, copy the cursor back
to userspace" on top of 4.7-rc7 kernel and did some tests, in some cases
test failed because nothing was printed by the test program, e.g.

# diff -u tests/xfs/700.out /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad
--- tests/xfs/700.out   2016-08-03 11:32:02.502000000 +0800
+++ /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad        2016-08-03 11:34:57.018000000 +0800
@@ -2,4 +2,3 @@
 Format and mount
 Stuff file with xattrs
 Run test program
-Test passes.

Debug code shows that this is because attrlist->al_count is 0 in such
cases, so it doesn't go into the for loop.

Failed test configs are:

2k block size XFS with CRC enabled
1k block size XFS with CRC enabled
512 block size XFS

> +			ent = ATTR_ENTRY(attrlist, i);
> +
> +			if (i != 0)
> +				continue;
> +
> +			if (firstname == NULL) {
> +				firstname = malloc(ent->a_valuelen);
> +				memcpy(firstname, ent->a_name, ent->a_valuelen);
> +			} else {
> +				if (memcmp(firstname, ent->a_name,
> +					   ent->a_valuelen) == 0)
> +					fprintf(stderr,
> +						"Saw duplicate xattr \"%s\", buggy XFS?\n",
> +						ent->a_name);
> +				else
> +					fprintf(stderr,
> +						"Test passes.\n");
> +				goto out;
> +			}
> +		}
> +
> +		if (!attrlist->al_more)
> +			break;
> +	}
> +
> +out:
> +	if (firstname)
> +		free(firstname);
> +	if (error)
> +		perror("attr_list_by_handle");
> +	return;
> +}
> +
> +/* Iterate a range of inodes. */
> +void
> +find_inode(
> +	struct xfs_handle	*fshandle,
> +	int			fd,
> +	ino_t			ino)
> +{
> +	struct xfs_fsop_bulkreq	bulkreq;
> +	struct xfs_bstat	*bstatbuf;
> +	struct xfs_bstat	*p;
> +	struct xfs_bstat	*endp;
> +	struct xfs_handle	handle;
> +	__u64			first_ino = ino & ~63;
> +	__s32			buflenout = 0;
> +	int			error;
> +
> +	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
> +	if (!bstatbuf) {
> +		perror("bulkstat malloc");
> +		return;
> +	}
> +
> +	bulkreq.lastip = (__u64 *)&first_ino;
> +	bulkreq.icount  = BSTATBUF_SZ;
> +	bulkreq.ubuffer = (void *)bstatbuf;
> +	bulkreq.ocount  = &buflenout;
> +
> +	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
> +	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
> +			sizeof(handle.ha_fid.fid_len);
> +	handle.ha_fid.fid_pad = 0;
> +	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
> +		if (buflenout == 0)
> +			break;
> +		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
> +			if (p->bs_ino > ino) {
> +				fprintf(stderr,
> +					"Expected ino %llu, got %llu.\n",
> +					(unsigned long long)ino, p->bs_ino);
> +				goto out;
> +			}
> +
> +			handle.ha_fid.fid_gen = p->bs_gen;
> +			handle.ha_fid.fid_ino = p->bs_ino;
> +
> +			read_handle_xattrs(&handle, false);
> +			read_handle_xattrs(&handle, true);
> +			goto out;
> +		}
> +	}
> +
> +	if (error)
> +		perror("bulkstat");
> +out:
> +	free(bstatbuf);
> +	return;
> +}
> +
> +int main(
> +	int			argc,
> +	char			*argv[])
> +{
> +	struct xfs_handle	*fshandle;
> +	size_t			fshandle_len;
> +	struct stat		sb;
> +	int			fd;
> +	int			error;
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "Usage: %s filename\n", argv[0]);
> +		return 1;
> +	}
> +
> +	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
> +	if (error) {
> +		perror("getting fshandle");
> +		return 2;
> +	}
> +
> +	fd = open(argv[1], O_RDONLY);
> +	if (fd < 0) {
> +		perror("opening file");
> +		return 2;
> +	}
> +
> +	error = fstat(fd, &sb);
> +	if (error) {
> +		perror("fstat file");
> +		return 2;
> +	}
> +
> +	find_inode(fshandle, fd, sb.st_ino);
> +
> +	close(fd);
> +	free_handle(fshandle, fshandle_len);
> +	return 0;
> +}
> diff --git a/tests/xfs/700 b/tests/xfs/700
> new file mode 100755
> index 0000000..4a5680d
> --- /dev/null
> +++ b/tests/xfs/700
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# FS QA Test No. 700
> +#
> +# Check that attr_list_by_handle copies the cursor back to userspace.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2016, Oracle and/or its affiliates.  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!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +. ./common/populate
> +
> +# real QA test starts here
> +_supported_os Linux
> +_require_scratch
> +_require_test_program "attr-list-by-handle-cursor-test"
> +
> +rm -f "$seqres.full"
> +
> +echo "Format and mount"
> +_scratch_mkfs > "$seqres.full" 2>&1
> +_scratch_mount
> +
> +echo "Stuff file with xattrs"
> +mkdir $SCRATCH_MNT/foo
> +__populate_create_attr $SCRATCH_MNT/foo 100
> +
> +echo "Run test program"
> +./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/700.out b/tests/xfs/700.out
> new file mode 100644
> index 0000000..493a68a
> --- /dev/null
> +++ b/tests/xfs/700.out
> @@ -0,0 +1,5 @@
> +QA output created by 700
> +Format and mount
> +Stuff file with xattrs
> +Run test program
> +Test passes.
> diff --git a/tests/xfs/group b/tests/xfs/group
> index ff0efa5..ae12e74 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -307,3 +307,4 @@
>  325 auto quick clone
>  326 auto quick clone
>  327 auto quick clone
> +700 auto quick

Also add ioctl group?

Thanks,
Eryu

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] xfs: test attr_list_by_handle cursor iteration
  2016-08-03  4:47   ` Eryu Guan
@ 2016-08-03 22:42     ` Darrick J. Wong
  -1 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-03 22:42 UTC (permalink / raw)
  To: Eryu Guan; +Cc: david, linux-btrfs, fstests, xfs, Christoph Hellwig

On Wed, Aug 03, 2016 at 12:47:22PM +0800, Eryu Guan wrote:
> On Tue, Aug 02, 2016 at 04:52:51PM -0700, Darrick J. Wong wrote:
> > Apparently the XFS attr_list_by_handle ioctl has never actually copied
> > the cursor contents back to user space, which means that iteration has
> > never worked.  Add a test case for this and see the patch
> > "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  src/Makefile                          |    3 -
> >  src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++
> 
> New test program needs an entry in .gitignore

Ok.

> >  tests/xfs/700                         |   64 +++++++++++
> >  tests/xfs/700.out                     |    5 +
> >  tests/xfs/group                       |    1 
> >  5 files changed, 258 insertions(+), 1 deletion(-)
> >  create mode 100644 src/attr-list-by-handle-cursor-test.c
> >  create mode 100755 tests/xfs/700
> >  create mode 100644 tests/xfs/700.out
> > 
> > diff --git a/src/Makefile b/src/Makefile
> > index 1bf318b..ae06d50 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> >  	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 cloner \
> > -	renameat2 t_getcwd e4compact test-nextquota punch-alternating
> > +	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
> > +	attr-list-by-handle-cursor-test
> >  
> >  SUBDIRS =
> >  
> > diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
> > new file mode 100644
> > index 0000000..5aef79c
> > --- /dev/null
> > +++ b/src/attr-list-by-handle-cursor-test.c
> > @@ -0,0 +1,186 @@
> > +/*
> > + * Copyright (C) 2016 Oracle.  All Rights Reserved.
> > + *
> > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > + *
> > + * 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 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.
> > + */
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <attr/attributes.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <asm/types.h>
> > +#include <xfs/xfs.h>
> > +#include <xfs/handle.h>
> > +
> > +#define ATTRBUFSZ		1024
> > +#define BSTATBUF_SZ		1024
> > +
> > +/* Read all the extended attributes of a file handle. */
> > +void
> > +read_handle_xattrs(
> > +	struct xfs_handle	*handle,
> > +	bool			root_space)
> 
> On RHEL6, stdbool.h is needed for bool. Or simply change it to int?

Yes, that's sufficient.

>     [CC]    attr-list-by-handle-cursor-test
> attr-list-by-handle-cursor-test.c:38: error: expected declaration specifiers or '...' before 'bool'
> attr-list-by-handle-cursor-test.c: In function 'read_handle_xattrs':
> attr-list-by-handle-cursor-test.c:49: error: 'root_space' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:49: error: (Each undeclared identifier is reported only once
> attr-list-by-handle-cursor-test.c:49: error: for each function it appears in.)
> attr-list-by-handle-cursor-test.c: In function 'find_inode':
> attr-list-by-handle-cursor-test.c:135: error: 'false' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:135: error: too many arguments to function 'read_handle_xattrs'
> attr-list-by-handle-cursor-test.c:136: error: 'true' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:136: error: too many arguments to function 'read_handle_xattrs'
> gmake[2]: *** [attr-list-by-handle-cursor-test] Error 1
> gmake[1]: *** [src] Error 2
> make: *** [default] Error 2
> 
> > +{
> > +	struct attrlist_cursor	cur;
> > +	char			attrbuf[ATTRBUFSZ];
> > +	char			*firstname = NULL;
> > +	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
> > +	struct attrlist_ent	*ent;
> > +	int			i;
> > +	int			flags = 0;
> > +	int			error;
> > +
> > +	if (root_space)
> > +		flags |= ATTR_ROOT;
> > +
> > +	memset(&cur, 0, sizeof(cur));
> > +	while ((error = attr_list_by_handle(handle, sizeof(*handle),
> > +					    attrbuf, ATTRBUFSZ, flags,
> > +					    &cur)) == 0) {
> > +		for (i = 0; i < attrlist->al_count; i++) {
> 
> I applied the patch "xfs: in _attrlist_by_handle, copy the cursor back
> to userspace" on top of 4.7-rc7 kernel and did some tests, in some cases
> test failed because nothing was printed by the test program, e.g.
> 
> # diff -u tests/xfs/700.out /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad
> --- tests/xfs/700.out   2016-08-03 11:32:02.502000000 +0800
> +++ /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad        2016-08-03 11:34:57.018000000 +0800
> @@ -2,4 +2,3 @@
>  Format and mount
>  Stuff file with xattrs
>  Run test program
> -Test passes.
> 
> Debug code shows that this is because attrlist->al_count is 0 in such
> cases, so it doesn't go into the for loop.
> 
> Failed test configs are:
> 
> 2k block size XFS with CRC enabled
> 1k block size XFS with CRC enabled
> 512 block size XFS

Hmmm.... apparently I was feeding the wrong inputs to BULKSTAT.  Most
of the time it would (eventually) return the $SCRATCH_MNT/foo inode,
but occasionally it wouldn't, which caused the test not to pass.

I've changed the program to use BULKSTAT_SINGLE since we only care
about the one inode and it seems to work now.  This also makes the
program a lot less convoluted.

> > +			ent = ATTR_ENTRY(attrlist, i);
> > +
> > +			if (i != 0)
> > +				continue;
> > +
> > +			if (firstname == NULL) {
> > +				firstname = malloc(ent->a_valuelen);
> > +				memcpy(firstname, ent->a_name, ent->a_valuelen);
> > +			} else {
> > +				if (memcmp(firstname, ent->a_name,
> > +					   ent->a_valuelen) == 0)
> > +					fprintf(stderr,
> > +						"Saw duplicate xattr \"%s\", buggy XFS?\n",
> > +						ent->a_name);
> > +				else
> > +					fprintf(stderr,
> > +						"Test passes.\n");
> > +				goto out;
> > +			}
> > +		}
> > +
> > +		if (!attrlist->al_more)
> > +			break;
> > +	}
> > +
> > +out:
> > +	if (firstname)
> > +		free(firstname);
> > +	if (error)
> > +		perror("attr_list_by_handle");
> > +	return;
> > +}
> > +
> > +/* Iterate a range of inodes. */
> > +void
> > +find_inode(
> > +	struct xfs_handle	*fshandle,
> > +	int			fd,
> > +	ino_t			ino)
> > +{
> > +	struct xfs_fsop_bulkreq	bulkreq;
> > +	struct xfs_bstat	*bstatbuf;
> > +	struct xfs_bstat	*p;
> > +	struct xfs_bstat	*endp;
> > +	struct xfs_handle	handle;
> > +	__u64			first_ino = ino & ~63;
> > +	__s32			buflenout = 0;
> > +	int			error;
> > +
> > +	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
> > +	if (!bstatbuf) {
> > +		perror("bulkstat malloc");
> > +		return;
> > +	}
> > +
> > +	bulkreq.lastip = (__u64 *)&first_ino;
> > +	bulkreq.icount  = BSTATBUF_SZ;
> > +	bulkreq.ubuffer = (void *)bstatbuf;
> > +	bulkreq.ocount  = &buflenout;
> > +
> > +	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
> > +	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
> > +			sizeof(handle.ha_fid.fid_len);
> > +	handle.ha_fid.fid_pad = 0;
> > +	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
> > +		if (buflenout == 0)
> > +			break;
> > +		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
> > +			if (p->bs_ino > ino) {
> > +				fprintf(stderr,
> > +					"Expected ino %llu, got %llu.\n",
> > +					(unsigned long long)ino, p->bs_ino);
> > +				goto out;
> > +			}
> > +
> > +			handle.ha_fid.fid_gen = p->bs_gen;
> > +			handle.ha_fid.fid_ino = p->bs_ino;
> > +
> > +			read_handle_xattrs(&handle, false);
> > +			read_handle_xattrs(&handle, true);
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	if (error)
> > +		perror("bulkstat");
> > +out:
> > +	free(bstatbuf);
> > +	return;
> > +}
> > +
> > +int main(
> > +	int			argc,
> > +	char			*argv[])
> > +{
> > +	struct xfs_handle	*fshandle;
> > +	size_t			fshandle_len;
> > +	struct stat		sb;
> > +	int			fd;
> > +	int			error;
> > +
> > +	if (argc != 2) {
> > +		fprintf(stderr, "Usage: %s filename\n", argv[0]);
> > +		return 1;
> > +	}
> > +
> > +	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
> > +	if (error) {
> > +		perror("getting fshandle");
> > +		return 2;
> > +	}
> > +
> > +	fd = open(argv[1], O_RDONLY);
> > +	if (fd < 0) {
> > +		perror("opening file");
> > +		return 2;
> > +	}
> > +
> > +	error = fstat(fd, &sb);
> > +	if (error) {
> > +		perror("fstat file");
> > +		return 2;
> > +	}
> > +
> > +	find_inode(fshandle, fd, sb.st_ino);
> > +
> > +	close(fd);
> > +	free_handle(fshandle, fshandle_len);
> > +	return 0;
> > +}
> > diff --git a/tests/xfs/700 b/tests/xfs/700
> > new file mode 100755
> > index 0000000..4a5680d
> > --- /dev/null
> > +++ b/tests/xfs/700
> > @@ -0,0 +1,64 @@
> > +#! /bin/bash
> > +# FS QA Test No. 700
> > +#
> > +# Check that attr_list_by_handle copies the cursor back to userspace.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2016, Oracle and/or its affiliates.  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!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/attr
> > +. ./common/populate
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_require_scratch
> > +_require_test_program "attr-list-by-handle-cursor-test"
> > +
> > +rm -f "$seqres.full"
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs > "$seqres.full" 2>&1
> > +_scratch_mount
> > +
> > +echo "Stuff file with xattrs"
> > +mkdir $SCRATCH_MNT/foo
> > +__populate_create_attr $SCRATCH_MNT/foo 100
> > +
> > +echo "Run test program"
> > +./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/700.out b/tests/xfs/700.out
> > new file mode 100644
> > index 0000000..493a68a
> > --- /dev/null
> > +++ b/tests/xfs/700.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 700
> > +Format and mount
> > +Stuff file with xattrs
> > +Run test program
> > +Test passes.
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index ff0efa5..ae12e74 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -307,3 +307,4 @@
> >  325 auto quick clone
> >  326 auto quick clone
> >  327 auto quick clone
> > +700 auto quick
> 
> Also add ioctl group?

Ok.  Thank you for the review!

--D

> 
> Thanks,
> Eryu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-03 22:42     ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-03 22:42 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

On Wed, Aug 03, 2016 at 12:47:22PM +0800, Eryu Guan wrote:
> On Tue, Aug 02, 2016 at 04:52:51PM -0700, Darrick J. Wong wrote:
> > Apparently the XFS attr_list_by_handle ioctl has never actually copied
> > the cursor contents back to user space, which means that iteration has
> > never worked.  Add a test case for this and see the patch
> > "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  src/Makefile                          |    3 -
> >  src/attr-list-by-handle-cursor-test.c |  186 +++++++++++++++++++++++++++++++++
> 
> New test program needs an entry in .gitignore

Ok.

> >  tests/xfs/700                         |   64 +++++++++++
> >  tests/xfs/700.out                     |    5 +
> >  tests/xfs/group                       |    1 
> >  5 files changed, 258 insertions(+), 1 deletion(-)
> >  create mode 100644 src/attr-list-by-handle-cursor-test.c
> >  create mode 100755 tests/xfs/700
> >  create mode 100644 tests/xfs/700.out
> > 
> > diff --git a/src/Makefile b/src/Makefile
> > index 1bf318b..ae06d50 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> >  	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 cloner \
> > -	renameat2 t_getcwd e4compact test-nextquota punch-alternating
> > +	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
> > +	attr-list-by-handle-cursor-test
> >  
> >  SUBDIRS =
> >  
> > diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
> > new file mode 100644
> > index 0000000..5aef79c
> > --- /dev/null
> > +++ b/src/attr-list-by-handle-cursor-test.c
> > @@ -0,0 +1,186 @@
> > +/*
> > + * Copyright (C) 2016 Oracle.  All Rights Reserved.
> > + *
> > + * Author: Darrick J. Wong <darrick.wong@oracle.com>
> > + *
> > + * 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 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.
> > + */
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <attr/attributes.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <asm/types.h>
> > +#include <xfs/xfs.h>
> > +#include <xfs/handle.h>
> > +
> > +#define ATTRBUFSZ		1024
> > +#define BSTATBUF_SZ		1024
> > +
> > +/* Read all the extended attributes of a file handle. */
> > +void
> > +read_handle_xattrs(
> > +	struct xfs_handle	*handle,
> > +	bool			root_space)
> 
> On RHEL6, stdbool.h is needed for bool. Or simply change it to int?

Yes, that's sufficient.

>     [CC]    attr-list-by-handle-cursor-test
> attr-list-by-handle-cursor-test.c:38: error: expected declaration specifiers or '...' before 'bool'
> attr-list-by-handle-cursor-test.c: In function 'read_handle_xattrs':
> attr-list-by-handle-cursor-test.c:49: error: 'root_space' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:49: error: (Each undeclared identifier is reported only once
> attr-list-by-handle-cursor-test.c:49: error: for each function it appears in.)
> attr-list-by-handle-cursor-test.c: In function 'find_inode':
> attr-list-by-handle-cursor-test.c:135: error: 'false' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:135: error: too many arguments to function 'read_handle_xattrs'
> attr-list-by-handle-cursor-test.c:136: error: 'true' undeclared (first use in this function)
> attr-list-by-handle-cursor-test.c:136: error: too many arguments to function 'read_handle_xattrs'
> gmake[2]: *** [attr-list-by-handle-cursor-test] Error 1
> gmake[1]: *** [src] Error 2
> make: *** [default] Error 2
> 
> > +{
> > +	struct attrlist_cursor	cur;
> > +	char			attrbuf[ATTRBUFSZ];
> > +	char			*firstname = NULL;
> > +	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
> > +	struct attrlist_ent	*ent;
> > +	int			i;
> > +	int			flags = 0;
> > +	int			error;
> > +
> > +	if (root_space)
> > +		flags |= ATTR_ROOT;
> > +
> > +	memset(&cur, 0, sizeof(cur));
> > +	while ((error = attr_list_by_handle(handle, sizeof(*handle),
> > +					    attrbuf, ATTRBUFSZ, flags,
> > +					    &cur)) == 0) {
> > +		for (i = 0; i < attrlist->al_count; i++) {
> 
> I applied the patch "xfs: in _attrlist_by_handle, copy the cursor back
> to userspace" on top of 4.7-rc7 kernel and did some tests, in some cases
> test failed because nothing was printed by the test program, e.g.
> 
> # diff -u tests/xfs/700.out /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad
> --- tests/xfs/700.out   2016-08-03 11:32:02.502000000 +0800
> +++ /root/workspace/xfstests/results//xfs_4k_crc/xfs/700.out.bad        2016-08-03 11:34:57.018000000 +0800
> @@ -2,4 +2,3 @@
>  Format and mount
>  Stuff file with xattrs
>  Run test program
> -Test passes.
> 
> Debug code shows that this is because attrlist->al_count is 0 in such
> cases, so it doesn't go into the for loop.
> 
> Failed test configs are:
> 
> 2k block size XFS with CRC enabled
> 1k block size XFS with CRC enabled
> 512 block size XFS

Hmmm.... apparently I was feeding the wrong inputs to BULKSTAT.  Most
of the time it would (eventually) return the $SCRATCH_MNT/foo inode,
but occasionally it wouldn't, which caused the test not to pass.

I've changed the program to use BULKSTAT_SINGLE since we only care
about the one inode and it seems to work now.  This also makes the
program a lot less convoluted.

> > +			ent = ATTR_ENTRY(attrlist, i);
> > +
> > +			if (i != 0)
> > +				continue;
> > +
> > +			if (firstname == NULL) {
> > +				firstname = malloc(ent->a_valuelen);
> > +				memcpy(firstname, ent->a_name, ent->a_valuelen);
> > +			} else {
> > +				if (memcmp(firstname, ent->a_name,
> > +					   ent->a_valuelen) == 0)
> > +					fprintf(stderr,
> > +						"Saw duplicate xattr \"%s\", buggy XFS?\n",
> > +						ent->a_name);
> > +				else
> > +					fprintf(stderr,
> > +						"Test passes.\n");
> > +				goto out;
> > +			}
> > +		}
> > +
> > +		if (!attrlist->al_more)
> > +			break;
> > +	}
> > +
> > +out:
> > +	if (firstname)
> > +		free(firstname);
> > +	if (error)
> > +		perror("attr_list_by_handle");
> > +	return;
> > +}
> > +
> > +/* Iterate a range of inodes. */
> > +void
> > +find_inode(
> > +	struct xfs_handle	*fshandle,
> > +	int			fd,
> > +	ino_t			ino)
> > +{
> > +	struct xfs_fsop_bulkreq	bulkreq;
> > +	struct xfs_bstat	*bstatbuf;
> > +	struct xfs_bstat	*p;
> > +	struct xfs_bstat	*endp;
> > +	struct xfs_handle	handle;
> > +	__u64			first_ino = ino & ~63;
> > +	__s32			buflenout = 0;
> > +	int			error;
> > +
> > +	bstatbuf = malloc(BSTATBUF_SZ * sizeof(struct xfs_bstat));
> > +	if (!bstatbuf) {
> > +		perror("bulkstat malloc");
> > +		return;
> > +	}
> > +
> > +	bulkreq.lastip = (__u64 *)&first_ino;
> > +	bulkreq.icount  = BSTATBUF_SZ;
> > +	bulkreq.ubuffer = (void *)bstatbuf;
> > +	bulkreq.ocount  = &buflenout;
> > +
> > +	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
> > +	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
> > +			sizeof(handle.ha_fid.fid_len);
> > +	handle.ha_fid.fid_pad = 0;
> > +	while ((error = xfsctl("", fd, XFS_IOC_FSBULKSTAT, &bulkreq)) == 0) {
> > +		if (buflenout == 0)
> > +			break;
> > +		for (p = bstatbuf, endp = bstatbuf + buflenout; p < endp; p++) {
> > +			if (p->bs_ino > ino) {
> > +				fprintf(stderr,
> > +					"Expected ino %llu, got %llu.\n",
> > +					(unsigned long long)ino, p->bs_ino);
> > +				goto out;
> > +			}
> > +
> > +			handle.ha_fid.fid_gen = p->bs_gen;
> > +			handle.ha_fid.fid_ino = p->bs_ino;
> > +
> > +			read_handle_xattrs(&handle, false);
> > +			read_handle_xattrs(&handle, true);
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	if (error)
> > +		perror("bulkstat");
> > +out:
> > +	free(bstatbuf);
> > +	return;
> > +}
> > +
> > +int main(
> > +	int			argc,
> > +	char			*argv[])
> > +{
> > +	struct xfs_handle	*fshandle;
> > +	size_t			fshandle_len;
> > +	struct stat		sb;
> > +	int			fd;
> > +	int			error;
> > +
> > +	if (argc != 2) {
> > +		fprintf(stderr, "Usage: %s filename\n", argv[0]);
> > +		return 1;
> > +	}
> > +
> > +	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
> > +	if (error) {
> > +		perror("getting fshandle");
> > +		return 2;
> > +	}
> > +
> > +	fd = open(argv[1], O_RDONLY);
> > +	if (fd < 0) {
> > +		perror("opening file");
> > +		return 2;
> > +	}
> > +
> > +	error = fstat(fd, &sb);
> > +	if (error) {
> > +		perror("fstat file");
> > +		return 2;
> > +	}
> > +
> > +	find_inode(fshandle, fd, sb.st_ino);
> > +
> > +	close(fd);
> > +	free_handle(fshandle, fshandle_len);
> > +	return 0;
> > +}
> > diff --git a/tests/xfs/700 b/tests/xfs/700
> > new file mode 100755
> > index 0000000..4a5680d
> > --- /dev/null
> > +++ b/tests/xfs/700
> > @@ -0,0 +1,64 @@
> > +#! /bin/bash
> > +# FS QA Test No. 700
> > +#
> > +# Check that attr_list_by_handle copies the cursor back to userspace.
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2016, Oracle and/or its affiliates.  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!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/attr
> > +. ./common/populate
> > +
> > +# real QA test starts here
> > +_supported_os Linux
> > +_require_scratch
> > +_require_test_program "attr-list-by-handle-cursor-test"
> > +
> > +rm -f "$seqres.full"
> > +
> > +echo "Format and mount"
> > +_scratch_mkfs > "$seqres.full" 2>&1
> > +_scratch_mount
> > +
> > +echo "Stuff file with xattrs"
> > +mkdir $SCRATCH_MNT/foo
> > +__populate_create_attr $SCRATCH_MNT/foo 100
> > +
> > +echo "Run test program"
> > +./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/700.out b/tests/xfs/700.out
> > new file mode 100644
> > index 0000000..493a68a
> > --- /dev/null
> > +++ b/tests/xfs/700.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 700
> > +Format and mount
> > +Stuff file with xattrs
> > +Run test program
> > +Test passes.
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index ff0efa5..ae12e74 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -307,3 +307,4 @@
> >  325 auto quick clone
> >  326 auto quick clone
> >  327 auto quick clone
> > +700 auto quick
> 
> Also add ioctl group?

Ok.  Thank you for the review!

--D

> 
> Thanks,
> Eryu

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2] xfs: test attr_list_by_handle cursor iteration
  2016-08-02 23:52 ` Darrick J. Wong
@ 2016-08-03 22:51   ` Darrick J. Wong
  -1 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-03 22:51 UTC (permalink / raw)
  To: david, eguan; +Cc: linux-btrfs, fstests, xfs, Christoph Hellwig

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 .gitignore                            |    1 
 src/Makefile                          |    3 -
 src/attr-list-by-handle-cursor-test.c |  185 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 +++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 6 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/.gitignore b/.gitignore
index 28bd180..e184a6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@
 /src/alloc
 /src/append_reader
 /src/append_writer
+/src/attr-list-by-handle-cursor-test
 /src/bstat
 /src/bulkstat_unlink_test
 /src/bulkstat_unlink_test_modified
diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..ac6ab05
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,185 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_NR		32
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle,
+	int			root_space)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	if (root_space)
+		flags |= ATTR_ROOT;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+/* Iterate a range of inodes. */
+void
+find_inode(
+	struct xfs_handle	*fshandle,
+	int			fd,
+	ino_t			ino)
+{
+	struct xfs_fsop_bulkreq	bulkreq;
+	struct xfs_bstat	*bstatbuf;
+	struct xfs_handle	handle;
+	__u64			first_ino = ino;
+	__s32			buflenout = 0;
+	int			error;
+
+	bstatbuf = malloc(BSTATBUF_NR * sizeof(struct xfs_bstat));
+	if (!bstatbuf) {
+		perror("bulkstat malloc");
+		return;
+	}
+
+	bulkreq.lastip = (__u64 *)&first_ino;
+	bulkreq.icount  = BSTATBUF_NR;
+	bulkreq.ubuffer = (void *)bstatbuf;
+	bulkreq.ocount  = &buflenout;
+
+	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
+			sizeof(handle.ha_fid.fid_len);
+	handle.ha_fid.fid_pad = 0;
+	error = xfsctl("", fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
+	if (error) {
+		perror("bulkstat");
+		goto out;
+	}
+
+	if (buflenout == 0) {
+		fprintf(stderr, "buflenout = 0??\n");
+		goto out;
+	}
+
+	if (bstatbuf->bs_ino != ino) {
+		fprintf(stderr, "Expected ino %llu, got %llu.\n",
+			(unsigned long long)ino, bstatbuf->bs_ino);
+		goto out;
+	}
+
+	handle.ha_fid.fid_gen = bstatbuf->bs_gen;
+	handle.ha_fid.fid_ino = bstatbuf->bs_ino;
+
+	read_handle_xattrs(&handle, false);
+	read_handle_xattrs(&handle, true);
+
+out:
+	free(bstatbuf);
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct stat		sb;
+	int			fd;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("opening file");
+		return 2;
+	}
+
+	error = fstat(fd, &sb);
+	if (error) {
+		perror("fstat file");
+		return 2;
+	}
+
+	find_inode(fshandle, fd, sb.st_ino);
+
+	close(fd);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..b42153d 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick ioctl

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-03 22:51   ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-03 22:51 UTC (permalink / raw)
  To: david, eguan; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 .gitignore                            |    1 
 src/Makefile                          |    3 -
 src/attr-list-by-handle-cursor-test.c |  185 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 +++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 6 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/.gitignore b/.gitignore
index 28bd180..e184a6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@
 /src/alloc
 /src/append_reader
 /src/append_writer
+/src/attr-list-by-handle-cursor-test
 /src/bstat
 /src/bulkstat_unlink_test
 /src/bulkstat_unlink_test_modified
diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..ac6ab05
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,185 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_NR		32
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle,
+	int			root_space)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	if (root_space)
+		flags |= ATTR_ROOT;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+/* Iterate a range of inodes. */
+void
+find_inode(
+	struct xfs_handle	*fshandle,
+	int			fd,
+	ino_t			ino)
+{
+	struct xfs_fsop_bulkreq	bulkreq;
+	struct xfs_bstat	*bstatbuf;
+	struct xfs_handle	handle;
+	__u64			first_ino = ino;
+	__s32			buflenout = 0;
+	int			error;
+
+	bstatbuf = malloc(BSTATBUF_NR * sizeof(struct xfs_bstat));
+	if (!bstatbuf) {
+		perror("bulkstat malloc");
+		return;
+	}
+
+	bulkreq.lastip = (__u64 *)&first_ino;
+	bulkreq.icount  = BSTATBUF_NR;
+	bulkreq.ubuffer = (void *)bstatbuf;
+	bulkreq.ocount  = &buflenout;
+
+	memcpy(&handle.ha_fsid, fshandle, sizeof(handle.ha_fsid));
+	handle.ha_fid.fid_len = sizeof(xfs_fid_t) -
+			sizeof(handle.ha_fid.fid_len);
+	handle.ha_fid.fid_pad = 0;
+	error = xfsctl("", fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
+	if (error) {
+		perror("bulkstat");
+		goto out;
+	}
+
+	if (buflenout == 0) {
+		fprintf(stderr, "buflenout = 0??\n");
+		goto out;
+	}
+
+	if (bstatbuf->bs_ino != ino) {
+		fprintf(stderr, "Expected ino %llu, got %llu.\n",
+			(unsigned long long)ino, bstatbuf->bs_ino);
+		goto out;
+	}
+
+	handle.ha_fid.fid_gen = bstatbuf->bs_gen;
+	handle.ha_fid.fid_ino = bstatbuf->bs_ino;
+
+	read_handle_xattrs(&handle, false);
+	read_handle_xattrs(&handle, true);
+
+out:
+	free(bstatbuf);
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct stat		sb;
+	int			fd;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0) {
+		perror("opening file");
+		return 2;
+	}
+
+	error = fstat(fd, &sb);
+	if (error) {
+		perror("fstat file");
+		return 2;
+	}
+
+	find_inode(fshandle, fd, sb.st_ino);
+
+	close(fd);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..b42153d 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick ioctl

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
  2016-08-03 22:51   ` Darrick J. Wong
@ 2016-08-04  4:24     ` Eryu Guan
  -1 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-04  4:24 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: david, linux-btrfs, fstests, xfs, Christoph Hellwig

On Wed, Aug 03, 2016 at 03:51:50PM -0700, Darrick J. Wong wrote:
> Apparently the XFS attr_list_by_handle ioctl has never actually copied
> the cursor contents back to user space, which means that iteration has
> never worked.  Add a test case for this and see
> "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> 
> v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  .gitignore                            |    1 
>  src/Makefile                          |    3 -
>  src/attr-list-by-handle-cursor-test.c |  185 +++++++++++++++++++++++++++++++++
>  tests/xfs/700                         |   64 +++++++++++
>  tests/xfs/700.out                     |    5 +
>  tests/xfs/group                       |    1 
>  6 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 src/attr-list-by-handle-cursor-test.c
>  create mode 100755 tests/xfs/700
>  create mode 100644 tests/xfs/700.out
> 

[snip]

> +
> +	read_handle_xattrs(&handle, false);
> +	read_handle_xattrs(&handle, true);

I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
at commit time if there's no other major updates.

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-04  4:24     ` Eryu Guan
  0 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-04  4:24 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

On Wed, Aug 03, 2016 at 03:51:50PM -0700, Darrick J. Wong wrote:
> Apparently the XFS attr_list_by_handle ioctl has never actually copied
> the cursor contents back to user space, which means that iteration has
> never worked.  Add a test case for this and see
> "xfs: in _attrlist_by_handle, copy the cursor back to userspace".
> 
> v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  .gitignore                            |    1 
>  src/Makefile                          |    3 -
>  src/attr-list-by-handle-cursor-test.c |  185 +++++++++++++++++++++++++++++++++
>  tests/xfs/700                         |   64 +++++++++++
>  tests/xfs/700.out                     |    5 +
>  tests/xfs/group                       |    1 
>  6 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644 src/attr-list-by-handle-cursor-test.c
>  create mode 100755 tests/xfs/700
>  create mode 100644 tests/xfs/700.out
> 

[snip]

> +
> +	read_handle_xattrs(&handle, false);
> +	read_handle_xattrs(&handle, true);

I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
at commit time if there's no other major updates.

Thanks,
Eryu

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] xfs: test attr_list_by_handle cursor iteration
  2016-08-02 23:52 ` Darrick J. Wong
@ 2016-08-05  2:03   ` Darrick J. Wong
  -1 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-05  2:03 UTC (permalink / raw)
  To: david, eguan; +Cc: linux-btrfs, fstests, xfs, Christoph Hellwig

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.
v3: Use path_to_handle instead of bulkstat.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 .gitignore                            |    1 
 src/Makefile                          |    3 +
 src/attr-list-by-handle-cursor-test.c |  118 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 ++++++++++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 6 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/.gitignore b/.gitignore
index 28bd180..e184a6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@
 /src/alloc
 /src/append_reader
 /src/append_writer
+/src/attr-list-by-handle-cursor-test
 /src/bstat
 /src/bulkstat_unlink_test
 /src/bulkstat_unlink_test_modified
diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..4269d1e
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_NR		32
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct xfs_handle	*handle;
+	size_t			handle_len;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	error = path_to_handle(argv[1], (void **)&handle, &handle_len);
+	if (error) {
+		perror("getting handle");
+		return 3;
+	}
+
+	read_handle_xattrs(handle);
+
+	free_handle(handle, handle_len);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..b42153d 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick ioctl

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-05  2:03   ` Darrick J. Wong
  0 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2016-08-05  2:03 UTC (permalink / raw)
  To: david, eguan; +Cc: Christoph Hellwig, fstests, linux-btrfs, xfs

Apparently the XFS attr_list_by_handle ioctl has never actually copied
the cursor contents back to user space, which means that iteration has
never worked.  Add a test case for this and see
"xfs: in _attrlist_by_handle, copy the cursor back to userspace".

v2: Use BULKSTAT_SINGLE for less confusion, fix build errors on RHEL6.
v3: Use path_to_handle instead of bulkstat.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 .gitignore                            |    1 
 src/Makefile                          |    3 +
 src/attr-list-by-handle-cursor-test.c |  118 +++++++++++++++++++++++++++++++++
 tests/xfs/700                         |   64 ++++++++++++++++++
 tests/xfs/700.out                     |    5 +
 tests/xfs/group                       |    1 
 6 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 src/attr-list-by-handle-cursor-test.c
 create mode 100755 tests/xfs/700
 create mode 100644 tests/xfs/700.out

diff --git a/.gitignore b/.gitignore
index 28bd180..e184a6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@
 /src/alloc
 /src/append_reader
 /src/append_writer
+/src/attr-list-by-handle-cursor-test
 /src/bstat
 /src/bulkstat_unlink_test
 /src/bulkstat_unlink_test_modified
diff --git a/src/Makefile b/src/Makefile
index 1bf318b..ae06d50 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,8 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	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 cloner \
-	renameat2 t_getcwd e4compact test-nextquota punch-alternating
+	renameat2 t_getcwd e4compact test-nextquota punch-alternating \
+	attr-list-by-handle-cursor-test
 
 SUBDIRS =
 
diff --git a/src/attr-list-by-handle-cursor-test.c b/src/attr-list-by-handle-cursor-test.c
new file mode 100644
index 0000000..4269d1e
--- /dev/null
+++ b/src/attr-list-by-handle-cursor-test.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2016 Oracle.  All Rights Reserved.
+ *
+ * Author: Darrick J. Wong <darrick.wong@oracle.com>
+ *
+ * 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 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.
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <attr/attributes.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stdio.h>
+#include <asm/types.h>
+#include <xfs/xfs.h>
+#include <xfs/handle.h>
+
+#define ATTRBUFSZ		1024
+#define BSTATBUF_NR		32
+
+/* Read all the extended attributes of a file handle. */
+void
+read_handle_xattrs(
+	struct xfs_handle	*handle)
+{
+	struct attrlist_cursor	cur;
+	char			attrbuf[ATTRBUFSZ];
+	char			*firstname = NULL;
+	struct attrlist		*attrlist = (struct attrlist *)attrbuf;
+	struct attrlist_ent	*ent;
+	int			i;
+	int			flags = 0;
+	int			error;
+
+	memset(&cur, 0, sizeof(cur));
+	while ((error = attr_list_by_handle(handle, sizeof(*handle),
+					    attrbuf, ATTRBUFSZ, flags,
+					    &cur)) == 0) {
+		for (i = 0; i < attrlist->al_count; i++) {
+			ent = ATTR_ENTRY(attrlist, i);
+
+			if (i != 0)
+				continue;
+
+			if (firstname == NULL) {
+				firstname = malloc(ent->a_valuelen);
+				memcpy(firstname, ent->a_name, ent->a_valuelen);
+			} else {
+				if (memcmp(firstname, ent->a_name,
+					   ent->a_valuelen) == 0)
+					fprintf(stderr,
+						"Saw duplicate xattr \"%s\", buggy XFS?\n",
+						ent->a_name);
+				else
+					fprintf(stderr,
+						"Test passes.\n");
+				goto out;
+			}
+		}
+
+		if (!attrlist->al_more)
+			break;
+	}
+
+out:
+	if (firstname)
+		free(firstname);
+	if (error)
+		perror("attr_list_by_handle");
+	return;
+}
+
+int main(
+	int			argc,
+	char			*argv[])
+{
+	struct xfs_handle	*fshandle;
+	size_t			fshandle_len;
+	struct xfs_handle	*handle;
+	size_t			handle_len;
+	int			error;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s filename\n", argv[0]);
+		return 1;
+	}
+
+	error = path_to_fshandle(argv[1], (void **)&fshandle, &fshandle_len);
+	if (error) {
+		perror("getting fshandle");
+		return 2;
+	}
+
+	error = path_to_handle(argv[1], (void **)&handle, &handle_len);
+	if (error) {
+		perror("getting handle");
+		return 3;
+	}
+
+	read_handle_xattrs(handle);
+
+	free_handle(handle, handle_len);
+	free_handle(fshandle, fshandle_len);
+	return 0;
+}
diff --git a/tests/xfs/700 b/tests/xfs/700
new file mode 100755
index 0000000..4a5680d
--- /dev/null
+++ b/tests/xfs/700
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test No. 700
+#
+# Check that attr_list_by_handle copies the cursor back to userspace.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016, Oracle and/or its affiliates.  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!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -rf "$tmp".* $TEST_DIR/fsmap $TEST_DIR/testout
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+. ./common/populate
+
+# real QA test starts here
+_supported_os Linux
+_require_scratch
+_require_test_program "attr-list-by-handle-cursor-test"
+
+rm -f "$seqres.full"
+
+echo "Format and mount"
+_scratch_mkfs > "$seqres.full" 2>&1
+_scratch_mount
+
+echo "Stuff file with xattrs"
+mkdir $SCRATCH_MNT/foo
+__populate_create_attr $SCRATCH_MNT/foo 100
+
+echo "Run test program"
+./src/attr-list-by-handle-cursor-test $SCRATCH_MNT/foo
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/700.out b/tests/xfs/700.out
new file mode 100644
index 0000000..493a68a
--- /dev/null
+++ b/tests/xfs/700.out
@@ -0,0 +1,5 @@
+QA output created by 700
+Format and mount
+Stuff file with xattrs
+Run test program
+Test passes.
diff --git a/tests/xfs/group b/tests/xfs/group
index ff0efa5..b42153d 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -307,3 +307,4 @@
 325 auto quick clone
 326 auto quick clone
 327 auto quick clone
+700 auto quick ioctl

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
  2016-08-04  4:24     ` Eryu Guan
@ 2016-08-11 17:48       ` Christoph Hellwig
  -1 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2016-08-11 17:48 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Darrick J. Wong, david, linux-btrfs, fstests, xfs, Christoph Hellwig

On Thu, Aug 04, 2016 at 12:24:48PM +0800, Eryu Guan wrote:
> I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
> at commit time if there's no other major updates.

Simply including <stdbool.h> would fix that on RHEL6, wouldn't it?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-11 17:48       ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2016-08-11 17:48 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Darrick J. Wong, fstests, xfs, Christoph Hellwig, linux-btrfs

On Thu, Aug 04, 2016 at 12:24:48PM +0800, Eryu Guan wrote:
> I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
> at commit time if there's no other major updates.

Simply including <stdbool.h> would fix that on RHEL6, wouldn't it?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
  2016-08-11 17:48       ` Christoph Hellwig
@ 2016-08-12  3:14         ` Eryu Guan
  -1 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-12  3:14 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Darrick J. Wong, david, linux-btrfs, fstests, xfs

On Thu, Aug 11, 2016 at 10:48:55AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 04, 2016 at 12:24:48PM +0800, Eryu Guan wrote:
> > I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
> > at commit time if there's no other major updates.
> 
> Simply including <stdbool.h> would fix that on RHEL6, wouldn't it?

Yes, including <stdbool.h> fixes the build failure on RHEL6, another
option is to replace all bool with int, as mentioned in my first review
email. And Darric chose the "int" way in v2 but only updated the
function definition and forgot to update the callers (still passing
true/false, not 1/0).

Thanks for the review!

Eryu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] xfs: test attr_list_by_handle cursor iteration
@ 2016-08-12  3:14         ` Eryu Guan
  0 siblings, 0 replies; 16+ messages in thread
From: Eryu Guan @ 2016-08-12  3:14 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs, fstests, linux-btrfs, Darrick J. Wong

On Thu, Aug 11, 2016 at 10:48:55AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 04, 2016 at 12:24:48PM +0800, Eryu Guan wrote:
> > I have to replace false/true with 0/1 to build it on RHEL6. I can fix it
> > at commit time if there's no other major updates.
> 
> Simply including <stdbool.h> would fix that on RHEL6, wouldn't it?

Yes, including <stdbool.h> fixes the build failure on RHEL6, another
option is to replace all bool with int, as mentioned in my first review
email. And Darric chose the "int" way in v2 but only updated the
function definition and forgot to update the callers (still passing
true/false, not 1/0).

Thanks for the review!

Eryu

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-08-12  3:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-02 23:52 [PATCH] xfs: test attr_list_by_handle cursor iteration Darrick J. Wong
2016-08-02 23:52 ` Darrick J. Wong
2016-08-03  4:47 ` Eryu Guan
2016-08-03  4:47   ` Eryu Guan
2016-08-03 22:42   ` Darrick J. Wong
2016-08-03 22:42     ` Darrick J. Wong
2016-08-03 22:51 ` [PATCH v2] " Darrick J. Wong
2016-08-03 22:51   ` Darrick J. Wong
2016-08-04  4:24   ` Eryu Guan
2016-08-04  4:24     ` Eryu Guan
2016-08-11 17:48     ` Christoph Hellwig
2016-08-11 17:48       ` Christoph Hellwig
2016-08-12  3:14       ` Eryu Guan
2016-08-12  3:14         ` Eryu Guan
2016-08-05  2:03 ` [PATCH v3] " Darrick J. Wong
2016-08-05  2:03   ` Darrick J. Wong

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.