Re: [RFC][PATCH 13/14] ext3 whiteout support

2007-05-15 Thread Bharata B Rao
On Mon, May 14, 2007 at 01:16:57PM -0700, Badari Pulavarty wrote:
 On Mon, 2007-05-14 at 15:14 +0530, Bharata B Rao wrote:
  From: Bharata B Rao [EMAIL PROTECTED]
  Subject: ext3 whiteout support
  
  Introduce whiteout support for ext3.
  
  Signed-off-by: Bharata B Rao [EMAIL PROTECTED]
  Signed-off-by: Jan Blunck [EMAIL PROTECTED]
  ---
   fs/ext3/dir.c   |2 -
   fs/ext3/namei.c |   62 
  
   fs/ext3/super.c |   11 +++-
   include/linux/ext3_fs.h |5 +++
   4 files changed, 72 insertions(+), 8 deletions(-)
  
  --- a/fs/ext3/dir.c
  +++ b/fs/ext3/dir.c
  @@ -29,7 +29,7 @@
   #include linux/rbtree.h
   
   static unsigned char ext3_filetype_table[] = {
  -   DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK
  +   DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK, 
  DT_WHT
   };
   
   static int ext3_readdir(struct file *, void *, filldir_t);
  --- a/fs/ext3/namei.c
  +++ b/fs/ext3/namei.c
  @@ -1071,6 +1071,7 @@ static unsigned char ext3_type_by_mode[S
  [S_IFIFO  S_SHIFT]= EXT3_FT_FIFO,
  [S_IFSOCK  S_SHIFT]   = EXT3_FT_SOCK,
  [S_IFLNK  S_SHIFT]= EXT3_FT_SYMLINK,
  +   [S_IFWHT  S_SHIFT]= EXT3_FT_WHT,
   };
   
   static inline void ext3_set_de_type(struct super_block *sb,
  @@ -1786,7 +1787,7 @@ out_stop:
   /*
* routine to check that the specified directory is empty (for rmdir)
*/
  -static int empty_dir (struct inode * inode)
  +static int empty_dir (handle_t *handle, struct inode * inode)
 
 Is there a reason for passing the handle ? Why couldn't you get it from
 journal_current_handle() if needed to do the delete the whiteout ?

Yes, using journal_current_handle() is possible, didn't realize it earlier.

 
   {
  unsigned long offset;
  struct buffer_head * bh;
  @@ -1848,8 +1849,28 @@ static int empty_dir (struct inode * ino
  continue;
  }
  if (le32_to_cpu(de-inode)) {
  -   brelse (bh);
  -   return 0;
  +   /* If this is a whiteout, remove it */
  +   if (de-file_type == EXT3_FT_WHT) {
  +   unsigned long ino = le32_to_cpu(de-inode);
  +   struct inode *tmp_inode = iget(inode-i_sb, 
  ino);
  +   if (!tmp_inode) {
  +   brelse (bh);
  +   return 0;
  +   }
  +
  +   if (ext3_delete_entry(handle, inode, de, bh)) {
  +   iput(tmp_inode);
  +   brelse (bh);
  +   return 0;
  +   }
  +
  +   tmp_inode-i_ctime = inode-i_ctime;
  +   tmp_inode-i_nlink--;
  +   iput(tmp_inode);
  +   } else {
  +   brelse (bh);
  +   return 0;
  +   }
  }
  offset += le16_to_cpu(de-rec_len);
  de = (struct ext3_dir_entry_2 *)
  @@ -2031,7 +2052,7 @@ static int ext3_rmdir (struct inode * di
  goto end_rmdir;
   
  retval = -ENOTEMPTY;
  -   if (!empty_dir (inode))
  +   if (!empty_dir (handle, inode))
  goto end_rmdir;
   
  retval = ext3_delete_entry(handle, dir, de, bh);
  @@ -2060,6 +2081,36 @@ end_rmdir:
  return retval;
   }
   
  +static int ext3_whiteout(struct inode *dir, struct dentry *dentry)
  +{
  +   struct inode * inode;
  +   int err, retries = 0;
  +   handle_t *handle;
  +
  +retry:
  +   handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir-i_sb) +
  +   EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
  +   2*EXT3_QUOTA_INIT_BLOCKS(dir-i_sb));
  +   if (IS_ERR(handle))
  +   return PTR_ERR(handle);
  +
  +   if (IS_DIRSYNC(dir))
  +   handle-h_sync = 1;
  +
  +   inode = ext3_new_inode (handle, dir, S_IFWHT | S_IRUGO);
  +   err = PTR_ERR(inode);
  +   if (IS_ERR(inode))
  +   goto out_stop;
 
 Don't you need to call init_special_inode() here ?
 Or this is handled somewhere else ?

Whiteout doesn't have any attributes and hence we are not explicitly
doing init_special_inode() on this. Accesses to whiteout files are trapped
at the VFS lookup itself and creation and deletion of whiteouts are handled
automatically by VFS. So I believe init_special_inode() isn't necessary
on a whiteout file.

Regards,
Bharata.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 13/14] ext3 whiteout support

