diff mbox series

commit 1f981816ac8d855cc2af6150711f3417e02e12bf Author: Andreas Dilger <andreas.dilger@intel.com> Date: Fri Apr 13 02:01:12 2012 -0600

Message ID 20180702161416.60769-1-c17828@cray.com
State Rejected, archived
Headers show
Series commit 1f981816ac8d855cc2af6150711f3417e02e12bf Author: Andreas Dilger <andreas.dilger@intel.com> Date: Fri Apr 13 02:01:12 2012 -0600 | expand

Commit Message

Artem Blagodarenko July 2, 2018, 4:14 p.m. UTC
tests: attr checking code test

Add a regression test for in-inode xattrs.

This is part of "e2fsck: clean up xattr checking code, add test" patch.
Cleanup is removed becaus is not actual for this codebase.

Change-Id: I905130bb5adcfe84860bf8316f6a89a2b1ebca80
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
---
 lib/ext2fs/Makefile.in   |  20 +++-
 lib/ext2fs/tst_read_ea.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 255 insertions(+), 4 deletions(-)
 create mode 100644 lib/ext2fs/tst_read_ea.c

Comments

Theodore Ts'o July 2, 2018, 6:05 p.m. UTC | #1
On Mon, Jul 02, 2018 at 07:14:16PM +0300, c17828 wrote:
> tests: attr checking code test
> 
> Add a regression test for in-inode xattrs.
> 
> This is part of "e2fsck: clean up xattr checking code, add test" patch.
> Cleanup is removed becaus is not actual for this codebase.

This commit description doesn't really explain what the test does.

The test is also a complete no-op.  Alas, it appears it was never
seriously tested, at least not recently.  (See below for more
details.)

> --- /dev/nullt
> +++ b/lib/ext2fs/tst_read_ea.c
> @@ -0,0 +1,239 @@
> +/*
> + * tst_getsize.c --- this function tests the getsize function

The header comment was never adjusted after test was cribbed from
tst_getsize.c.

> + * %Begin-Header%
> + * This file may be redistributed under the terms of the GNU Public
> + * License.
> + * %End-Header%
> + */
> +
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE	/* for asprintf */
> +#endif
> +#include <stdio.h>

Before all of the header declarations, you have to include "config.h".
Otherwise, all of the HAVE_UNISTD_H, or more critically, HAVE_MNTENT_H
was never defined:

