[v3] btrfs-progs: add --subvol option to mkfs.btrfs

Commit Message

Mark Harmstone July 30, 2024, 2:21 p.m. UTC

This patch adds a --subvol option, which tells mkfs.btrfs to create the
specified directories as subvolumes.

Given a populated directory img, the command

$ mkfs.btrfs --rootdir img --subvol img/usr --subvol img/home --subvol img/home/username /dev/loop0

will create subvolumes usr and home within the FS root, and subvolume
username within the home subvolume. It will fail if any of the
directories do not yet exist.

Signed-off-by: Mark Harmstone <[e-mail]>
---
Changelog:

Patch 2:
* Rebased against upstream changes
* Rewrote so that directory sizes are correct within transactions
* Changed --subvol so that it is relative to cwd rather than rootdir, so
that in future we might allow out-of-tree subvols

Patch 3:
* Changed btrfs_mkfs_fill_dir so it doesn't start a transaction itself
* Moved subvol creation and linking into traverse_directory
* Removed depth calculation code, no longer needed

 mkfs/main.c                                 | 173 +++++++++++++++--
 mkfs/rootdir.c                              | 194 +++++++++++++++-----
 mkfs/rootdir.h                              |  14 +-
 tests/mkfs-tests/034-rootdir-subvol/test.sh |  33 ++++
 4 files changed, 360 insertions(+), 54 deletions(-)
 create mode 100755 tests/mkfs-tests/034-rootdir-subvol/test.sh

Comments

Qu Wenruo July 30, 2024, 9:40 p.m. UTC | #1

在 2024/7/30 23:51, Mark Harmstone 写道:
> This patch adds a --subvol option, which tells mkfs.btrfs to create the
> specified directories as subvolumes.
>
> Given a populated directory img, the command
>
> $ mkfs.btrfs --rootdir img --subvol img/usr --subvol img/home --subvol img/home/username /dev/loop0
>
> will create subvolumes usr and home within the FS root, and subvolume
> username within the home subvolume. It will fail if any of the
> directories do not yet exist.
>
> Signed-off-by: Mark Harmstone <[e-mail]>
[...]
> @@ -570,6 +626,76 @@ static int traverse_directory(struct btrfs_trans_handle *trans,
>   				pr_verbose(LOG_INFO, "ADD: %s\n", tmp);
>   			}
>
> +			if (S_ISDIR(st.st_mode)) {
> +				if (!strcmp(parent_dir_entry->dest_dir, "")) {
> +					strcpy(cur_dest, cur_file->d_name);
> +				} else {
> +					strcpy(cur_dest, parent_dir_entry->dest_dir);
> +					strcat(cur_dest, "/");
> +					strcat(cur_dest, cur_file->d_name);
> +				}
> +			}
> +
> +			if (!list_empty(subvols) && S_ISDIR(st.st_mode)) {
> +				struct rootdir_subvol *s;
> +				bool found = false;
> +
> +				list_for_each_entry(s, subvols, list) {
> +					if (!strcmp(cur_dest, s->destpath)) {
> +						found = true;
> +						break;
> +					}
> +				}
> +
> +				if (found) {

Although I'm still not a fan of merging subvolume creation now, before
we refactor the existing way of adding/linking new inodes, at least the
code style can be improved.

Under most case we prefer to reduce the indent by exiting early for
non-hit cases.

In this case, we prefer something like :

				if (!found)
					continue;
				/* do the subvolume add/link here */

Thanks,
Qu

Patch

diff --git a/mkfs/main.c b/mkfs/main.c
index e000aeff..72256816 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -442,6 +442,7 @@  static const char * const mkfs_usage[] = {
 	"Creation:",
 	OPTLINE("-b|--byte-count SIZE", "set size of each device to SIZE (filesystem size is sum of all device sizes)"),
 	OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory"),
+	OPTLINE("-u|--subvol SUBDIR", "create SUBDIR as subvolume rather than normal directory"),
 	OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"),
 	OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"),
 	OPTLINE("-f|--force", "force overwrite of existing filesystem"),
@@ -1057,6 +1058,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 	char *label = NULL;
 	int nr_global_roots = sysconf(_SC_NPROCESSORS_ONLN);
 	char *source_dir = NULL;
+	LIST_HEAD(subvols);
 
 	cpu_detect_flags();
 	hash_init_accel();
@@ -1087,6 +1089,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 			{ "data", required_argument, NULL, 'd' },
 			{ "version", no_argument, NULL, 'V' },
 			{ "rootdir", required_argument, NULL, 'r' },
+			{ "subvol", required_argument, NULL, 'u' },
 			{ "nodiscard", no_argument, NULL, 'K' },
 			{ "features", required_argument, NULL, 'O' },
 			{ "runtime-features", required_argument, NULL, 'R' },
@@ -1104,7 +1107,7 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 			{ NULL, 0, NULL, 0}
 		};
 
-		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:R:O:r:U:VvMKq",
+		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:R:O:r:U:VvMKqu:",
 				long_options, NULL);
 		if (c < 0)
 			break;
@@ -1210,6 +1213,23 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 				free(source_dir);
 				source_dir = strdup(optarg);
 				break;
+			case 'u': {
+				struct rootdir_subvol *s;
+
+				s = malloc(sizeof(struct rootdir_subvol));
+				if (!s) {
+					error("out of memory");
+					ret = 1;
+					goto error;
+				}
+
+				s->dir = strdup(optarg);
+				s->srcpath = NULL;
+				s->destpath = NULL;
+
+				list_add_tail(&s->list, &subvols);
+				break;
+				}
 			case 'U':
 				strncpy_null(fs_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE);
 				break;
@@ -1274,6 +1294,91 @@  int BOX_MAIN(mkfs)(int argc, char **argv)
 		ret = 1;
 		goto error;
 	}
