[PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Zheng Liu]

From: Zheng Liu <wenqing.lz@taobao.com>

ext2fs_file_llseek is extented to introduce SEEK_DATA/HOLE.  In *_data()
function it will find the next data, and in *_hole() function it will find the
next hole.  A new error code called EXT2_ET_SEEK_BEYOND_EOF is define to
indicate that the offset is beyond the end of file.

Here they need to solve a problem that the caller can not dereference
ext2_file_t->pos because this structure is hidden by a typedef.  Thus,
EXT2_SEEK_OFFSET_INVALID is define to indicate whether or not it will
find the data/hole from ext2_file_t->pos.

CC: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
Hi Ted,

ext2fs_file_llseek_data/hole() seem to be weird because ext2_file_t structure is
hidden by a typedef.  The caller can not dereference it.  So I define a marco
called EXT2_SEEK_OFFSET_INVALID to let the caller indicate that it find the
data/hole from ext2_file_t->pos or from offset.  What do you think?

Thanks,
						- Zheng

 lib/ext2fs/ext2_err.et.in |  3 ++
 lib/ext2fs/ext2fs.h       |  4 +++
 lib/ext2fs/fileio.c       | 80 ++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
index c99a167..f763938 100644
--- a/lib/ext2fs/ext2_err.et.in
+++ b/lib/ext2fs/ext2_err.et.in
@@ -473,4 +473,7 @@ ec	EXT2_ET_UNKNOWN_CSUM,
 ec	EXT2_ET_MMP_CSUM_INVALID,
 	"MMP block checksum does not match MMP block"
 
+ec	EXT2_ET_SEEK_BEYOND_EOF,
+	"lseek beyond the EOF"
+
 	end
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 7139b4d..474049f 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -160,6 +160,10 @@ typedef struct ext2_file *ext2_file_t;
 #define EXT2_SEEK_SET	0
 #define EXT2_SEEK_CUR	1
 #define EXT2_SEEK_END	2