> +int main(int argc, const char *argv[])
> +{
> +#if HAVE_MNTENT_H
> +	ext2_filsys fs;

As a result, the tst_read_ea program is one gigantic no-op:

% size tst_read_ea.o
   text	   data	    bss	    dec	    hex	filename
     51	      0	      0	     51	     33	tst_read_ea.o

I tried adding the missing '#include "config.h' line, at which point
tst_read_ea.c fails to compile:

/usr/projects/e2fsprogs/e2fsprogs-maint/lib/ext2fs/tst_read_ea.c: In function ‘get_xattrs2’:
/usr/projects/e2fsprogs/e2fsprogs-maint/lib/ext2fs/tst_read_ea.c:150:9: warning: implicit declaration of function ‘ext2fs_attr_get’; did you mean ‘ext2fs_xattr_get’? [-Wimplicit-function-declaration]
   err = ext2fs_attr_get(fs, inode, EXT2_ATTR_INDEX_USER,
         ^~~~~~~~~~~~~~~
         ext2fs_xattr_get
/usr/projects/e2fsprogs/e2fsprogs-maint/lib/ext2fs/tst_read_ea.c:150:36: error: ‘EXT2_ATTR_INDEX_USER’ undeclared (first use in this function); did you mean ‘EXT_LAST_INDEX’?
   err = ext2fs_attr_get(fs, inode, EXT2_ATTR_INDEX_USER,
                                    ^~~~~~~~~~~~~~~~~~~~
                                    EXT_LAST_INDEX
/usr/projects/e2fsprogs/e2fsprogs-maint/lib/ext2fs/tst_read_ea.c:150:36: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:665: tst_read_ea.o] Error 1

It looks like tst_read_ea is using some older version of the xattr
support functions, and was never cleaned up when those functions went
upstream.  Apparently no one noticed because all of these calls were
compiled out.


More generally, this test depnds on user running this test to (a) be
able to find a currently mounted ext4 file system, (b) be able to open
its underlying block device, and (c) be able to write to the root
directory of the mounted file system.  In practice, it's going to
require root access.

A much better way of testing this would be to use a pre-made file
system, much all of the regression tests in the tests/ directory.  You
can either then use a stand-alone C program, or you can just use
debugfs.  (For example, see attached below.)

Anyway, I'm not going to apply this commit in its current state, and
I'd suggest completely working it in the Lustre's version of
e2fsprogs, since it's really not doing any good there.

Regards,

						- Ted

Script started on 2018-07-02 14:00:39-04:00

% /build/e2fsprogs-maint/debugfs/debugfs /tmp/foo.img
debugfs 1.44.3-rc1 (24-June-2018)
debugfs:  stat short
Inode: 13   Type: regular    Mode:  0644   Flags: 0x80000
Generation: 1647185658    Version: 0x00000000:00000001
User:     0   Group:     0   Project:     0   Size: 0
File ACL: 0
Links: 1   Blockcount: 0
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x5b3a671f:e8ffa0fc -- Mon Jul  2 13:55:43 2018
 atime: 0x5b3a670a:19faf30c -- Mon Jul  2 13:55:22 2018
 mtime: 0x5b3a670a:19faf30c -- Mon Jul  2 13:55:22 2018
crtime: 0x5b3a670a:19faf30c -- Mon Jul  2 13:55:22 2018
Size of extra inode fields: 32
Extended attributes:
  user.test (7) = "testing"
Inode checksum: 0xe53b328b
EXTENTS:
debugfs:  ea_get short user.test
user.test (7) = "testing"

debugfs:  stat long
Inode: 14   Type: regular    Mode:  0644   Flags: 0x80000
Generation: 2203030209    Version: 0x00000000:00000001
User:     0   Group:     0   Project:     0   Size: 0
File ACL: 1617
Links: 1   Blockcount: 2
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x5b3a6717:3656cf8c -- Mon Jul  2 13:55:35 2018
 atime: 0x5b3a6711:ab445038 -- Mon Jul  2 13:55:29 2018
 mtime: 0x5b3a6711:ab445038 -- Mon Jul  2 13:55:29 2018
crtime: 0x5b3a6711:ab445038 -- Mon Jul  2 13:55:29 2018
Size of extra inode fields: 32
Extended attributes:
  user.test (87)
Inode checksum: 0x00833190
EXTENTS:
debugfs:  ea_get long user.test
user.test (87) = "testing12345678901234567890123456789012345678901234567890123456789012345678901234567890"

debugfs:  stat inline
Inode: 12   Type: regular    Mode:  0644   Flags: 0x10000000
Generation: 1942101456    Version: 0x00000000:00000001
User:     0   Group:     0   Project:     0   Size: 88
File ACL: 0
Links: 1   Blockcount: 0
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x5b3a67bf:b7f94e2c -- Mon Jul  2 13:58:23 2018
 atime: 0x5b3a66fe:9af75700 -- Mon Jul  2 13:55:10 2018
 mtime: 0x5b3a67bf:b7f94e2c -- Mon Jul  2 13:58:23 2018
crtime: 0x5b3a66fe:9af75700 -- Mon Jul  2 13:55:10 2018
Size of extra inode fields: 32
Extended attributes:
  system.data (28)
Inode checksum: 0xedce581b
Size of inline data: 88
debugfs:  ea_get inline system.data
system.data (28) = "456789012345678901234567890\n"

debugfs:  inode_dump -b inline
0000  7465 7374 696e 6731 3233 3435 3637 3839  testing123456789
0020  3031 3233 3435 3637 3839 3031 3233 3435  0123456789012345
0040  3637 3839 3031 3233 3435 3637 3839 3031  6789012345678901
0060  3233 3435 3637 3839 3031 3233            234567890123

debugfs:  cat inline
testing12345678901234567890123456789012345678901234567890123456789012345678901234567890
debugfs:  inode_dump -x inline
magic = ea020000, length = 96, value_start =4 

offset = 4 (0004), name_len = 4, name_index = 7
value_offset = 64 (0104), value_inum = 0, value_size = 28
name = data
value = 456789012345678901234567890^J

last entry found at offset 24 (0030)
debugfs:  q
% exit

Script done on 2018-07-02 14:02:10-04:00
diff mbox series

Patch

diff --git a/lib/ext2fs/Makefile.in b/lib/ext2fs/Makefile.in
index cb81882b..0bd017fd 100644
--- a/lib/ext2fs/Makefile.in
+++ b/lib/ext2fs/Makefile.in
@@ -211,6 +211,7 @@  SRCS= ext2_err.c \
 	$(srcdir)/tst_byteswap.c \
 	$(srcdir)/tst_getsize.c \
 	$(srcdir)/tst_iscan.c \
+	$(srcdir)/tst_read_ea.c \
 	$(srcdir)/undo_io.c \
 	$(srcdir)/unix_io.c \
 	$(srcdir)/sparse_io.c \
@@ -292,6 +293,11 @@  tst_iscan: tst_iscan.o $(STATIC_LIBEXT2FS) $(DEPSTATIC_LIBCOM_ERR)
 	$(Q) $(CC) -o tst_iscan tst_iscan.o $(ALL_LDFLAGS) \
 		$(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) $(SYSLIBS)
 
+tst_read_ea: tst_read_ea.o $(STATIC_LIBEXT2FS) $(DEPSTATIC_LIBCOM_ERR)
+	$(E) "	LD $@"
+	$(Q) $(CC) -o tst_read_ea tst_read_ea.o $(STATIC_LIBEXT2FS) \
+		$(STATIC_LIBCOM_ERR)
+
 tst_getsize: tst_getsize.o $(STATIC_LIBEXT2FS) $(DEPSTATIC_LIBCOM_ERR)
 	$(E) "	LD $@"
 	$(Q) $(CC) -o tst_getsize tst_getsize.o $(ALL_LDFLAGS) \
@@ -524,14 +530,15 @@  mkjournal: mkjournal.c $(STATIC_LIBEXT2FS) $(DEPLIBCOM_ERR)
 		$(STATIC_LIBEXT2FS) $(LIBCOM_ERR) $(ALL_CFLAGS) $(SYSLIBS)
 
 fullcheck check:: tst_bitops tst_badblocks tst_iscan tst_types tst_icount \
-    tst_super_size tst_types tst_inode_size tst_csum tst_crc32c tst_bitmaps \
-    tst_inline tst_inline_data tst_libext2fs tst_sha256 tst_sha512 \
-    tst_digest_encode tst_getsize tst_getsectsize
+    tst_read_ea tst_super_size tst_types tst_inode_size tst_csum tst_crc32c \
+    tst_bitmaps tst_inline tst_inline_data tst_libext2fs tst_sha256 \
+    tst_sha512 tst_digest_encode tst_getsize tst_getsectsize
 	$(TESTENV) ./tst_bitops
 	$(TESTENV) ./tst_badblocks
 	$(TESTENV) ./tst_iscan
 	$(TESTENV) ./tst_types
 	$(TESTENV) ./tst_icount
+	$(TESTENV) ./tst_read_ea
 	$(TESTENV) ./tst_super_size
 	$(TESTENV) ./tst_inode_size
 	$(TESTENV) ./tst_csum
@@ -581,7 +588,7 @@  clean::
 		tst_badblocks tst_iscan ext2_err.et ext2_err.c ext2_err.h \
 		tst_byteswap tst_ismounted tst_getsize tst_getsectsize \
 		tst_bitops tst_types tst_icount tst_super_size tst_csum \
-		tst_bitmaps tst_bitmaps_out tst_extents tst_inline \
+		tst_read_ea tst_bitmaps tst_bitmaps_out tst_extents tst_inline \
 		tst_inline_data tst_inode_size tst_bitmaps_cmd.c \
 		tst_digest_encode tst_sha256 tst_sha512 \
 		ext2_tdbtool mkjournal debug_cmds.c tst_cmds.c extent_cmds.c \
@@ -1099,6 +1106,11 @@  tst_iscan.o: $(srcdir)/tst_iscan.c $(top_builddir)/lib/config.h \
  $(srcdir)/ext2_fs.h $(srcdir)/ext3_extents.h $(top_srcdir)/lib/et/com_err.h \
  $(srcdir)/ext2_io.h $(top_builddir)/lib/ext2fs/ext2_err.h \
  $(srcdir)/ext2_ext_attr.h $(srcdir)/bitops.h
+tst_read_ea.o: $(srcdir)/tst_read_ea.c $(srcdir)/ext2_fs.h \
+ $(top_builddir)/lib/ext2fs/ext2_types.h $(srcdir)/ext2fs.h \
+ $(srcdir)/ext2_fs.h $(srcdir)/ext3_extents.h $(top_srcdir)/lib/et/com_err.h \
+ $(srcdir)/ext2_io.h $(top_builddir)/lib/ext2fs/ext2_err.h \
+ $(srcdir)/ext2_ext_attr.h $(srcdir)/bitops.h
 undo_io.o: $(srcdir)/undo_io.c $(top_builddir)/lib/config.h \
  $(top_builddir)/lib/dirpaths.h $(srcdir)/ext2_fs.h \
  $(top_builddir)/lib/ext2fs/ext2_types.h $(srcdir)/ext2fs.h \
diff --git a/lib/ext2fs/tst_read_ea.c b/lib/ext2fs/tst_read_ea.c
new file mode 100644
index 00000000..21fdf9bf
--- /dev/null
+++ b/lib/ext2fs/tst_read_ea.c
@@ -0,0 +1,239 @@ 
+/*
+ * tst_getsize.c --- this function tests the getsize function
+ *
+ * %Begin-Header%
+ * This file may be redistributed under the terms of the GNU Public
+ * License.
+ * %End-Header%
+ */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE	/* for asprintf */
+#endif
+#include <stdio.h>
+#include <string.h>
+#if HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#include <fcntl.h>
+#include <time.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#ifdef HAVE_ATTR_XATTR_H
+#include <attr/xattr.h>
+#elif HAVE_SYS_XATTR_H
+#include <sys/xattr.h>
+#else
+/* This is just a test program, let's try to work around the lack of header */
+extern ssize_t fgetxattr (int __filedes, const char *__name,
+				void *__value, size_t __size);
+extern int fsetxattr (int __filedes, const char *__name,
+		      const void *__value, size_t __size, int __flags);
+#endif
+#if HAVE_MNTENT_H
+#include <mntent.h>
+#include <assert.h>
+#if HAVE_ERRNO_H
+#include <errno.h>
+#endif
+
+#include "ext2_fs.h"
+#include "ext2fs.h"
+
+#define NR_XATTRS 256
+char tmpvalue[NR_XATTRS + 1];
+
+struct ea {
+	char *name;
+	char *value;
+};
+
+struct ea *ea_table;
+
+static void init_ea_table(void)
+{
+	int i;
+
+	ea_table = malloc(sizeof(struct ea) * NR_XATTRS);
+	if (ea_table == NULL) {
+		perror("maloc failed");
+		exit(1);
+	}
+	for (i = 0; i < NR_XATTRS; i ++) {
+		ea_table[i].name = malloc(i + 2 + strlen("user."));
+		if (ea_table[i].name == NULL) {
+			perror("malloc failed");
+			exit(1);
+		}
+		strcpy(ea_table[i].name, "user.");
+		memset(ea_table[i].name + strlen("user."), 'X', i + 1);
+		ea_table[i].name[i + 1 + strlen("user.")] = 0;
+
+		ea_table[i].value = malloc(NR_XATTRS - i + 1);
+		if (ea_table[i].value == NULL) {
+			perror("malloc failed");
+			exit(1);
+		}
+		memset(ea_table[i].value, 'Y', NR_XATTRS - i);
+		ea_table[i].value[NR_XATTRS - i] = 0;
+	}
+}
+
+static int set_xattrs(int fd)
+{
+	int i;
+
+	for (i = 0; i < NR_XATTRS; i ++) {
+		if (fsetxattr(fd, ea_table[i].name, ea_table[i].value,
+			      NR_XATTRS - i + 1, XATTR_CREATE) == -1) {
+			if (errno != ENOSPC) {
+				perror("fsetxattr failed");
+				exit(1);
+			}
+			break;
+		}
+	}
+	printf("\t%d xattrs are set\n", i);
+	return i;
+}
+
+void get_xattrs1(int fd, int nr)
+{
+	int i;
+	ssize_t size;
+
+	printf("\ttesting fgetxattr .. "); fflush(stdout);
+
+	for (i = 0; i < nr; i ++) {
+		size = fgetxattr(fd, ea_table[i].name, tmpvalue,
+				 NR_XATTRS - i + 1);
+		if (size == -1) {
+			perror("fgetxattr failed");
+			exit(1);
+		}
+		if (memcmp(ea_table[i].value, tmpvalue, nr - i + 1)) {
+			fprintf(stderr, "value mismatch");
+			exit(1);
+		}
+	}
+
+	printf("%d xattrs are checked, ok\n", i);
+}
+
+void get_xattrs2(const char *device, ext2_ino_t ino, int nr)
+{
+	ext2_filsys fs;
+	int i;
+	struct ext2_inode *inode;
+	errcode_t err;
+	int size;
+
+	printf("\ttesting ext2fs_attr_get .. "); fflush(stdout);
+
+	err = ext2fs_open(device, 0, 0, 0, unix_io_manager, &fs);
+	assert(err == 0);
+
+	err = ext2fs_get_mem(EXT2_INODE_SIZE(fs->super), &inode);
+	if (err) {
+		com_err("get_xattrs2", err, "allocating memory");
+		exit(1);
+	}
+
+	err = ext2fs_read_inode_full(fs, ino, inode,
+				     EXT2_INODE_SIZE(fs->super));
+	if (err) {
+		com_err("get_xattrs2", err, "reading inode");
+		exit(1);
+	}
+	for (i = 0; i < nr; i ++) {
+		err = ext2fs_attr_get(fs, inode, EXT2_ATTR_INDEX_USER,
+				      ea_table[i].name + strlen("user."),
+				      tmpvalue, sizeof(tmpvalue), &size);
+		if (err) {
+			com_err("get_xattrs2", err, "getting xattr");
+			exit(1);
+		}
+		assert(size == (NR_XATTRS - i + 1));
+
+		if (memcmp(ea_table[i].value, tmpvalue, size)) {
+			fprintf(stderr, "value mismatch");
+			exit(1);
+		}
+	}
+	ext2fs_close(fs);
+
+	printf("%d xattrs are checked, ok\n", i);
+}
+#endif /* HAVE_MNTENT_H */
+
+int main(int argc, const char *argv[])
+{
+#if HAVE_MNTENT_H
+	ext2_filsys fs;
+	FILE *f;
+	struct mntent *mnt;
+	char *name;
+	int fd;
+	errcode_t err;
+	struct stat st;
+	int nr;
+	int tested = 0;
+
+	initialize_ext2_error_table();
+
+	init_ea_table();
+
+	f = setmntent(MOUNTED, "r");
+	if (!f) {
+		fprintf(stderr, "failed to setmntent\n");
+		return 1;
+	}
+
+	while ((mnt = getmntent(f)) != NULL) {
+		if (hasmntopt(mnt, "user_xattr") == NULL)
+			continue;
+		err = ext2fs_open(mnt->mnt_fsname, 0, 0, 0,
+				  unix_io_manager, &fs);
+		if (err) {
+			com_err("tst_read_ea", err, "opening fs %s:%s",
+				mnt->mnt_fsname, mnt->mnt_type);
+			continue;
+		}
+		ext2fs_close(fs);
+
+		printf("type=%s, fsname=%s, mtpt=%s\n", mnt->mnt_type,
+		       mnt->mnt_fsname, mnt->mnt_dir);
+
+		asprintf(&name, "%s/readeaXXXXXX", mnt->mnt_dir);
+		fd = mkstemp(name);
+		if (fd == -1) {
+			fprintf(stderr, "tst_read_ea: mkstemp failed on %s: %s",
+				name, strerror(errno));
+			continue;
+		}
+		if (fstat(fd, &st)) {
+			fprintf(stderr, "tst_read_ea: fstat failed on %s: %s\n",
+				name, strerror(errno));
+			exit(1);
+		}
+		nr = set_xattrs(fd);
+
+		sync();
+		get_xattrs1(fd, nr);
+		close(fd);
+
+		get_xattrs2(mnt->mnt_fsname, st.st_ino, nr);
+
+		unlink(name);
+		free(name);
+		tested = 1;
+	}
+	endmntent(f);
+
+	if (!tested)
+		fprintf(stderr,
+			"\tno ext2 based filesystems mounted with user_xattr\n"
+			"\thope it is ok\n");
+#endif /* HAVE_MNTENT_H */
+	return 0;
+}