+	if (!list_empty(&subvols) && source_dir == NULL) {
+		error("the option --subvol must be used with --rootdir");
+		ret = 1;
+		goto error;
+	}
+
+	if (source_dir) {
+		char *canonical = realpath(source_dir, NULL);
+
+		if (!canonical) {
+			error("could not get canonical path to %s", source_dir);
+			ret = 1;
+			goto error;
+		}
+
+		free(source_dir);
+		source_dir = canonical;
+	}
+
+	if (!list_empty(&subvols)) {
+		size_t source_dir_len = strlen(source_dir);
+		struct rootdir_subvol *s;
+
+		list_for_each_entry(s, &subvols, list) {
+			char *path;
+			struct rootdir_subvol *s2;
+
+			if (!path_exists(s->dir)) {
+				error("subvol %s does not exist",
+				      s->dir);
+				ret = 1;
+				goto error;
+			}
+
+			if (!path_is_dir(s->dir)) {
+				error("subvol %s is not a directory", s->dir);
+				ret = 1;
+				goto error;
+			}
+
+			path = realpath(s->dir, NULL);
+
+			if (!path) {
+				error("could not get canonical path to %s",
+				      s->dir);
+				ret = 1;
+				goto error;
+			}
+
+			s->srcpath = path;
+			s->srcpath_len = strlen(path);
+
+			if (s->srcpath_len >= source_dir_len + 1 &&
+			    !memcmp(path, source_dir, source_dir_len) &&
+			    path[source_dir_len] == '/') {
+				s->destpath_len = s->srcpath_len - source_dir_len - 1;
+
+				s->destpath = malloc(s->destpath_len + 1);
+				if (!s->destpath) {
+					error("out of memory");
+					ret = 1;
+					goto error;
+				}
+
+				memcpy(s->destpath, s->srcpath + source_dir_len + 1,
+				       s->destpath_len);
+				s->destpath[s->destpath_len] = 0;
+			} else {
+				error("subvol %s is not a child of %s",
+				      s->dir, source_dir);
+				ret = 1;
+				goto error;
+			}
+
+			for (s2 = list_first_entry(&subvols, struct rootdir_subvol, list);
+			     s2 != s; s2 = list_next_entry(s2, list)) {
+				if (!strcmp(s2->srcpath, path)) {
+					error("subvol %s specified more than once",
+					      s->dir);
+					ret = 1;
+					goto error;
+				}
+			}
+		}
+	}
 
 	if (*fs_uuid) {
 		uuid_t dummy_uuid;
@@ -1823,24 +1928,44 @@  raid_groups:
 		error_msg(ERROR_MSG_START_TRANS, "%m");
 		goto out;
 	}
