Nathan Scott writes:
 > Hi there,
 > 
 > On Wed, Aug 31, 2005 at 09:48:23AM +1000, Nathan Scott wrote:
 > > ...
 > > # find /relay
 > > /relay
 > > /relay/block
 > > /relay/block/sdd
 > > /relay/block/sdd/trace3
 > > /relay/block/sdd/trace2
 > > /relay/block/sdd/trace1
 > > /relay/block/sdd/trace0
 > > /relay/block/sdb
 > > /relay/block/sdb/trace3
 > > /relay/block/sdb/trace2
 > > /relay/block/sdb/trace1
 > > /relay/block/sdb/trace0
 > > 
 > > and does the correct dynamic setup and teardown of the hierarchy
 > > as the userspace tool starts and stops tracing.  I had to modify
 > > the relayfs rmdir code a bit to make this work properly, I'll
 > > send a separate patch for that shortly.
 > 
 > Here it is.  The problem was that relayfs is allowing a directory
 > with children to be removed rather than returning -ENOTEMPTY.  It
 > looks like this can be resolved by splitting the shared relayfs
 > unlink code (which is using simple_unlink) into separate file/dir
 > variants, one using simple_unlink, the other using simple_rmdir.
 > 

Hi,

You're right, it should be using simple_rmdir rather than
simple_unlink for removing directories.  Thanks for sending the patch,
which I've modified a bit to avoid splitting the rmdir/unlink cases
into separate functions, since they're almost the same except for what
they end up calling.  relayfs_remove_dir now doesn't do anything but
call relayfs_remove (it didn't do much more than that before anyway),
but it makes sense to me to keep it, as the counterpart to
relayfs_create_dir.  Let me know if you see any problems with it.

Thanks,

Tom

--- inode.c~    2005-08-31 04:08:07.000000000 -0500
+++ inode.c     2005-08-31 03:44:40.000000000 -0500
@@ -189,26 +189,39 @@ struct dentry *relayfs_create_dir(const 
 /**
  *     relayfs_remove - remove a file or directory in the relay filesystem
  *     @dentry: file or directory dentry
+ *
+ *     Returns 0 if successful, negative otherwise.
  */
 int relayfs_remove(struct dentry *dentry)
 {
-       struct dentry *parent = dentry->d_parent;
+       struct dentry *parent;
+       int error = 0;
+
+       if (!dentry)
+               return -EINVAL;
+       parent = dentry->d_parent;
        if (!parent)
                return -EINVAL;
 
        parent = dget(parent);
        down(&parent->d_inode->i_sem);
        if (dentry->d_inode) {
-               simple_unlink(parent->d_inode, dentry);
-               d_delete(dentry);
+               if (S_ISDIR(dentry->d_inode->i_mode))
+                       error = simple_rmdir(parent->d_inode, dentry);
+               else
+                       error = simple_unlink(parent->d_inode, dentry);
+               if (!error)
+                       d_delete(dentry);
        }
-       dput(dentry);
+       if (!error)
+               dput(dentry);
        up(&parent->d_inode->i_sem);
        dput(parent);
 
-       simple_release_fs(&relayfs_mount, &relayfs_mount_count);
+       if (!error)
+               simple_release_fs(&relayfs_mount, &relayfs_mount_count);
 
-       return 0;
+       return error;
 }
 
 /**
@@ -219,9 +232,6 @@ int relayfs_remove(struct dentry *dentry
  */
 int relayfs_remove_dir(struct dentry *dentry)
 {
-       if (!dentry)
-               return -EINVAL;
-
        return relayfs_remove(dentry);
 }
 


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

Reply via email to