* [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
* 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
* [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