-	ret = btrfs_rebuild_uuid_tree(fs_info);
-	if (ret < 0)
-		goto out;
-
-	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
-				  metadata_profile, metadata_profile);
-	if (ret < 0) {
-		error("failed to cleanup temporary chunks: %d", ret);
-		goto out;
-	}
 
 	if (source_dir) {
+		u64 next_subvol_id = BTRFS_FIRST_FREE_OBJECTID;
+
 		pr_verbose(LOG_DEFAULT, "Rootdir from:       %s\n", source_dir);
-		ret = btrfs_mkfs_fill_dir(source_dir, root);
+
+		trans = btrfs_start_transaction(root, 1);
+		if (IS_ERR(trans)) {
+			errno = -PTR_ERR(trans);
+			error_msg(ERROR_MSG_START_TRANS, "%m");
+			goto out;
+		}
+
+		ret = btrfs_mkfs_fill_dir(trans, source_dir, root,
+					  &subvols, "", &next_subvol_id);
 		if (ret) {
 			error("error while filling filesystem: %d", ret);
+			btrfs_commit_transaction(trans, root);
 			goto out;
 		}
+
+		ret = btrfs_commit_transaction(trans, root);
+		if (ret) {
+			errno = -ret;
+			error_msg(ERROR_MSG_COMMIT_TRANS, "%m");
+			goto out;
+		}
+
+		if (!list_empty(&subvols)) {
+			struct rootdir_subvol *s;
+
+			list_for_each_entry(s, &subvols, list) {
+				pr_verbose(LOG_DEFAULT,
+					   "  Subvol from:      %s -> %s\n",
+					   s->srcpath, s->destpath);
+			}
+		}
+
 		if (shrink_rootdir) {
 			pr_verbose(LOG_DEFAULT, "  Shrink:           yes\n");
 			ret = btrfs_mkfs_shrink_fs(fs_info, &shrink_size,
@@ -1855,6 +1980,17 @@  raid_groups:
 		}
 	}
 
+	ret = btrfs_rebuild_uuid_tree(fs_info);
+	if (ret < 0)
+		goto out;
+
+	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
+				  metadata_profile, metadata_profile);
+	if (ret < 0) {
+		error("failed to cleanup temporary chunks: %d", ret);
+		goto out;
+	}
+
 	if (features.runtime_flags & BTRFS_FEATURE_RUNTIME_QUOTA ||
 	    features.incompat_flags & BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA) {
 		ret = setup_quota_root(fs_info);
@@ -1948,6 +2084,19 @@  error:
 	free(label);
 	free(source_dir);
 
+	while (!list_empty(&subvols)) {
+		struct rootdir_subvol *head = list_entry(subvols.next,
+					      struct rootdir_subvol,
+					      list);
+
+		free(head->dir);
+		free(head->srcpath);
+		free(head->destpath);
+
+		list_del(&head->list);
+		free(head);
+	}
+
 	return !!ret;
 
 success:
diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index 2b39f6bb..981f106f 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -41,6 +41,7 @@ 
 #include "common/path-utils.h"
 #include "common/utils.h"
 #include "common/extent-tree-utils.h"
+#include "common/root-tree-utils.h"
 #include "mkfs/rootdir.h"
 
 static u32 fs_block_size;
@@ -184,17 +185,58 @@  static void free_namelist(struct dirent **files, int count)
 	free(files);
 }
 
