Btrfs devel - Sep 2010 - [RFC] Removing a subvolume by an ordinary user

Hi all,

currently BTRFS doesn''t allow an ordinary user to remove a subvolume
(or
snapshot). I think that the reasons is simple: a subvolume may contain 
files/directories owned by other user.
Allowing an ordinary user to remove a subvolume means allowing an ordinary 
user to remove filess/directories owned by other user. And this is not good.

Moreover BTRFS removes  a subvolume asynchronously, so it is not possible to 
return an error like “hey you are trying to remove a not your file  ! Don’t do 
it !”.

My idea is to add another ioctl that permits to remove a subvolume only when 
it is empty and its host directory is writable by the user… like a directory. 
An option is to allow to remove an empty subvolume  with the unlink(2) syscall: 
no more tool is needed ! 
This will solve a lot of problem:
-	Consistently with the current unlink(2) behavior
-	The kernel has not to do complicate check
-	There no is necessity to add another interface to wait the releasing of the 
space (see other thread reserving an IOCTL number; other details ).
The disadvantage is that it should be slower than the currently 
implementation.
Of course I don’t want to remove the existing interface. I want only to add 
another one.

Comments ? Thoughts ?
Regards
G.Baroncelli 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi all,

enclosed you can find a patch which permits to remove a volume via the 
rmdir(2) syscall by an ordinary user. 
The rules for a subvolume removal are the same ones of a directory:
- the user shall have the write permission on the parent directory
- the subvolume shall be empty

Comments are welcome

Reagrds
G.Baroncelli

NB: this is code is not fully tested, handle with care.

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index f08427c..47d11d8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2944,6 +2944,86 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
+int may_destroy_subvol(struct btrfs_root *root);
+static noinline int btrfs_snap_destroy(struct inode *dir,
+				       struct dentry *dentry)
+
+{
+
+	struct inode *inode;
+	struct btrfs_root *root = BTRFS_I(dir)->root;
+	struct btrfs_root *dest = NULL;
+	struct btrfs_trans_handle *trans;
+
+	int ret;
+	int err = 0;
+
+
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
+		goto out;
+	}
+
+	if (!dentry->d_inode) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	inode = dentry->d_inode;
+	if (inode->i_ino != BTRFS_FIRST_FREE_OBJECTID) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	dest = BTRFS_I(inode)->root;
+
+	down_write(&root->fs_info->subvol_sem);
+
+	err = may_destroy_subvol(dest);
+	if (err)
+		goto out_up_write;
+
+	trans = btrfs_start_transaction(root, 0);
+	if (IS_ERR(trans)) {
+		err = PTR_ERR(trans);
+		goto out_up_write;
+	}
+	trans->block_rsv = &root->fs_info->global_block_rsv;
+
+	ret = btrfs_unlink_subvol(trans, root, dir,
+				dest->root_key.objectid,
+				dentry->d_name.name,
+				dentry->d_name.len);
+	BUG_ON(ret);
+
+	btrfs_record_root_in_trans(trans, dest);
+
+	memset(&dest->root_item.drop_progress, 0,
+		sizeof(dest->root_item.drop_progress));
+	dest->root_item.drop_level = 0;
+	btrfs_set_root_refs(&dest->root_item, 0);
+
+	if (!xchg(&dest->orphan_item_inserted, 1)) {
+		ret = btrfs_insert_orphan_item(trans,
+					root->fs_info->tree_root,
+					dest->root_key.objectid);
+		BUG_ON(ret);
+	}
+
+	ret = btrfs_commit_transaction(trans, root);
+	BUG_ON(ret);
+	inode->i_flags |= S_DEAD;
+out_up_write:
+	up_write(&root->fs_info->subvol_sem);
+	if (!err) {
+		shrink_dcache_sb(root->fs_info->sb);
+		btrfs_invalidate_inodes(dest);
+		/*d_delete(dentry);*/
+	}
+out:
+	return err;
+}
+
 static int btrfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
@@ -2952,10 +3032,12 @@ static int btrfs_rmdir(struct inode *dir, struct dentry
*dentry)
 	struct btrfs_trans_handle *trans;
 	unsigned long nr = 0;
 
-	if (inode->i_size > BTRFS_EMPTY_DIR_SIZE ||
-	    inode->i_ino == BTRFS_FIRST_FREE_OBJECTID)
+	if (inode->i_size > BTRFS_EMPTY_DIR_SIZE)
 		return -ENOTEMPTY;
 
+	if (inode->i_ino == BTRFS_FIRST_FREE_OBJECTID)
+		return btrfs_snap_destroy(dir, dentry);
+
 	trans = __unlink_start_trans(dir, dentry);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
@@ -4242,7 +4324,6 @@ static int btrfs_real_readdir(struct file *filp, void
*dirent,
 			over = filldir(dirent, name_ptr, name_len,
 				       found_key.offset, location.objectid,
 				       d_type);
-
 skip:
 			if (name_ptr != tmp_name)
 				kfree(name_ptr);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9254b3d..a7b242e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -855,7 +855,7 @@ out:
 /*
  * helper to check if the subvolume references other subvolumes
  */
-static noinline int may_destroy_subvol(struct btrfs_root *root)
+int may_destroy_subvol(struct btrfs_root *root)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;




-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo)
<kreijackATinwind.it>
Key fingerprint = 4769 7E51 5293 D36C 814E  C054 BF04 F161 3DC5 0512