[RFC PATCH 0/3][e2fsprogs] 64bit IO manager support.

[RFC PATCH 0/3][e2fsprogs] 64bit IO manager support.

[Jose R. Santos]

This patch series add support for 64bit blocks to the IO manager
interface.  It add additional read_blk64 and write_blk64 routines to
the struct_io_manager as well as adding 64 bit routines to the unix
and inode io managers.  Coments on whether this approach is right(or
wrong) for adding support for 64bit blocks to e2fsprogs are welcome.

-JRS

[RFC PATCH 1/3][e2fsprogs] Add 64-bit IO manager operations to struct_io_manager.

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Add 64-bit IO manager operations to struct_io_manager.

In order to provide 64-bit block support for IO managers an maintain
ABI compatibility with the old API, some new functions need to be
added to struct_io_manger.  Luckily, strcut_io_manager has some
reserved space that we can use to add these new functions.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/ext2fs/ext2_io.h |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/lib/ext2fs/ext2_io.h b/lib/ext2fs/ext2_io.h
index 864d1f9..e3b0b1e 100644
--- a/lib/ext2fs/ext2_io.h
+++ b/lib/ext2fs/ext2_io.h
@@ -79,7 +79,11 @@ struct struct_io_manager {
 	errcode_t (*set_option)(io_channel channel, const char *option, 
 				const char *arg);
 	errcode_t (*get_stats)(io_channel channel, io_stats *io_stats);
-	int		reserved[14];
+	errcode_t (*read_blk64)(io_channel channel, unsigned long long block,
+					int count, void *data);
+	errcode_t (*write_blk64)(io_channel channel, unsigned long long block,
+					int count, const void *data);
+	int		reserved[12];
 };
 
 #define IO_FLAG_RW		0x0001
@@ -91,7 +95,9 @@ struct struct_io_manager {
 #define io_channel_close(c) 		((c)->manager->close((c)))
 #define io_channel_set_blksize(c,s)	((c)->manager->set_blksize((c),s))
 #define io_channel_read_blk(c,b,n,d)	((c)->manager->read_blk((c),b,n,d))
+#define io_channel_read_blk64(c,b,n,d)	((c)->manager->read_blk64((c),b,n,d))
 #define io_channel_write_blk(c,b,n,d)	((c)->manager->write_blk((c),b,n,d))
+#define io_channel_write_blk64(c,b,n,d) ((c)->manager->write_blk64((c),b,n,d))
 #define io_channel_flush(c) 		((c)->manager->flush((c)))
 #define io_channel_bumpcount(c)		((c)->refcount++)
 	

Re: [RFC PATCH 1/3][e2fsprogs] Add 64-bit IO manager operations to struct_io_manager.

[Theodore Tso]

On Mon, Mar 03, 2008 at 10:41:18AM -0600, Jose R. Santos wrote:
> From: Jose R. Santos <jrs@us.ibm.com>
> 
> Add 64-bit IO manager operations to struct_io_manager.
> 
> In order to provide 64-bit block support for IO managers an maintain
> ABI compatibility with the old API, some new functions need to be
> added to struct_io_manger.  Luckily, strcut_io_manager has some
> reserved space that we can use to add these new functions.

Looks good, and I won't NACK this patch just because of this.
However, be aware that:

> -	int		reserved[14];
> +	errcode_t (*read_blk64)(io_channel channel, unsigned long long block,
> +					int count, void *data);
> +	errcode_t (*write_blk64)(io_channel channel, unsigned long long block,
> +					int count, const void *data);
> +	int		reserved[12];
>  };

... this doesn't really work on x86_64 platforms since int's are 4
bytes and pointers are 8 bytes.

*Fortunately* it doesn't really matter here since the reserved block
is mainly to make sure that there are sufficient zero-filled bytes
after the structure so that there is little or no risk that if new
code which looks for a new function pointer like read_blk64 tries to
access that area after the structure, it will find zero's.  

As it turns out we don't ever expect the user to allocate io manager
structures, and in fact the primary place (and probably the only
place) where io_managers are defined is in libext2fs --- although if a
user were to define their own io_manager in their program, we would be
relying on the reserved fields to provide enough zero-filled slack
space to avoid problems.