-static u64 calculate_dir_inode_size(const char *dirname)
+static u64 calculate_dir_inode_size(const char *src_dir,
+				    struct list_head *subvols,
+				    const char *dest_dir)
 {
 	int count, i;
 	struct dirent **files, *cur_file;
 	u64 dir_inode_size = 0;
+	struct rootdir_subvol *s;
 
-	count = scandir(dirname, &files, directory_select, NULL);
+	count = scandir(src_dir, &files, directory_select, NULL);
 
 	for (i = 0; i < count; i++) {
+		size_t name_len;
+
 		cur_file = files[i];
-		dir_inode_size += strlen(cur_file->d_name);
+		name_len = strlen(cur_file->d_name);
+
+		/* Skip entry if it's going to be a subvol (it will be accounted
+		 * for in btrfs_link_subvolume). */
+		if (cur_file->d_type == DT_DIR && !list_empty(subvols)) {
+			bool skip = false;
+
+			list_for_each_entry(s, subvols, list) {
+				if (!strcmp(dest_dir, "")) {
+					if (!strcmp(cur_file->d_name, s->destpath)) {
+						skip = true;
+						break;
+					}
+				} else {
+					if (s->destpath_len != strlen(dest_dir) + 1 +
+					    name_len)
+						continue;
+
+					if (memcmp(s->destpath, dest_dir, strlen(dest_dir)))
+						continue;
+
+					if (s->destpath[strlen(dest_dir)] != '/')
+						continue;
+
+					if (!memcmp(s->destpath + strlen(dest_dir) + 1,
+						    cur_file->d_name, name_len)) {
+						skip = true;
+						break;
+					}
+				}
+			}
+
+			if (skip)
+				continue;
+		}
+
+		dir_inode_size += name_len;
 	}
 
 	free_namelist(files, count);
@@ -207,7 +249,9 @@  static int add_inode_items(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root,
 			   struct stat *st, const char *name,
 			   u64 self_objectid,
-			   struct btrfs_inode_item *inode_ret)
+			   struct btrfs_inode_item *inode_ret,
+			   struct list_head *subvols,
+			   const char *dest_dir)
 {
 	int ret;
 	struct btrfs_inode_item btrfs_inode;
@@ -218,7 +262,7 @@  static int add_inode_items(struct btrfs_trans_handle *trans,
 	objectid = self_objectid;
 
 	if (S_ISDIR(st->st_mode)) {
-		inode_size = calculate_dir_inode_size(name);
+		inode_size = calculate_dir_inode_size(name, subvols, dest_dir);
 		btrfs_set_stack_inode_size(&btrfs_inode, inode_size);
 	}
 
@@ -430,7 +474,8 @@  end:
 }
 
 static int copy_rootdir_inode(struct btrfs_trans_handle *trans,
-			      struct btrfs_root *root, const char *dir_name)
+			      struct btrfs_root *root, const char *dir_name,
+			      struct list_head *subvols, const char *dest_dir)
 {
 	u64 root_dir_inode_size;
 	struct btrfs_inode_item *inode_item;
@@ -468,7 +513,8 @@  static int copy_rootdir_inode(struct btrfs_trans_handle *trans,
 	leaf = path.nodes[0];
 	inode_item = btrfs_item_ptr(leaf, path.slots[0], struct btrfs_inode_item);
 
-	root_dir_inode_size = calculate_dir_inode_size(dir_name);
+	root_dir_inode_size = calculate_dir_inode_size(dir_name, subvols,
+						       dest_dir);
 	btrfs_set_inode_size(leaf, inode_item, root_dir_inode_size);
 
 	/* Unlike fill_inode_item, we only need to copy part of the attributes. */
@@ -493,7 +539,9 @@  error:
 
 static int traverse_directory(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root, const char *dir_name,
-			      struct directory_name_entry *dir_head)
+			      struct directory_name_entry *dir_head,
+			      struct list_head *subvols, const char *dest_dir,
+			      u64 *next_subvol_id)
 {
 	int ret = 0;
 
@@ -519,12 +567,20 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 		goto fail_no_dir;
 	}
 
+	dir_entry->dest_dir = strdup(dest_dir);
+	if (!dir_entry->dest_dir) {
+		error_msg(ERROR_MSG_MEMORY, NULL);
+		ret = -ENOMEM;
+		goto fail_no_dir;
+	}
+
 	parent_inum = highest_inum + BTRFS_FIRST_FREE_OBJECTID;
 	dir_entry->inum = parent_inum;
 	list_add_tail(&dir_entry->list, &dir_head->list);
 
-	ret = copy_rootdir_inode(trans, root, dir_name);
+	ret = copy_rootdir_inode(trans, root, dir_name, subvols, dest_dir);
 	if (ret < 0) {
+		free(dir_entry->dest_dir);
 		errno = -ret;
 		error("failed to copy rootdir inode: %m");
 		goto fail_no_dir;
@@ -555,7 +611,7 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 		}
 
 		for (i = 0; i < count; i++) {
-			char tmp[PATH_MAX];
+			char tmp[PATH_MAX], cur_dest[PATH_MAX];
 
 			cur_file = files[i];
 
@@ -570,6 +626,76 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 				pr_verbose(LOG_INFO, "ADD: %s\n", tmp);
 			}
 
+			if (S_ISDIR(st.st_mode)) {
+				if (!strcmp(parent_dir_entry->dest_dir, "")) {
+					strcpy(cur_dest, cur_file->d_name);
+				} else {
+					strcpy(cur_dest, parent_dir_entry->dest_dir);
+					strcat(cur_dest, "/");
+					strcat(cur_dest, cur_file->d_name);
+				}
+			}
+
+			if (!list_empty(subvols) && S_ISDIR(st.st_mode)) {
+				struct rootdir_subvol *s;
+				bool found = false;
+
+				list_for_each_entry(s, subvols, list) {
+					if (!strcmp(cur_dest, s->destpath)) {
+						found = true;
+						break;
+					}
+				}
+
+				if (found) {
+					struct btrfs_key key;
+					struct btrfs_root *new_root;
+					u64 objectid;
+
+					objectid = *next_subvol_id;
+
+					ret = btrfs_make_subvolume(trans, objectid);
+					if (ret < 0) {
+						error("failed to create subvolume: %d",
+						      ret);
+						goto out;
+					}
+
+					key.objectid = objectid;
+					key.type = BTRFS_ROOT_ITEM_KEY;
+					key.offset = (u64)-1;
+
+					new_root = btrfs_read_fs_root(trans->fs_info, &key);
+					if (IS_ERR(new_root)) {
+						error("unable to fs read root: %lu",
+						      PTR_ERR(new_root));
+						goto out;
+					}
+
+					(*next_subvol_id)++;
+
+					ret = btrfs_mkfs_fill_dir(trans, s->srcpath,
+								  new_root, subvols,
+								  s->destpath, next_subvol_id);
+
+					if (ret)
+						goto out;
+
+					ret = btrfs_link_subvolume(trans, root,
+							parent_inum,
+							path_basename(s->destpath),
+							strlen(path_basename(s->destpath)),
+							new_root);
+					if (ret) {
+						error("unable to link subvolume %s",
+						      path_basename(s->destpath));
+						goto out;
+					}
+
+					continue;
+				}
+			}
+
 			/*
 			 * We can not directly use the source ino number,
 			 * as there is a chance that the ino is smaller than
@@ -589,7 +715,8 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 
 			ret = add_inode_items(trans, root, &st,
 					      cur_file->d_name, cur_inum,
-					      &cur_inode);
+					      &cur_inode, subvols,
+					      cur_dest);
 			if (ret == -EEXIST) {
 				if (st.st_nlink <= 1) {
 					error(
@@ -638,6 +765,15 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 					goto fail;
 				}
 				dir_entry->inum = cur_inum;
+
+				dir_entry->dest_dir = strdup(cur_dest);
+				if (!dir_entry->dest_dir) {
+					free(dir_entry->path);
+					error_msg(ERROR_MSG_MEMORY, NULL);
+					ret = -ENOMEM;
+					goto fail;
+				}
+
 				list_add_tail(&dir_entry->list,
 					      &dir_head->list);
 			} else if (S_ISREG(st.st_mode)) {
@@ -661,6 +797,7 @@  static int traverse_directory(struct btrfs_trans_handle *trans,
 		}
 
 		free_namelist(files, count);
+		free(parent_dir_entry->dest_dir);
 		free(parent_dir_entry->path);
 		free(parent_dir_entry);
 
@@ -680,10 +817,11 @@  fail_no_dir:
 	goto out;
 }
 
-int btrfs_mkfs_fill_dir(const char *source_dir, struct btrfs_root *root)
+int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir,
+			struct btrfs_root *root, struct list_head *subvols,
+			const char* dest_dir, u64 *next_subvol_id)
 {
 	int ret;
-	struct btrfs_trans_handle *trans;
 	struct stat root_st;
 	struct directory_name_entry dir_head;
 	struct directory_name_entry *dir_entry = NULL;
@@ -697,41 +835,15 @@  int btrfs_mkfs_fill_dir(const char *source_dir, struct btrfs_root *root)
 
 	INIT_LIST_HEAD(&dir_head.list);
 
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		errno = -ret;
-		error_msg(ERROR_MSG_START_TRANS, "%m");
-		goto fail;
-}
-
-	ret = traverse_directory(trans, root, source_dir, &dir_head);
+	ret = traverse_directory(trans, root, source_dir, &dir_head,
+				 subvols, dest_dir, next_subvol_id);
 	if (ret) {
 		error("unable to traverse directory %s: %d", source_dir, ret);
 		goto fail;
 	}
-	ret = btrfs_commit_transaction(trans, root);
-	if (ret) {
-		errno = -ret;
-		error_msg(ERROR_MSG_COMMIT_TRANS, "%m");
-		goto out;
-	}
 
 	return 0;
 fail:
-	/*
-	 * Since we don't have btrfs_abort_transaction() yet, uncommitted trans
-	 * will trigger a BUG_ON().
-	 *
-	 * However before mkfs is fully finished, the magic number is invalid,
-	 * so even we commit transaction here, the fs still can't be mounted.
-	 *
-	 * To do a graceful error out, here we commit transaction as a
-	 * workaround.
-	 * Since we have already hit some problem, the return value doesn't
-	 * matter now.
-	 */
-	btrfs_commit_transaction(trans, root);
 	while (!list_empty(&dir_head.list)) {
 		dir_entry = list_entry(dir_head.list.next,
 				       struct directory_name_entry, list);
diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h
index 8d5f6896..806c1d8e 100644
--- a/mkfs/rootdir.h
+++ b/mkfs/rootdir.h
@@ -33,10 +33,22 @@  struct directory_name_entry {
 	const char *dir_name;
 	char *path;
 	ino_t inum;
+	char *dest_dir;
 	struct list_head list;
 };
 
-int btrfs_mkfs_fill_dir(const char *source_dir, struct btrfs_root *root);
+struct rootdir_subvol {
+	struct list_head list;
+	char *dir;
+	char *srcpath;
+	size_t srcpath_len;
+	char *destpath;
+	size_t destpath_len;
+};
+
+int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir,
+			struct btrfs_root *root, struct list_head *subvols,
+			const char* dest_dir, u64 *next_subvol_id);
 u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size,
 			u64 meta_profile, u64 data_profile);
 int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