2007-05-15 Thread Jan Blunck

On 5/15/07, Bharata B Rao [EMAIL PROTECTED] wrote:

On Mon, May 14, 2007 at 01:16:57PM -0700, Badari Pulavarty wrote:
 On Mon, 2007-05-14 at 15:14 +0530, Bharata B Rao wrote:
  From: Bharata B Rao [EMAIL PROTECTED]
 
  +static int ext3_whiteout(struct inode *dir, struct dentry *dentry)
  +{
  +   struct inode * inode;
  +   int err, retries = 0;
  +   handle_t *handle;
  +
  +retry:
  +   handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir-i_sb) +
  +   EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
  +   2*EXT3_QUOTA_INIT_BLOCKS(dir-i_sb));
  +   if (IS_ERR(handle))
  +   return PTR_ERR(handle);
  +
  +   if (IS_DIRSYNC(dir))
  +   handle-h_sync = 1;
  +
  +   inode = ext3_new_inode (handle, dir, S_IFWHT | S_IRUGO);
  +   err = PTR_ERR(inode);
  +   if (IS_ERR(inode))
  +   goto out_stop;

 Don't you need to call init_special_inode() here ?
 Or this is handled somewhere else ?

Whiteout doesn't have any attributes and hence we are not explicitly
doing init_special_inode() on this. Accesses to whiteout files are trapped
at the VFS lookup itself and creation and deletion of whiteouts are handled
automatically by VFS. So I believe init_special_inode() isn't necessary
on a whiteout file.



I added default whiteout file operations. So calling
init_special_inode() seems to make sense.

I know the ext2/ext3 whiteout patches are not really where they should
be. I plan to use a reserved inode number to reflect the case that the
inode itself doesn't have any attributes itself. It makes sense to
have a singleton whiteout inode per superblock.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC][PATCH 13/14] ext3 whiteout support

2007-05-14 Thread Bharata B Rao
From: Bharata B Rao [EMAIL PROTECTED]
Subject: ext3 whiteout support

Introduce whiteout support for ext3.

Signed-off-by: Bharata B Rao [EMAIL PROTECTED]
Signed-off-by: Jan Blunck [EMAIL PROTECTED]
---
 fs/ext3/dir.c   |2 -
 fs/ext3/namei.c |   62 
 fs/ext3/super.c |   11 +++-
 include/linux/ext3_fs.h |5 +++
 4 files changed, 72 insertions(+), 8 deletions(-)