In structures where it you really do need to maintain structure sizes,
it is very important to have a separate reserved_int[16] and
reserved_ptr[16] array --- or better yet, make the structure private
and not visible to the external callers, using accessor functions as
necessary, and use a callee (as opposed to caller) allocation
convention, which removes the need for the caller (i.e., calling
application) to know the size of the structure.

This is all not relevant to the patch at hand, though (although I will
probably not decrease the size of the reserved array to provide slack
for future expansions), but is the sort of thing that should go into a
wiki for e2fsprogs coding/style conventions one of these days.  :-)

     	 	   			    	   - Ted

[RFC PATCH 2/3][e2fsprogs] Add {read,write}_blk64 to unix_io.c

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Add {read,write}_blk64 to unix_io.c

Add 64-bit block capable routines to Unix IO manager.

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/ext2fs/unix_io.c |   32 +++++++++++++++++++++++++-------
 1 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 8497a41..eedbcdb 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -88,7 +88,11 @@ static errcode_t unix_set_option(io_channel channel, const char *option,
 static errcode_t unix_get_stats(io_channel channel, io_stats *stats)
 ;
 static void reuse_cache(io_channel channel, struct unix_private_data *data,
-		 struct unix_cache *cache, unsigned long block);
+		 struct unix_cache *cache, unsigned long long block);
+static errcode_t unix_read_blk64(io_channel channel, unsigned long long block,
+			       int count, void *data);
+static errcode_t unix_write_blk64(io_channel channel, unsigned long long block,
+				int count, const void *data);
 
 /* __FreeBSD_kernel__ is defined by GNU/kFreeBSD - the FreeBSD kernel
  * does not know buffered block devices - everything is raw. */
@@ -114,6 +118,8 @@ static struct struct_io_manager struct_unix_manager = {
 #endif
 	unix_set_option,
 	unix_get_stats,
+	unix_read_blk64,
+	unix_write_blk64,
 };
 
 io_manager unix_io_manager = &struct_unix_manager;
