[PATCH 0/2] e2fsprogs: two fixes

[PATCH 0/2] e2fsprogs: two fixes

[Robert Yang]

* Fix icache
* Fix populate-extfs.sh

Robert Yang (1):
  inode.c: only update the icache for ext2_inode

Søren Holm (1):
  populate-extfs.sh: espace the space in the filename

 contrib/populate-extfs.sh |   20 ++++++++++----------
 lib/ext2fs/inode.c        |   20 ++++++++++++--------
 2 files changed, 22 insertions(+), 18 deletions(-)
 mode change 100755 => 100644 contrib/populate-extfs.sh

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] inode.c: only update the icache for ext2_inode

[Robert Yang]

We read the cache when:

bufsize == sizeof(struct ext2_inode)

so we should update the cache in the same rule, otherwise there would be
errors, for example:

cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()

Then update the cache:
cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()

And the ino 14 would hit the cache[1] when bufsize = 128 (but it was
cached by bufsize = 156), so there would be errors.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 lib/ext2fs/inode.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
index 573a8fa..cd85b47 100644
--- a/lib/ext2fs/inode.c
+++ b/lib/ext2fs/inode.c
@@ -614,10 +614,12 @@ errcode_t ext2fs_read_inode_full(ext2_filsys fs, ext2_ino_t ino,
 #endif
 
 	/* Update the inode cache */
-	fs->icache->cache_last = (fs->icache->cache_last + 1) %
-		fs->icache->cache_size;
-	fs->icache->cache[fs->icache->cache_last].ino = ino;
-	fs->icache->cache[fs->icache->cache_last].inode = *inode;
+	if (bufsize == sizeof(struct ext2_inode)) {
+		fs->icache->cache_last = (fs->icache->cache_last + 1) %
+			fs->icache->cache_size;
+		fs->icache->cache[fs->icache->cache_last].ino = ino;
+		fs->icache->cache[fs->icache->cache_last].inode = *inode;
+	}
 
 	return 0;
 }