--- a/fs/ext3/dir.c
+++ b/fs/ext3/dir.c
@@ -29,7 +29,7 @@
 #include linux/rbtree.h
 
 static unsigned char ext3_filetype_table[] = {
-   DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK
+   DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK, 
DT_WHT
 };
 
 static int ext3_readdir(struct file *, void *, filldir_t);
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -1071,6 +1071,7 @@ static unsigned char ext3_type_by_mode[S
[S_IFIFO  S_SHIFT]= EXT3_FT_FIFO,
[S_IFSOCK  S_SHIFT]   = EXT3_FT_SOCK,
[S_IFLNK  S_SHIFT]= EXT3_FT_SYMLINK,
+   [S_IFWHT  S_SHIFT]= EXT3_FT_WHT,
 };
 
 static inline void ext3_set_de_type(struct super_block *sb,
@@ -1786,7 +1787,7 @@ out_stop:
 /*
  * routine to check that the specified directory is empty (for rmdir)
  */
-static int empty_dir (struct inode * inode)
+static int empty_dir (handle_t *handle, struct inode * inode)
 {
unsigned long offset;
struct buffer_head * bh;
@@ -1848,8 +1849,28 @@ static int empty_dir (struct inode * ino
continue;
}
if (le32_to_cpu(de-inode)) {
-   brelse (bh);
-   return 0;
+   /* If this is a whiteout, remove it */
+   if (de-file_type == EXT3_FT_WHT) {
+   unsigned long ino = le32_to_cpu(de-inode);
+   struct inode *tmp_inode = iget(inode-i_sb, 
ino);
+   if (!tmp_inode) {
+   brelse (bh);
+   return 0;
+   }
+
+   if (ext3_delete_entry(handle, inode, de, bh)) {
+   iput(tmp_inode);
+   brelse (bh);
+   return 0;
+   }
+
+   tmp_inode-i_ctime = inode-i_ctime;
+   tmp_inode-i_nlink--;
+   iput(tmp_inode);
+   } else {
+   brelse (bh);
+   return 0;
+   }
}
offset += le16_to_cpu(de-rec_len);
de = (struct ext3_dir_entry_2 *)
@@ -2031,7 +2052,7 @@ static int ext3_rmdir (struct inode * di
goto end_rmdir;
 
retval = -ENOTEMPTY;
-   if (!empty_dir (inode))
+   if (!empty_dir (handle, inode))
goto end_rmdir;
 
retval = ext3_delete_entry(handle, dir, de, bh);
@@ -2060,6 +2081,36 @@ end_rmdir:
return retval;
 }
 
+static int ext3_whiteout(struct inode *dir, struct dentry *dentry)
+{
+   struct inode * inode;
+   int err, retries = 0;
+   handle_t *handle;
+
+retry:
+   handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir-i_sb) +
+   EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
+   2*EXT3_QUOTA_INIT_BLOCKS(dir-i_sb));
+   if (IS_ERR(handle))
+   return PTR_ERR(handle);
+
+   if (IS_DIRSYNC(dir))
+   handle-h_sync = 1;
+
+   inode = ext3_new_inode (handle, dir, S_IFWHT | S_IRUGO);
+   err = PTR_ERR(inode);
+   if (IS_ERR(inode))
+   goto out_stop;
+
+   err = ext3_add_nondir(handle, dentry, inode);
+
+out_stop:
+   ext3_journal_stop(handle);
+   if (err == -ENOSPC  ext3_should_retry_alloc(dir-i_sb, retries))
+   goto retry;
+   return err;
+}
+
 static int ext3_unlink(struct inode * dir, struct dentry *dentry)
 {
int retval;
@@ -2261,7 +2312,7 @@ static int ext3_rename (struct inode * o
if (S_ISDIR(old_inode-i_mode)) {
if (new_inode) {
retval = -ENOTEMPTY;
-   if (!empty_dir (new_inode))
+   if (!empty_dir (handle, new_inode))
goto end_rename;
}
retval = -EIO;
@@ -2377,6 +2428,7 @@ const struct inode_operations ext3_dir_i
.mkdir  = ext3_mkdir,
.rmdir  = ext3_rmdir,
.mknod  = ext3_mknod,
+   .whiteout   = ext3_whiteout,
.rename = ext3_rename,
.setattr= ext3_setattr,
 #ifdef CONFIG_EXT3_FS_XATTR
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1492,6 

Re: [RFC][PATCH 13/14] ext3 whiteout support

2007-05-14 Thread Badari Pulavarty
On Mon, 2007-05-14 at 15:14 +0530, Bharata B Rao wrote:
 From: Bharata B Rao [EMAIL PROTECTED]
 Subject: ext3 whiteout support
 
 Introduce whiteout support for ext3.
 
 Signed-off-by: Bharata B Rao [EMAIL PROTECTED]
 Signed-off-by: Jan Blunck [EMAIL PROTECTED]
 ---
  fs/ext3/dir.c   |2 -
  fs/ext3/namei.c |   62 
 
  fs/ext3/super.c |   11 +++-
  include/linux/ext3_fs.h |5 +++
  4 files changed, 72 insertions(+), 8 deletions(-)
 
 --- a/fs/ext3/dir.c
 +++ b/fs/ext3/dir.c
 @@ -29,7 +29,7 @@
  #include linux/rbtree.h
  
  static unsigned char ext3_filetype_table[] = {
 - DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK
 + DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK, 
 DT_WHT
  };
  
  static int ext3_readdir(struct file *, void *, filldir_t);
 --- a/fs/ext3/namei.c
 +++ b/fs/ext3/namei.c
 @@ -1071,6 +1071,7 @@ static unsigned char ext3_type_by_mode[S
   [S_IFIFO  S_SHIFT]= EXT3_FT_FIFO,
   [S_IFSOCK  S_SHIFT]   = EXT3_FT_SOCK,
   [S_IFLNK  S_SHIFT]= EXT3_FT_SYMLINK,
 + [S_IFWHT  S_SHIFT]= EXT3_FT_WHT,
  };
  
  static inline void ext3_set_de_type(struct super_block *sb,
 @@ -1786,7 +1787,7 @@ out_stop:
  /*
   * routine to check that the specified directory is empty (for rmdir)
   */
 -static int empty_dir (struct inode * inode)
 +static int empty_dir (handle_t *handle, struct inode * inode)

Is there a reason for passing the handle ? Why couldn't you get it from
journal_current_handle() if needed to do the delete the whiteout ?

  {
   unsigned long offset;
   struct buffer_head * bh;
 @@ -1848,8 +1849,28 @@ static int empty_dir (struct inode * ino
   continue;
   }
   if (le32_to_cpu(de-inode)) {
 - brelse (bh);
 - return 0;
 + /* If this is a whiteout, remove it */
 + if (de-file_type == EXT3_FT_WHT) {
 + unsigned long ino = le32_to_cpu(de-inode);
 + struct inode *tmp_inode = iget(inode-i_sb, 
 ino);
 + if (!tmp_inode) {
 + brelse (bh);
 + return 0;
 + }
 +
 + if (ext3_delete_entry(handle, inode, de, bh)) {
 + iput(tmp_inode);
 + brelse (bh);
 + return 0;
 + }
 +
 + tmp_inode-i_ctime = inode-i_ctime;
 + tmp_inode-i_nlink--;
 + iput(tmp_inode);
 + } else {
 + brelse (bh);
 + return 0;
 + }
   }
   offset += le16_to_cpu(de-rec_len);
   de = (struct ext3_dir_entry_2 *)
 @@ -2031,7 +2052,7 @@ static int ext3_rmdir (struct inode * di
   goto end_rmdir;
  
   retval = -ENOTEMPTY;
 - if (!empty_dir (inode))
 + if (!empty_dir (handle, inode))
   goto end_rmdir;
  
   retval = ext3_delete_entry(handle, dir, de, bh);
 @@ -2060,6 +2081,36 @@ end_rmdir:
   return retval;
  }
  
 +static int ext3_whiteout(struct inode *dir, struct dentry *dentry)
 +{
 + struct inode * inode;
 + int err, retries = 0;
 + handle_t *handle;
 +
 +retry:
 + handle = ext3_journal_start(dir, EXT3_DATA_TRANS_BLOCKS(dir-i_sb) +
 + EXT3_INDEX_EXTRA_TRANS_BLOCKS + 3 +
 + 2*EXT3_QUOTA_INIT_BLOCKS(dir-i_sb));
 + if (IS_ERR(handle))
 + return PTR_ERR(handle);
 +
 + if (IS_DIRSYNC(dir))
 + handle-h_sync = 1;
 +
 + inode = ext3_new_inode (handle, dir, S_IFWHT | S_IRUGO);
 + err = PTR_ERR(inode);
 + if (IS_ERR(inode))
 + goto out_stop;

Don't you need to call init_special_inode() here ?
Or this is handled somewhere else ?

 +
 + err = ext3_add_nondir(handle, dentry, inode);
 +
 +out_stop:
 + ext3_journal_stop(handle);
 + if (err == -ENOSPC  ext3_should_retry_alloc(dir-i_sb, retries))
 + goto retry;
 + return err;
 +}
 +

Thanks,
Badari

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 13/14] ext3 whiteout support

2007-05-14 Thread Andreas Dilger
On May 14, 2007  15:14 +0530, Bharata B Rao wrote:
  #define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV0x0008 /* Journal device */
  #define EXT3_FEATURE_INCOMPAT_META_BG0x0010
 +#define EXT3_FEATURE_INCOMPAT_WHITEOUT   0x0020

Is this flag reserved with Ted?  It isn't listed in the e2fsprogs repo.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH 13/14] ext3 whiteout support

2007-05-14 Thread Jan Blunck

On 5/14/07, Andreas Dilger [EMAIL PROTECTED] wrote:

On May 14, 2007  15:14 +0530, Bharata B Rao wrote:
  #define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV0x0008 /* Journal device */
  #define EXT3_FEATURE_INCOMPAT_META_BG0x0010
 +#define EXT3_FEATURE_INCOMPAT_WHITEOUT   0x0020

Is this flag reserved with Ted?  It isn't listed in the e2fsprogs repo.



I don't know. I tried to contact him a few weeks ago but failed.
Guess, maybe he isn't reading the @thunk.org email anymore which was
reference in the e2fsprogs source I used.

Ted,
from ext2_fs.h I learn that the value 0x0020 is left unused.

#define EXT2_FEATURE_INCOMPAT_META_BG   0x0010
#define EXT3_FEATURE_INCOMPAT_EXTENTS   0x0040
#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080

Is this intentionally?

Cheers,
Jan
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html