diff --git a/tests/mkfs-tests/034-rootdir-subvol/test.sh b/tests/mkfs-tests/034-rootdir-subvol/test.sh
new file mode 100755
index 00000000..ccd6893f
--- /dev/null
+++ b/tests/mkfs-tests/034-rootdir-subvol/test.sh
@@ -0,0 +1,33 @@ 
+#!/bin/bash
+# smoke test for mkfs.btrfs --subvol option
+
+source "$TEST_TOP/common" || exit
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+tmp=$(_mktemp_dir mkfs-rootdir)
+
+touch $tmp/foo
+mkdir $tmp/dir
+mkdir $tmp/dir/subvol
+touch $tmp/dir/subvol/bar
+
+run_check_mkfs_test_dev --rootdir "$tmp" --subvol "$tmp/dir/subvol"
+run_check $SUDO_HELPER "$TOP/btrfs" check "$TEST_DEV"
+
+run_check_mount_test_dev
+run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list "$TEST_MNT" | \
+	cut -d\  -f9 > "$tmp/output"
+run_check_umount_test_dev
+
+result=$(cat "$tmp/output")
+
+if [ "$result" != "dir/subvol" ]; then
+	_fail "dir/subvol not in subvolume list"
+fi
+
+rm -rf -- "$tmp"