@@ -650,10 +652,12 @@ errcode_t ext2fs_write_inode_full(ext2_filsys fs, ext2_ino_t ino,
 
 	/* Check to see if the inode cache needs to be updated */
 	if (fs->icache) {
-		for (i=0; i < fs->icache->cache_size; i++) {
-			if (fs->icache->cache[i].ino == ino) {
-				fs->icache->cache[i].inode = *inode;
-				break;
+		if (bufsize == sizeof(struct ext2_inode)) {
+			for (i=0; i < fs->icache->cache_size; i++) {
+				if (fs->icache->cache[i].ino == ino) {
+					fs->icache->cache[i].inode = *inode;
+					break;
+				}
 			}
 		}
 	} else {
-- 
1.7.10.4

Re: [PATCH 1/2] inode.c: only update the icache for ext2_inode

[Theodore Ts'o]

On Fri, Jan 03, 2014 at 04:15:40AM -0500, Robert Yang wrote:
> We read the cache when:
> 
> bufsize == sizeof(struct ext2_inode)
> 
> so we should update the cache in the same rule, otherwise there would be
> errors, for example:
> 
> cache[0]: cached ino 14 when bufsize = 128 by ext2fs_write_inode_full()
> cache[1]: cached ino 14 when bufsize = 156 by ext2fs_read_inode_full()
> 
> Then update the cache:
> cache[0]: cached ino 15 when bufsize = 156 by ext2fs_read_inode_full()
> 
> And the ino 14 would hit the cache[1] when bufsize = 128 (but it was
> cached by bufsize = 156), so there would be errors.

So I've been looking at this code, and it looks like what we have is
correct.  The inode cache is a writethrough cache of the first 128
bytes of the inode, which means that we always update the on-disk
image as well as updating the first 128 bytes of the inode.

Yoru change would actually add a bug, because if the inode 14 was in
the cache, and then we tried to write a large inode, skipping the
inode cache update would mean that a subsequent read of a original
inode size would result in inode cache not being updated, and there
would thus be stale data in the cache.

Here's a test program which proves that what we have right now is
working.  If you think there's a problem, could you perhaps try
adjusting this program to show what you think is the bug?

	       	       	       	    	      - Ted

/*
 * Test read/write inode functions
 */

#include <string.h>
#include <fcntl.h>
#include <ctype.h>
#include <termios.h>
#include <time.h>
#include <getopt.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/types.h>

#include <et/com_err.h>
#include <ext2fs/ext2fs.h>

extern int isatty(int);

static const char * program_name = "tst_rwinode";
static const char * device_name = NULL;
static int debug = 0;

static void usage(void)
{
	fprintf(stderr,
		"Usage: %s [-F] [-I inode_buffer_blocks] device\n",
		program_name);
	exit(1);
}

static void PRS(int argc, char *argv[])
{
	int		flush = 0;
	int		c;
#ifdef MTRACE
	extern void	*mallwatch;
#endif
	errcode_t	retval;

	setbuf(stdout, NULL);
	setbuf(stderr, NULL);
	add_error_table(&et_ext2_error_table);

	if (argc && *argv)
		program_name = *argv;
	while ((c = getopt (argc, argv, "d")) != EOF)
		switch (c) {
		case 'd':
			debug++;
			break;
		default:
			usage ();
		}
	device_name = argv[optind];
}

int main (int argc, char *argv[])
{
	errcode_t	retval = 0;
	ext2_filsys	fs;
	ext2_ino_t	ino;
	__u32	num_inodes = 0;
	struct ext2_inode_large large_inode;
	struct ext2_inode inode;
	ext2_inode_scan	scan;
	int err = 0;

	PRS(argc, argv);

	retval = ext2fs_open(device_name, EXT2_FLAG_RW,
			     0, 0, unix_io_manager, &fs);
	if (retval) {
		com_err(program_name, retval, "while trying to open '%s'",
			device_name);
		exit(1);
	}

	retval = ext2fs_read_bitmaps(fs);
	if (retval) {
		com_err(program_name, retval, "while reading bitmaps");
		exit(1);
	}

	retval = ext2fs_new_inode(fs, EXT2_ROOT_INO, 0644,
				  NULL, &ino);
	if (retval) {
		com_err(program_name, retval,
			"while trying to allocate inode");
		exit(1);
	}

	if (debug)
		printf("Using inode %u\n", ino);

	memset(&inode, 0, sizeof(inode));
	inode.i_size = 123;

	retval = ext2fs_write_new_inode(fs, ino, &inode);
	if (retval) {
		com_err(program_name, retval,
			"while trying to write inode %u", ino);
		exit(1);
	}

	memset(&inode, 0, sizeof(inode));

	retval = ext2fs_read_inode(fs, ino, &inode);
	if (retval) {
		com_err(program_name, retval,
			"while reading inode %u", ino);
		exit(1);
	}
	printf("inode size: %u\n", inode.i_size);
	if (inode.i_size != 123) {
		printf("Bad inode size!!\n");
		err++;
	}

	memset(&large_inode, 0, sizeof(large_inode));
	retval = ext2fs_read_inode_full(fs, ino,
					(struct ext2_inode *) &large_inode,
					sizeof(large_inode));
	if (retval) {
		com_err(program_name, retval,
			"while reading large inode %u", ino);
		exit(1);
	}

	printf("large inode size: %u\n", large_inode.i_size);
	if (large_inode.i_size != 123) {
		printf("Bad inode size!!\n");
		err++;
	}

	large_inode.i_size = 456;
	large_inode.i_crtime = 789;

	retval = ext2fs_write_inode_full(fs, ino,
					 (struct ext2_inode *) &large_inode,
					 sizeof(large_inode));
	if (retval) {
		com_err(program_name, retval,
			"while write large inode %u", ino);
		exit(1);
	}

	memset(&inode, 0, sizeof(inode));
	retval = ext2fs_read_inode(fs, ino, &inode);
	if (retval) {
		com_err(program_name, retval,
			"while reading inode %u", ino);
		exit(1);
	}
	printf("inode size: %u\n", inode.i_size);
	if (inode.i_size != 456) {
		printf("Bad inode size!!\n");
		err++;
	}

	memset(&large_inode, 0, sizeof(large_inode));
	retval = ext2fs_read_inode_full(fs, ino,
					(struct ext2_inode *) &large_inode,
					sizeof(large_inode));
	if (retval) {
		com_err(program_name, retval,
			"while reading large inode %u", ino);
		exit(1);
	}

	printf("large inode size: %u\n", large_inode.i_size);
	printf("large inode crtime: %u\n", large_inode.i_crtime);

	if (large_inode.i_size != 456) {
		printf("Bad inode size!!\n");
		err++;
	}
	if ((EXT2_INODE_SIZE(fs->super) > 128) &&
	    large_inode.i_crtime != 789) {
		printf("Bad crtime!!\n");
		err++;
	}

	retval = ext2fs_close(fs);
	if (retval) {
		com_err(program_name, retval,
			"while trying to close the filesystem");
		exit(1);
	}

	if (err == 0)
		printf("Test succeed!\n");

	remove_error_table(&et_ext2_error_table);
	exit(err);
}

[PATCH 2/2] populate-extfs.sh: espace the space in the filename

[Robert Yang]

From: Søren Holm <sgh@sgh.dk>

The filename which contains space would not get into the final
ext2/3/4 filsystem without this patch

Signed-off-by: Søren Holm <sgh@sgh.dk>
Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 contrib/populate-extfs.sh |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
 mode change 100755 => 100644 contrib/populate-extfs.sh

diff --git a/contrib/populate-extfs.sh b/contrib/populate-extfs.sh
old mode 100755
new mode 100644
index b1d3d1f..a84ec74
--- a/contrib/populate-extfs.sh
+++ b/contrib/populate-extfs.sh
@@ -44,7 +44,7 @@ fi
 		fi
 
 		# Only stat once since stat is a time consuming command
-		STAT=$(stat -c "TYPE=\"%F\";DEVNO=\"0x%t 0x%T\";MODE=\"%f\";U=\"%u\";G=\"%g\"" $FILE)
+		STAT=$(stat -c "TYPE=\"%F\";DEVNO=\"0x%t 0x%T\";MODE=\"%f\";U=\"%u\";G=\"%g\"" "$FILE")
 		eval $STAT
 
 		case $TYPE in
@@ -52,20 +52,20 @@ fi
 			echo "mkdir $TGT"
 			;;
 		"regular file" | "regular empty file")
-			echo "write $FILE $TGT"
+			echo "write \"$FILE\" \"$TGT\""
 			;;
 		"symbolic link")
-			LINK_TGT=$(readlink $FILE)
-			echo "symlink $TGT $LINK_TGT"
+			LINK_TGT=$(readlink "$FILE")
+			echo "symlink \"$TGT\" \"$LINK_TGT\""
 			;;
 		"block special file")
-			echo "mknod $TGT b $DEVNO"
+			echo "mknod \"$TGT\" b $DEVNO"
 			;;
 		"character special file")
-			echo "mknod $TGT c $DEVNO"
+			echo "mknod \"$TGT\" c $DEVNO"
 			;;
 		"fifo")
-			echo "mknod $TGT p"
+			echo "mknod \"$TGT\" p"
 			;;
 		*)
 			echo "Unknown/unhandled file type '$TYPE' file: $FILE" 1>&2
@@ -73,11 +73,11 @@ fi
 		esac
 
 		# Set the file mode
-		echo "sif $TGT mode 0x$MODE"
+		echo "sif \"$TGT\" mode 0x$MODE"
 
 		# Set uid and gid
-		echo "sif $TGT uid $U"
-		echo "sif $TGT gid $G"
+		echo "sif \"$TGT\" uid $U"
+		echo "sif \"$TGT\" gid $G"
 	done
 
 	# Handle the hard links.
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html