@@ -140,7 +146,7 @@ static errcode_t unix_get_stats(io_channel channel, io_stats *stats)
 #ifndef NEED_BOUNCE_BUFFER
 static errcode_t raw_read_blk(io_channel channel,
 			      struct unix_private_data *data,
-			      unsigned long block,
+			      unsigned long long block,
 			      int count, void *buf)
 {
 	errcode_t	retval;
@@ -229,7 +235,7 @@ error_out:
 
 static errcode_t raw_write_blk(io_channel channel,
 			       struct unix_private_data *data,
-			       unsigned long block,
+			       unsigned long long block,
 			       int count, const void *buf)
 {
 	ssize_t		size;
@@ -318,7 +324,7 @@ static void free_cache(struct unix_private_data *data)
  * entry to that should be reused.
  */
 static struct unix_cache *find_cached_block(struct unix_private_data *data,
-					    unsigned long block,
+					    unsigned long long block,
 					    struct unix_cache **eldest)
 {
 	struct unix_cache	*cache, *unused_cache, *oldest_cache;
@@ -348,7 +354,7 @@ static struct unix_cache *find_cached_block(struct unix_private_data *data,
  * Reuse a particular cache entry for another block.
  */
 static void reuse_cache(io_channel channel, struct unix_private_data *data,
-		 struct unix_cache *cache, unsigned long block)
+		 struct unix_cache *cache, unsigned long long block)
 {
 	if (cache->dirty && cache->in_use)
 		raw_write_blk(channel, data, cache->block, 1, cache->buf);
@@ -545,7 +551,7 @@ static errcode_t unix_set_blksize(io_channel channel, int blksize)
 }
 
 
-static errcode_t unix_read_blk(io_channel channel, unsigned long block,
+static errcode_t unix_read_blk64(io_channel channel, unsigned long long block,
 			       int count, void *buf)
 {
 	struct unix_private_data *data;
@@ -610,7 +616,13 @@ static errcode_t unix_read_blk(io_channel channel, unsigned long block,
 #endif /* NO_IO_CACHE */
 }
 
-static errcode_t unix_write_blk(io_channel channel, unsigned long block,
+static errcode_t unix_read_blk(io_channel channel, unsigned long block,
+			       int count, void *buf)
+{
+	return unix_read_blk64(channel, block, count, buf);
+}
+
+static errcode_t unix_write_blk64(io_channel channel, unsigned long long block,
 				int count, const void *buf)
 {
 	struct unix_private_data *data;
@@ -662,6 +674,12 @@ static errcode_t unix_write_blk(io_channel channel, unsigned long block,
 #endif /* NO_IO_CACHE */
 }
 
+static errcode_t unix_write_blk(io_channel channel, unsigned long block,
+				int count, const void *buf)
+{
+	return unix_write_blk64(channel, block, count, buf);
+}
+
 static errcode_t unix_write_byte(io_channel channel, unsigned long offset,
 				 int size, const void *buf)
 {

[RFC PATCH 3/3][e2fsprogs] Add {read,write}_blk64 to inode_io.c

[Jose R. Santos]

From: Jose R. Santos <jrs@us.ibm.com>

Add {read,write}_blk64 to inode_io.c

Add 64-bit block capable routines to inode IO manager.  Since fileio.c
does not yet have 64bit support, these routines will not handle 64bit
block numbers correctly (currently working on that).

Signed-off-by: Jose R. Santos <jrs@us.ibm.com>
--

 lib/ext2fs/inode_io.c |   32 ++++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/lib/ext2fs/inode_io.c b/lib/ext2fs/inode_io.c
index b5c08b9..0e23494 100644
--- a/lib/ext2fs/inode_io.c
+++ b/lib/ext2fs/inode_io.c
@@ -56,7 +56,11 @@ static errcode_t inode_write_blk(io_channel channel, unsigned long block,
 static errcode_t inode_flush(io_channel channel);
 static errcode_t inode_write_byte(io_channel channel, unsigned long offset,
 				int size, const void *data);
-
+static errcode_t inode_read_blk64(io_channel channel, 
+				unsigned long long block, int count, void *data);
+static errcode_t inode_write_blk64(io_channel channel, 
+				unsigned long long block, int count, const void *data);
+				
 static struct struct_io_manager struct_inode_manager = {
 	EXT2_ET_MAGIC_IO_MANAGER,
 	"Inode I/O Manager",
@@ -66,7 +70,11 @@ static struct struct_io_manager struct_inode_manager = {
 	inode_read_blk,
 	inode_write_blk,
 	inode_flush,
-	inode_write_byte
+	inode_write_byte,
+	NULL,
+	NULL,
+	inode_read_blk64,
+	inode_write_blk64
 };
 
 io_manager inode_io_manager = &struct_inode_manager;
@@ -197,8 +205,8 @@ static errcode_t inode_set_blksize(io_channel channel, int blksize)
 }
 
 
-static errcode_t inode_read_blk(io_channel channel, unsigned long block,
-			       int count, void *buf)
+static errcode_t inode_read_blk64(io_channel channel,
+				unsigned long long block, int count, void *buf)
 {
 	struct inode_private_data *data;
 	errcode_t	retval;
@@ -217,8 +225,14 @@ static errcode_t inode_read_blk(io_channel channel, unsigned long block,
 	return ext2fs_file_read(data->file, buf, count, 0);
 }
 
-static errcode_t inode_write_blk(io_channel channel, unsigned long block,
-				int count, const void *buf)
+static errcode_t inode_read_blk(io_channel channel, unsigned long block,
+			       int count, void *buf)
+{
+	return inode_read_blk64(channel, block, count, buf);
+}
+
+static errcode_t inode_write_blk64(io_channel channel,
+				unsigned long long block, int count, const void *buf)
 {
 	struct inode_private_data *data;
 	errcode_t	retval;
@@ -237,6 +251,12 @@ static errcode_t inode_write_blk(io_channel channel, unsigned long block,
 	return ext2fs_file_write(data->file, buf, count, 0);
 }
 
+static errcode_t inode_write_blk(io_channel channel, unsigned long block, 
+				int count, const void *buf)
+{
+	return inode_write_blk64(channel, block, count, buf);
+}
+
 static errcode_t inode_write_byte(io_channel channel, unsigned long offset,
 				 int size, const void *buf)
 {