Unionfs ODF: use first writable branch (fix/cleanup)

Cleanup code in ->create, ->symlink, and ->mknod: refactor common code into
helper functions.  Also, this allows writing to multiple branches again,
which was broken by an earlier patch.

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index b292f88b3f7ff843e4bf164c3ad5261512ef19d1..9abc39cd3668dd1ba9260ab70da21a0883cdbc82 100644 (file)
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -18,13 +18,94 @@
 
 #include "union.h"
 
+/*
+ * Find a writeable branch to create new object in.  Checks all writeble
+ * branches of the parent inode, from istart to iend order; if none are
+ * suitable, also tries branch 0 (which may require a copyup).
+ *
+ * Return a lower_dentry we can use to create object in, or ERR_PTR.
+ */
+static struct dentry *find_writeable_branch(struct inode *parent,
+                                               struct dentry *dentry)
+{
+       int err = -EINVAL;
+       int bindex, istart, iend;
+       struct dentry *lower_dentry = NULL;
+
+       istart = ibstart(parent);
+       iend = ibend(parent);
+       if (istart < 0)
+               goto out;
+
+begin:
+       for (bindex = istart; bindex <= iend; bindex++) {
+               /* skip non-writeable branches */
+               err = is_robranch_super(dentry->d_sb, bindex);
+               if (err) {
+                       err = -EROFS;
+                       continue;
+               }
+               lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+               if (!lower_dentry)
+                       continue;
+               if (lower_dentry->d_inode && UNIONFS_D(dentry)->odf.whiteout)
+                       /*
+                        * This file logically does not exist. Delete the lower
+                        * file and make way for the new creation here
+                        */
+                       err = unionfs_force_rm(dentry, &lower_dentry, bindex);
+
+               /*
+                * we have either found the right branch to create this
+                * dentry in, or we are bailing out with an error, which
+                * will be handled outside this loop
+                */
+               break;
+       }
+       /*
+        * If istart wasn't already branch 0, and we got any error, then try
+        * branch 0 (which may require copyup)
+        */
+       if (err && istart > 0) {
+               istart = 0;
+               iend = 0;
+               goto begin;
+       }
+
+       /*
+        * If we tried even branch 0, and still got an error, abort.  But if
+        * the error was an EROFS, then we should try to copyup.
+        */
+       if (err && err != -EROFS)
+               goto out;
+
+       /*
+        * If we get here, then check if copyup needed.  If lower_dentry is
+        * NULL, create the entire dentry directory structure in branch 0.
+        */
+       if (!lower_dentry) {
+               bindex = 0;
+               lower_dentry = create_parents(parent, dentry,
+                                               dentry->d_name.name, bindex);
+               if (IS_ERR(lower_dentry)) {
+                       err = PTR_ERR(lower_dentry);
+                       goto out;
+               }
+       }
+       err = 0;                /* all's well */
+out:
+       if (err)
+               return ERR_PTR(err);
+       return lower_dentry;
+}
+
+
 static int unionfs_create(struct inode *parent, struct dentry *dentry,
                          int mode, struct nameidata *nd)
 {
        int err = 0;
        struct dentry *lower_dentry = NULL;
        struct dentry *lower_parent_dentry = NULL;
-       int bstart;
        int valid = 0;
        struct nameidata lower_nd;
 
@@ -45,47 +126,12 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
         */
        BUG_ON(!valid && dentry->d_inode);
 
-       /*
-        * We shouldn't create things in a read-only branch; this check is a
-        * bit redundant as we don't allow branch 0 to be read-only at the
-        * moment
-        */
-       err = is_robranch_super(dentry->d_sb, 0);
-       if (err) {
-               err = -EROFS;
+       lower_dentry = find_writeable_branch(parent, dentry);
+       if (IS_ERR(lower_dentry)) {
+               err = PTR_ERR(lower_dentry);
                goto out;
        }
 
-       /*
-        * We _always_ create on branch 0
-        */
-       bstart = 0;
-
-       lower_dentry = unionfs_lower_dentry_idx(dentry, bstart);
-       if (!lower_dentry) {
-               /*
-                * if lower_dentry is NULL, create the entire
-                * dentry directory structure in branch 'bindex'.
-                * lower_dentry will NOT be null when bindex == bstart
-                * because lookup passed as a negative unionfs dentry
-                * pointing to a lone negative underlying dentry
-                */
-               lower_dentry = create_parents(parent, dentry,
-                                             dentry->d_name.name,
-                                             bstart);
-               if (!lower_dentry || IS_ERR(lower_dentry)) {
-                       if (IS_ERR(lower_dentry))
-                               err = PTR_ERR(lower_dentry);
-                       goto out;
-               }
-               unionfs_postcopyup_setmnt(dentry->d_parent);
-       }
-
-       if (lower_dentry->d_inode && UNIONFS_D(dentry)->odf.whiteout) {
-               err = unionfs_force_rm(dentry, &lower_dentry, bstart);
-               if (err)
-                       goto out;
-       }
        lower_parent_dentry = lock_parent(lower_dentry);
        if (IS_ERR(lower_parent_dentry)) {
                err = PTR_ERR(lower_parent_dentry);
@@ -101,7 +147,6 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
 
        if (err || !lower_dentry->d_inode)
                unlock_dir(lower_parent_dentry);
-
        else {
                err = odf_lookup(dentry->d_parent, dentry,
                                 ODF_LOOKUP_FILE|ODF_LOOKUP_RMV_WH);
@@ -353,7 +398,6 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
        struct dentry *lower_dentry = NULL;
        struct dentry *lower_dir_dentry = NULL;
        umode_t mode;
-       int bstart;
 
        unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
        unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
@@ -364,48 +408,17 @@ static int unionfs_symlink(struct inode *dir, struct dentry *dentry,
                goto out;
        }
 
-       /* We create in the leftmost branch. */
-       bstart = 0;
-
-       lower_dentry = unionfs_lower_dentry_idx(dentry, bstart);
-       if (!lower_dentry) {
-               /*
-                * if lower_dentry is NULL, create the entire
-                * dentry directory structure in branch 'bindex'.
-                * lower_dentry will NOT be null when bindex ==
-                * bstart because lookup passed as a negative
-                * unionfs dentry pointing to a lone negative
-                * underlying dentry
-                */
-               lower_dentry = create_parents(dir, dentry,
-                                             dentry->d_name.name,
-                                             bstart);
-               if (!lower_dentry || IS_ERR(lower_dentry)) {
-                       if (IS_ERR(lower_dentry))
-                               err = PTR_ERR(lower_dentry);
-
-                       printk(KERN_ERR "unionfs: lower dentry "
-                              "NULL (or error) for bindex = %d\n",
-                              bstart);
-                       goto out;
-               }
-               unionfs_postcopyup_setmnt(dentry->d_parent);
-       }
-
-       if (lower_dentry->d_inode && UNIONFS_D(dentry)->odf.whiteout) {
-               err = unionfs_force_rm(dentry, &lower_dentry, bstart);
-               if (err)
-                       goto out;
+       lower_dentry = find_writeable_branch(dir, dentry);
+       if (IS_ERR(lower_dentry)) {
+               err = PTR_ERR(lower_dentry);
+               goto out;
        }
 
        lower_dir_dentry = lock_parent(lower_dentry);
 
-       err = is_robranch_super(dentry->d_sb, bstart);
-       if (!err) {
-               mode = S_IALLUGO;
-               err = vfs_symlink(lower_dir_dentry->d_inode,
-                                 lower_dentry, symname, mode);
-       }
+       mode = S_IALLUGO;
+       err = vfs_symlink(lower_dir_dentry->d_inode,
+                               lower_dentry, symname, mode);
        unlock_dir(lower_dir_dentry);
 
        if (!(err || !lower_dentry->d_inode)) {
@@ -562,7 +575,6 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
        int err = 0;
        struct dentry *lower_dentry = NULL;
        struct dentry *lower_parent_dentry = NULL;
-       int bstart;
 
        unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
        unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
@@ -573,29 +585,10 @@ static int unionfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
                goto out;
        }
 
-       bstart = 0;
-
-       if (is_robranch_super(dentry->d_sb, bstart))
+       lower_dentry = find_writeable_branch(dir, dentry);
+       if (IS_ERR(lower_dentry)) {
+               err = PTR_ERR(lower_dentry);
                goto out;
-
-       lower_dentry = unionfs_lower_dentry_idx(dentry, bstart);
-       if (!lower_dentry) {
-               lower_dentry = create_parents(dir, dentry,
-                                             dentry->d_name.name,
-                                             bstart);
-               if (IS_ERR(lower_dentry)) {
-                       printk(KERN_ERR "unionfs: failed to create "
-                              "parents on %d, err = %ld\n",
-                              bstart, PTR_ERR(lower_dentry));
-                       goto out;
-               }
-               unionfs_postcopyup_setmnt(dentry->d_parent);
-       }
-
-       if (lower_dentry->d_inode && UNIONFS_D(dentry)->odf.whiteout) {
-               err = unionfs_force_rm(dentry, &lower_dentry, bstart);
-               if (err)
-                       goto out;
        }
 
        lower_parent_dentry = lock_parent(lower_dentry);