[PATCH 0/4] Parameter cleanups for subvolume/snapshots ioctls

[PATCH 0/4] Parameter cleanups for subvolume/snapshots ioctls

[David Sterba]

Use qstr for name + length parameters, some other parameter type or name
adjustments.

David Sterba (4):
  btrfs: use struct qstr for subvolume ioctl helpers
  btrfs: pass dentry to btrfs_mksubvol() and btrfs_mksnapshot()
  btrfs: pass bool to indicate subvolume/snapshot creation type
  btrfs: rename inode number parameter passed to
    btrfs_check_dir_item_collision()

 fs/btrfs/dir-item.c |  4 ++--
 fs/btrfs/dir-item.h |  2 +-
 fs/btrfs/ioctl.c    | 37 +++++++++++++++++--------------------
 3 files changed, 20 insertions(+), 23 deletions(-)

-- 
2.49.0

[PATCH 1/4] btrfs: use struct qstr for subvolume ioctl helpers

[David Sterba]

We pass name and length of subvolumes separately to the related
functions, while this can be a struct qstr which is otherwise used for
dentry interfaces.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 5068f1fa86f632..092320cb467ef6 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -894,22 +894,21 @@ static inline int btrfs_may_create(struct mnt_idmap *idmap,
  */
 static noinline int btrfs_mksubvol(const struct path *parent,
 				   struct mnt_idmap *idmap,
-				   const char *name, int namelen,
-				   struct btrfs_root *snap_src,
+				   struct qstr *qname, struct btrfs_root *snap_src,
 				   bool readonly,
 				   struct btrfs_qgroup_inherit *inherit)
 {
 	struct inode *dir = d_inode(parent->dentry);
 	struct btrfs_fs_info *fs_info = inode_to_fs_info(dir);
 	struct dentry *dentry;
-	struct fscrypt_str name_str = FSTR_INIT((char *)name, namelen);
+	struct fscrypt_str name_str = FSTR_INIT((char *)qname->name, qname->len);
 	int ret;
 
 	ret = down_write_killable_nested(&dir->i_rwsem, I_MUTEX_PARENT);
 	if (ret == -EINTR)
 		return ret;
 
-	dentry = lookup_one(idmap, &QSTR_LEN(name, namelen), parent->dentry);
+	dentry = lookup_one(idmap, qname, parent->dentry);
 	ret = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto out_unlock;
@@ -949,7 +948,7 @@ static noinline int btrfs_mksubvol(const struct path *parent,
 
 static noinline int btrfs_mksnapshot(const struct path *parent,
 				   struct mnt_idmap *idmap,
-				   const char *name, int namelen,
+				   struct qstr *qname,
 				   struct btrfs_root *root,
 				   bool readonly,
 				   struct btrfs_qgroup_inherit *inherit)
@@ -976,8 +975,8 @@ static noinline int btrfs_mksnapshot(const struct path *parent,
 
 	btrfs_wait_ordered_extents(root, U64_MAX, NULL);
 
-	ret = btrfs_mksubvol(parent, idmap, name, namelen,
-			     root, readonly, inherit);
+	ret = btrfs_mksubvol(parent, idmap, qname, root, readonly, inherit);
+
 	atomic_dec(&root->snapshot_force_cow);
 out:
 	btrfs_drew_read_unlock(&root->snapshot_lock);
@@ -1187,8 +1186,8 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
 				bool readonly,
 				struct btrfs_qgroup_inherit *inherit)
 {
-	int namelen;
 	int ret = 0;
+	struct qstr qname = QSTR_INIT(name, strlen(name));
 
 	if (!S_ISDIR(file_inode(file)->i_mode))
 		return -ENOTDIR;
@@ -1197,21 +1196,20 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
 	if (ret)
 		goto out;
 
-	namelen = strlen(name);
 	if (strchr(name, '/')) {
 		ret = -EINVAL;
 		goto out_drop_write;
 	}
 
-	if (name[0] == '.' &&
-	   (namelen == 1 || (name[1] == '.' && namelen == 2))) {
+	if (qname.name[0] == '.' &&
+	   (qname.len == 1 || (qname.name[1] == '.' && qname.len == 2))) {
 		ret = -EEXIST;
 		goto out_drop_write;
 	}
 
 	if (subvol) {
-		ret = btrfs_mksubvol(&file->f_path, idmap, name,
-				     namelen, NULL, readonly, inherit);
+		ret = btrfs_mksubvol(&file->f_path, idmap, &qname, NULL,
+				     readonly, inherit);
 	} else {
 		CLASS(fd, src)(fd);
 		struct inode *src_inode;
@@ -1241,8 +1239,7 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
 			 */
 			ret = -EINVAL;
 		} else {
-			ret = btrfs_mksnapshot(&file->f_path, idmap,
-					       name, namelen,
+			ret = btrfs_mksnapshot(&file->f_path, idmap, &qname,
 					       BTRFS_I(src_inode)->root,
 					       readonly, inherit);
 		}
-- 
2.49.0

[PATCH 2/4] btrfs: pass dentry to btrfs_mksubvol() and btrfs_mksnapshot()

[David Sterba]

There's no reason to pass 'struct path' to btrfs_mksubvol(), though it's
been like the since the first commit 76dda93c6ae2c1 ("Btrfs: add
snapshot/subvolume destroy ioctl").  We only use the dentry so we should
pass it directly.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 092320cb467ef6..225fc00311a4c4 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -892,13 +892,13 @@ static inline int btrfs_may_create(struct mnt_idmap *idmap,
  * sys_mkdirat and vfs_mkdir, but we only do a single component lookup
  * inside this filesystem so it's quite a bit simpler.
  */
-static noinline int btrfs_mksubvol(const struct path *parent,
+static noinline int btrfs_mksubvol(struct dentry *parent,
 				   struct mnt_idmap *idmap,
 				   struct qstr *qname, struct btrfs_root *snap_src,
 				   bool readonly,
 				   struct btrfs_qgroup_inherit *inherit)
 {
-	struct inode *dir = d_inode(parent->dentry);
+	struct inode *dir = d_inode(parent);
 	struct btrfs_fs_info *fs_info = inode_to_fs_info(dir);
 	struct dentry *dentry;
 	struct fscrypt_str name_str = FSTR_INIT((char *)qname->name, qname->len);
@@ -908,7 +908,7 @@ static noinline int btrfs_mksubvol(const struct path *parent,
 	if (ret == -EINTR)
 		return ret;
 
-	dentry = lookup_one(idmap, qname, parent->dentry);
+	dentry = lookup_one(idmap, qname, parent);
 	ret = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto out_unlock;
@@ -946,7 +946,7 @@ static noinline int btrfs_mksubvol(const struct path *parent,
 	return ret;
 }
 
-static noinline int btrfs_mksnapshot(const struct path *parent,
+static noinline int btrfs_mksnapshot(struct dentry *parent,
 				   struct mnt_idmap *idmap,
 				   struct qstr *qname,
 				   struct btrfs_root *root,
@@ -1208,7 +1208,7 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
 	}
 
 	if (subvol) {
-		ret = btrfs_mksubvol(&file->f_path, idmap, &qname, NULL,
+		ret = btrfs_mksubvol(file_dentry(file), idmap, &qname, NULL,
 				     readonly, inherit);
 	} else {
 		CLASS(fd, src)(fd);
@@ -1239,7 +1239,7 @@ static noinline int __btrfs_ioctl_snap_create(struct file *file,
 			 */
 			ret = -EINVAL;
 		} else {
-			ret = btrfs_mksnapshot(&file->f_path, idmap, &qname,
+			ret = btrfs_mksnapshot(file_dentry(file), idmap, &qname,
 					       BTRFS_I(src_inode)->root,
 					       readonly, inherit);
 		}
-- 
2.49.0

[PATCH 3/4] btrfs: pass bool to indicate subvolume/snapshot creation type

[David Sterba]

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 225fc00311a4c4..8e90d4c9035e9c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1182,7 +1182,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
 
 static noinline int __btrfs_ioctl_snap_create(struct file *file,
 				struct mnt_idmap *idmap,
-				const char *name, unsigned long fd, int subvol,
+				const char *name, unsigned long fd, bool subvol,
 				bool readonly,
 				struct btrfs_qgroup_inherit *inherit)
 {
@@ -1276,7 +1276,7 @@ static noinline int btrfs_ioctl_snap_create(struct file *file,
 }
 
 static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
-					       void __user *arg, int subvol)
+					       void __user *arg, bool subvol)
 {
 	struct btrfs_ioctl_vol_args_v2 *vol_args;
 	int ret;
-- 
2.49.0

[PATCH 4/4] btrfs: rename inode number parameter passed to btrfs_check_dir_item_collision()

[David Sterba]

The name 'dir' is misleading as it's the inode number of the directory,
so rename it accordingly.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dir-item.c | 4 ++--
 fs/btrfs/dir-item.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index b29cc31a7c4a9d..69863e398e223f 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -227,7 +227,7 @@ struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
 	return di;
 }
 
-int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
+int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir_ino,
 				   const struct fscrypt_str *name)
 {
 	int ret;
@@ -242,7 +242,7 @@ int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
 	if (!path)
 		return -ENOMEM;
 
-	key.objectid = dir;
+	key.objectid = dir_ino;
 	key.type = BTRFS_DIR_ITEM_KEY;
 	key.offset = btrfs_name_hash(name->name, name->len);
 
diff --git a/fs/btrfs/dir-item.h b/fs/btrfs/dir-item.h
index 8462579a95f404..e52174a8baf92c 100644
--- a/fs/btrfs/dir-item.h
+++ b/fs/btrfs/dir-item.h
@@ -14,7 +14,7 @@ struct btrfs_inode;
 struct btrfs_root;
 struct btrfs_trans_handle;
 
-int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
+int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir_ino,
 			  const struct fscrypt_str *name);
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 			  const struct fscrypt_str *name, struct btrfs_inode *dir,
-- 
2.49.0

Re: [PATCH 0/4] Parameter cleanups for subvolume/snapshots ioctls

[Boris Burkov]

On Thu, Jun 26, 2025 at 04:30:08PM +0200, David Sterba wrote:
> Use qstr for name + length parameters, some other parameter type or name
> adjustments.

Reviewed-by: Boris Burkov <boris@bur.io>

> 
> David Sterba (4):
>   btrfs: use struct qstr for subvolume ioctl helpers
>   btrfs: pass dentry to btrfs_mksubvol() and btrfs_mksnapshot()
>   btrfs: pass bool to indicate subvolume/snapshot creation type
>   btrfs: rename inode number parameter passed to
>     btrfs_check_dir_item_collision()
> 
>  fs/btrfs/dir-item.c |  4 ++--
>  fs/btrfs/dir-item.h |  2 +-
>  fs/btrfs/ioctl.c    | 37 +++++++++++++++++--------------------
>  3 files changed, 20 insertions(+), 23 deletions(-)
> 
> -- 
> 2.49.0
>