+#define EXT2_SEEK_DATA	3
+#define EXT2_SEEK_HOLE	4
+
+#define EXT2_SEEK_OFFSET_INVALID	-1
 
 /*
  * Flags for the ext2_filsys structure and for ext2fs_open()
diff --git a/lib/ext2fs/fileio.c b/lib/ext2fs/fileio.c
index 1f7002c..ab33290 100644
--- a/lib/ext2fs/fileio.c
+++ b/lib/ext2fs/fileio.c
@@ -312,10 +312,84 @@ fail:
 	return retval;
 }
 
+static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset)
+{
+	int		ret_flags, flag = 1;
+	ext2_filsys	fs = file->fs;
+	errcode_t	retval;
+
+	/*
+	 * If offset == EXT2_SEEK_OFFSET_INVALID, that means that
+	 * the caller wants to find the data from file->pos.
+	 */
+	offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos;
+	file->blockno = offset / fs->blocksize;
+	while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) {
+		retval = ext2fs_bmap2(fs, file->ino, &file->inode,
+				      BMAP_BUFFER, 0, file->blockno,
+				      &ret_flags, &file->physblock);
+		if (retval)
+			return retval;
+		if (file->physblock == 0 || (ret_flags & BMAP_RET_UNINIT)) {
+			file->blockno++;
+			flag = 0;
+		} else {
+			if (flag)
+				file->pos = offset;
+			else
+				file->pos = file->blockno * fs->blocksize;
+			break;
+		}
+	}
+
+	/* notify the caller that there is no any data */
+	if (file->blockno * fs->blocksize >= EXT2_I_SIZE(&file->inode))
+		return EXT2_ET_SEEK_BEYOND_EOF;
+
+	return 0;
+}
+
+static errcode_t ext2fs_file_llseek_hole(ext2_file_t file, __u64 offset)
+{
+	int		ret_flags, flag = 1;
+	ext2_filsys	fs = file->fs;
+	errcode_t	retval;
+
+	/*
+	 * If offset == EXT2_SEEK_OFFSET_INVALID, that means that
+	 * the caller wants to find the hole from file->pos.
+	 */
+	offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos;
+	if (offset >= EXT2_I_SIZE(&file->inode))
+		return EXT2_ET_SEEK_BEYOND_EOF;
+
+	file->blockno = offset / fs->blocksize;
+	while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) {
+		retval = ext2fs_bmap2(fs, file->ino, &file->inode,
+				      BMAP_BUFFER, 0, file->blockno,
+				      &ret_flags, &file->physblock);
+		if (retval)
+			return retval;
+		if (file->physblock == 0 || (ret_flags & BMAP_RET_UNINIT)) {
+			if (flag)
+				file->pos = offset;
+			else
+				file->pos = file->blockno * fs->blocksize;
+			break;
+		} else {
+			file->blockno++;
+			flag = 0;
+		}
+	}
+
+	return 0;
+}
+
 errcode_t ext2fs_file_llseek(ext2_file_t file, __u64 offset,
 			    int whence, __u64 *ret_pos)
 {
 	EXT2_CHECK_MAGIC(file, EXT2_ET_MAGIC_EXT2_FILE);
+	errcode_t retval = 0;
 
 	if (whence == EXT2_SEEK_SET)
 		file->pos = offset;
@@ -323,13 +397,17 @@ errcode_t ext2fs_file_llseek(ext2_file_t file, __u64 offset,
 		file->pos += offset;
 	else if (whence == EXT2_SEEK_END)
 		file->pos = EXT2_I_SIZE(&file->inode) + offset;
+	else if (whence == EXT2_SEEK_DATA)
+		retval = ext2fs_file_llseek_data(file, offset);
+	else if (whence == EXT2_SEEK_HOLE)
+		retval = ext2fs_file_llseek_hole(file, offset);
 	else
 		return EXT2_ET_INVALID_ARGUMENT;
 
 	if (ret_pos)
 		*ret_pos = file->pos;
 
-	return 0;
+	return retval;
 }
 
 errcode_t ext2fs_file_lseek(ext2_file_t file, ext2_off_t offset,
-- 
1.7.12.rc2.18.g61b472e

[PATCH 2/2 v2] debugfs: dump a sparse file

[Zheng Liu]

From: Zheng Liu <wenqing.lz@taobao.com>

When ext2fs_file_open() is called with EXT2_FILE_SPAESE flag, ext2fs_file_read()
will return EXT2_ET_READ_HOLE_FOUND if it meets a hole/uninitialized block.

CC: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 debugfs/dump.c            | 44 ++++++++++++++++++++++++++++++++++++--------
 lib/ext2fs/ext2_err.et.in |  3 +++
 lib/ext2fs/ext2fs.h       |  1 +
 lib/ext2fs/fileio.c       | 11 +++++++++--
 4 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/debugfs/dump.c b/debugfs/dump.c
index a15a0b7..a34b293 100644
--- a/debugfs/dump.c
+++ b/debugfs/dump.c
@@ -107,26 +107,54 @@ static void dump_file(const char *cmdname, ext2_ino_t ino, int fd,
 	struct ext2_inode	inode;
 	char 		buf[8192];
 	ext2_file_t	e2_file;
-	int		nbytes;
+	ext2_off64_t	ret_pos;
+	int		nbytes, flags = 0;
 	unsigned int	got;
 
 	if (debugfs_read_inode(ino, &inode, cmdname))
 		return;
 
-	retval = ext2fs_file_open(current_fs, ino, 0, &e2_file);
+	/*
+	 * we need to handle stdout because this function is called by
+	 * do_cat() and do_dump().
+	 */
+	if (fd != 1)
+		flags = EXT2_FILE_SPARSE;
+	retval = ext2fs_file_open(current_fs, ino, flags, &e2_file);
 	if (retval) {
 		com_err(cmdname, retval, "while opening ext2 file");
 		return;
 	}
 	while (1) {
 		retval = ext2fs_file_read(e2_file, buf, sizeof(buf), &got);
-		if (retval)
+		if (retval && retval != EXT2_ET_READ_HOLE_FOUND)
 			com_err(cmdname, retval, "while reading ext2 file");
-		if (got == 0)
-			break;
-		nbytes = write(fd, buf, got);
-		if ((unsigned) nbytes != got)
-			com_err(cmdname, errno, "while writing file");
+		if (retval & EXT2_ET_READ_HOLE_FOUND) {
+			if (got) {
+				nbytes = write(fd, buf, got);
+				if ((unsigned) nbytes != got)
+					com_err(cmdname, errno,
+						"while writing file");
+			}
+			retval = ext2fs_file_llseek(e2_file,
+						    EXT2_SEEK_OFFSET_INVALID,
+						    EXT2_SEEK_DATA, &ret_pos);
+			if (retval == EXT2_ET_SEEK_BEYOND_EOF)
+				break;
+			if (retval)
+				com_err(cmdname, retval,
+					"while lseeking ext2 file");
+			ret_pos = lseek64(fd, ret_pos, SEEK_SET);
+			if (ret_pos < 0)
+				com_err(cmdname, retval,
+					"while lseeking target file");
+		} else {
+			if (got == 0)
+				break;
+			nbytes = write(fd, buf, got);
+			if ((unsigned) nbytes != got)
+				com_err(cmdname, errno, "while writing file");
+		}
 	}
 	retval = ext2fs_file_close(e2_file);
 	if (retval) {
diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
index f763938..0d6a031 100644
--- a/lib/ext2fs/ext2_err.et.in
+++ b/lib/ext2fs/ext2_err.et.in
@@ -476,4 +476,7 @@ ec	EXT2_ET_MMP_CSUM_INVALID,
 ec	EXT2_ET_SEEK_BEYOND_EOF,
 	"lseek beyond the EOF"
 
+ec	EXT2_ET_READ_HOLE_FOUND,
+	"We read a hole/uninitialized block with EXT2_FILE_SPARSE flag"
+
 	end
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 474049f..8aa24b3 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -149,6 +149,7 @@ typedef struct ext2_struct_dblist *ext2_dblist;
 
 #define EXT2_FILE_WRITE		0x0001
 #define EXT2_FILE_CREATE	0x0002
+#define EXT2_FILE_SPARSE	0x0004
 
 #define EXT2_FILE_MASK		0x00FF
 
diff --git a/lib/ext2fs/fileio.c b/lib/ext2fs/fileio.c
index ab33290..6281db7 100644
--- a/lib/ext2fs/fileio.c
+++ b/lib/ext2fs/fileio.c
@@ -185,11 +185,12 @@ static errcode_t load_buffer(ext2_file_t file, int dontfill)
 {
 	ext2_filsys	fs = file->fs;
 	errcode_t	retval;
+	int		ret_flags = 0;
 
 	if (!(file->flags & EXT2_FILE_BUF_VALID)) {
 		retval = ext2fs_bmap2(fs, file->ino, &file->inode,
-				     BMAP_BUFFER, 0, file->blockno, 0,
-				     &file->physblock);
+				     BMAP_BUFFER, 0, file->blockno,
+				     &ret_flags, &file->physblock);
 		if (retval)
 			return retval;
 		if (!dontfill) {
@@ -203,6 +204,12 @@ static errcode_t load_buffer(ext2_file_t file, int dontfill)
 				memset(file->buf, 0, fs->blocksize);
 		}
 		file->flags |= EXT2_FILE_BUF_VALID;
+		if (file->flags & EXT2_FILE_SPARSE) {
+			if (file->physblock == 0 ||
+			    ret_flags & BMAP_RET_UNINIT) {
+				return EXT2_ET_READ_HOLE_FOUND;
+			}
+		}
 	}
 	return 0;
 }
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Theodore Ts'o]

On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote:
> 
> ext2fs_file_llseek_data/hole() seem to be weird because ext2_file_t
> structure is hidden by a typedef.  The caller can not dereference
> it.  So I define a marco called EXT2_SEEK_OFFSET_INVALID to let the
> caller indicate that it find the data/hole from ext2_file_t->pos or
> from offset.  What do you think?

Is the problem you're worried about is that the user can't get current
location?

That's pretty easy to solve.  You can get it the same way it works
with the lseek(2) system call.

     retval = ext2fs_file_llseek(file, 0, SEEK_CUR, &pos);

Upon the return, pos will be contain the current file offset.

     	 	     	     	     	 	 - Ted

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Zheng Liu]

On Mon, Jan 14, 2013 at 10:23:29PM -0500, Theodore Ts'o wrote:
> On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote:
> > 
> > ext2fs_file_llseek_data/hole() seem to be weird because ext2_file_t
> > structure is hidden by a typedef.  The caller can not dereference
> > it.  So I define a marco called EXT2_SEEK_OFFSET_INVALID to let the
> > caller indicate that it find the data/hole from ext2_file_t->pos or
> > from offset.  What do you think?
> 
> Is the problem you're worried about is that the user can't get current
> location?
> 
> That's pretty easy to solve.  You can get it the same way it works
> with the lseek(2) system call.
> 
>      retval = ext2fs_file_llseek(file, 0, SEEK_CUR, &pos);
> 
> Upon the return, pos will be contain the current file offset.

Ah, I see.  I will remove EXT2_SEEK_OFFSET_INVALID flag in next version.

Thanks,
                                                - Zheng

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Theodore Ts'o]

On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote:
> +static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset)
> +{
> +	int		ret_flags, flag = 1;
> +	ext2_filsys	fs = file->fs;
> +	errcode_t	retval;
> +
> +	/*
> +	 * If offset == EXT2_SEEK_OFFSET_INVALID, that means that
> +	 * the caller wants to find the data from file->pos.
> +	 */
> +	offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos;
> +	file->blockno = offset / fs->blocksize;
> +	while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) {
> +		retval = ext2fs_bmap2(fs, file->ino, &file->inode,
> +				      BMAP_BUFFER, 0, file->blockno,
> +				      &ret_flags, &file->physblock);

Using bmap will certainly work, although as an optimization we can do
much better for extents if we use the extents interface.  I won't
insist on this, though.  (We can always optimize this later).

       	  		     	 	- Ted

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Zheng Liu]

On Tue, Jan 15, 2013 at 01:55:09PM -0500, Theodore Ts'o wrote:
> On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote:
> > +static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset)
> > +{
> > +	int		ret_flags, flag = 1;
> > +	ext2_filsys	fs = file->fs;
> > +	errcode_t	retval;
> > +
> > +	/*
> > +	 * If offset == EXT2_SEEK_OFFSET_INVALID, that means that
> > +	 * the caller wants to find the data from file->pos.
> > +	 */
> > +	offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos;
> > +	file->blockno = offset / fs->blocksize;
> > +	while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) {
> > +		retval = ext2fs_bmap2(fs, file->ino, &file->inode,
> > +				      BMAP_BUFFER, 0, file->blockno,
> > +				      &ret_flags, &file->physblock);
> 
> Using bmap will certainly work, although as an optimization we can do
> much better for extents if we use the extents interface.  I won't
> insist on this, though.  (We can always optimize this later).

Yeah, it allows us to skip to the next data/hole directly if the extents
interface is used.  But if we do that, we will need to handle
extent-based file and indirect-based file resptively like this.

        if (inode->i_flags & EXT4_EXTENTS_FL) {
                ext2fs_file_ext_llseek_data();
                ...
        } else {
                ext2fs_file_ind_llseek_data();
                ...
        }

I am not sure whether it is too complicated or not for us.  What do you
think?

Thanks,
                                                - Zheng

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Theodore Ts'o]

On Wed, Jan 16, 2013 at 08:04:56PM +0800, Zheng Liu wrote:
> Yeah, it allows us to skip to the next data/hole directly if the extents
> interface is used.  But if we do that, we will need to handle
> extent-based file and indirect-based file resptively like this.
> 
>         if (inode->i_flags & EXT4_EXTENTS_FL) {
>                 ext2fs_file_ext_llseek_data();
>                 ...
>         } else {
>                 ext2fs_file_ind_llseek_data();
>                 ...
>         }
> 
> I am not sure whether it is too complicated or not for us.  What do you
> think?

I'm not too worried about the performance issues for debugfs.  But for
clients who are accessing ext[234] using libext2fs and FUSE, they
would probably notice in at least some circumstances.  I'm not that
worried about the complexity, but it's also not a high priority thing,
either --- it's a "nice to have", not a "must have".

Regards,

					- Ted

Re: [PATCH 1/2 v2] libext2fs: introduce lseek SEEK_DATA/HOLE

[Zheng Liu]

On Wed, Jan 16, 2013 at 09:59:18AM -0500, Theodore Ts'o wrote:
> On Wed, Jan 16, 2013 at 08:04:56PM +0800, Zheng Liu wrote:
> > Yeah, it allows us to skip to the next data/hole directly if the extents
> > interface is used.  But if we do that, we will need to handle
> > extent-based file and indirect-based file resptively like this.
> > 
> >         if (inode->i_flags & EXT4_EXTENTS_FL) {
> >                 ext2fs_file_ext_llseek_data();
> >                 ...
> >         } else {
> >                 ext2fs_file_ind_llseek_data();
> >                 ...
> >         }
> > 
> > I am not sure whether it is too complicated or not for us.  What do you
> > think?
> 
> I'm not too worried about the performance issues for debugfs.  But for
> clients who are accessing ext[234] using libext2fs and FUSE, they
> would probably notice in at least some circumstances.  I'm not that
> worried about the complexity, but it's also not a high priority thing,
> either --- it's a "nice to have", not a "must have".

Understood.  Let me try it.

Thanks,
                                                - Zheng