[PATCH 0/6 v3] btrfs-progs: subvolume functions reorg

[PATCH 0/6 v3] btrfs-progs: subvolume functions reorg

[Anand Jain]

We need subvolume helper functions easily accessible for features
around subvolume. This patch set is just a cleanup of subvolume
functions.

This is tested fine with fstests group subvol and subvol hand tests.

v3: Separate changes into 6 commits,
    Drops the idea of creating subvolume.c, instead update utils.c.
    Adds 1/5 a minor func mv and 3/5 remove of a duplicate func and
    6/6 rename of get_subvol_name as per review comment.

v2: Update commit log. Separate this patch from the encryption
    patch set.


Anand Jain (6):
  btrfs-progs: spatial rearrange subvolume functions together
  btrfs-progs: move test_issubvolume() to utils.c
  btrfs-progs: remove duplicate function __is_subvol()
  btrfs-progs: move get_subvol_name() to utils.c
  btrfs-progs: create get_subvol_info()
  btrfs-progs: rename get_subvol_name() to subvol_minus_mnt()

 cmds-send.c      |  15 +-----
 cmds-subvolume.c | 107 +++++++++----------------------------
 utils.c          | 157 ++++++++++++++++++++++++++++++++++++++++++++-----------
 utils.h          |   4 ++
 4 files changed, 157 insertions(+), 126 deletions(-)

-- 
2.7.0

>From 22a64f723b6563785276d27b0ae4f7b8ded6febe Mon Sep 17 00:00:00 2001
From: Anand Jain <anand.jain@oracle.com>
Date: Mon, 21 Mar 2016 13:55:10 +0800

Anand Jain (5):
  btrfs-progs: spatial rearrange subvolume functions together
  btrfs-progs: move test_issubvolume() to utils.c
  btrfs-progs: remove duplicate function __is_subvol()
  btrfs-progs: move get_subvol_name() to utils.c
  btrfs-progs: create get_subvol_info()

 cmds-send.c      |  11 ----
 cmds-subvolume.c | 101 +++++++------------------------------
 utils.c          | 151 +++++++++++++++++++++++++++++++++++++++++++------------
 utils.h          |   4 ++
 4 files changed, 141 insertions(+), 126 deletions(-)

-- 
2.7.0

[PATCH 1/6] btrfs-progs: spatial rearrange subvolume functions together

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/utils.c b/utils.c
index c0c564e2f69b..fb549ee923c7 100644
--- a/utils.c
+++ b/utils.c
@@ -2784,17 +2784,6 @@ int test_minimum_size(const char *file, u32 nodesize)
 	return 0;
 }
 
-/*
- * test if name is a correct subvolume name
- * this function return
- * 0-> name is not a correct subvolume name
- * 1-> name is a correct subvolume name
- */
-int test_issubvolname(const char *name)
-{
-	return name[0] != '\0' && !strchr(name, '/') &&
-		strcmp(name, ".") && strcmp(name, "..");
-}
 
 /*
  * Test if path is a directory
@@ -3146,3 +3135,16 @@ void clean_args_no_options(int argc, char *argv[], const char * const *usagestr)
 		}
 	}
 }
+
+/* Subvolume helper functions */
+/*
+ * test if name is a correct subvolume name
+ * this function return
+ * 0-> name is not a correct subvolume name
+ * 1-> name is a correct subvolume name
+ */
+int test_issubvolname(const char *name)
+{
+	return name[0] != '\0' && !strchr(name, '/') &&
+		strcmp(name, ".") && strcmp(name, "..");
+}
-- 
2.7.0

[PATCH 2/6] btrfs-progs: move test_issubvolume() to utils.c

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c | 27 ---------------------------
 utils.c          | 27 +++++++++++++++++++++++++++
 utils.h          |  1 +
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 3953d7c060a2..f62ab9584ce7 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -224,33 +224,6 @@ out:
 	return retval;
 }
 
-/*
- * Test if path is a subvolume
- * Returns:
- *   0 - path exists but it is not a subvolume
- *   1 - path exists and it is  a subvolume
- * < 0 - error
- */
-int test_issubvolume(const char *path)
-{
-	struct stat	st;
-	struct statfs stfs;
-	int		res;
-
-	res = stat(path, &st);
-	if (res < 0)
-		return -errno;
-
-	if (st.st_ino != BTRFS_FIRST_FREE_OBJECTID || !S_ISDIR(st.st_mode))
-		return 0;
-
-	res = statfs(path, &stfs);
-	if (res < 0)
-		return -errno;
-
-	return (int)stfs.f_type == BTRFS_SUPER_MAGIC;
-}
-
 static int wait_for_commit(int fd)
 {
 	int ret;
diff --git a/utils.c b/utils.c
index fb549ee923c7..046ddf8eef19 100644
--- a/utils.c
+++ b/utils.c
@@ -3148,3 +3148,30 @@ int test_issubvolname(const char *name)
 	return name[0] != '\0' && !strchr(name, '/') &&
 		strcmp(name, ".") && strcmp(name, "..");
 }
+
+/*
+ * Test if path is a subvolume
+ * Returns:
+ *   0 - path exists but it is not a subvolume
+ *   1 - path exists and it is  a subvolume
+ * < 0 - error
+ */
+int test_issubvolume(const char *path)
+{
+	struct stat	st;
+	struct statfs stfs;
+	int		res;
+
+	res = stat(path, &st);
+	if (res < 0)
+		return -errno;
+
+	if (st.st_ino != BTRFS_FIRST_FREE_OBJECTID || !S_ISDIR(st.st_mode))
+		return 0;
+
+	res = statfs(path, &stfs);
+	if (res < 0)
+		return -errno;
+
+	return (int)stfs.f_type == BTRFS_SUPER_MAGIC;
+}
diff --git a/utils.h b/utils.h
index a4e55bf58914..32bb02005719 100644
--- a/utils.h
+++ b/utils.h
@@ -194,6 +194,7 @@ const char* group_profile_str(u64 flags);
 
 int test_minimum_size(const char *file, u32 leafsize);
 int test_issubvolname(const char *name);
+int test_issubvolume(const char *path);
 int test_isdir(const char *path);
 
 /*
-- 
2.7.0

[PATCH 3/6] btrfs-progs: remove duplicate function __is_subvol()

[Anand Jain]

The function test_issubvolume() provides the same check, and
has better logic.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/utils.c b/utils.c
index 046ddf8eef19..110a4badb764 100644
--- a/utils.c
+++ b/utils.c
@@ -1864,25 +1864,6 @@ int set_label(const char *btrfs_dev, const char *label)
 }
 
 /*
- * Unsafe subvolume check.
- *
- * This only checks ino == BTRFS_FIRST_FREE_OBJECTID, even it is not in a
- * btrfs mount point.
- * Must use together with other reliable method like btrfs ioctl.
- */
-static int __is_subvol(const char *path)
-{
-	struct stat st;
-	int ret;
-
-	ret = lstat(path, &st);
-	if (ret < 0)
-		return ret;
-
-	return st.st_ino == BTRFS_FIRST_FREE_OBJECTID;
-}
-
-/*
  * A not-so-good version fls64. No fascinating optimization since
  * no one except parse_size use it
  */
@@ -2002,7 +1983,7 @@ u64 parse_qgroupid(const char *p)
 
 path:
 	/* Path format like subv at 'my_subvol' is the fallback case */
-	ret = __is_subvol(p);
+	ret = test_issubvolume(p);
 	if (ret < 0 || !ret)
 		goto err;
 	fd = open(p, O_RDONLY);
-- 
2.7.0

[PATCH 4/6] btrfs-progs: move get_subvol_name() to utils.c

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-send.c | 11 -----------
 utils.c     | 12 ++++++++++++
 utils.h     |  1 +
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/cmds-send.c b/cmds-send.c
index 3e34d75bb834..7605ed94cd64 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -335,17 +335,6 @@ out:
 	return ret;
 }
 
-char *get_subvol_name(char *mnt, char *full_path)
-{
-	int len = strlen(mnt);
-	if (!len)
-		return full_path;
-	if (mnt[len - 1] != '/')
-		len += 1;
-
-	return full_path + len;
-}
-
 static int init_root_path(struct btrfs_send *s, const char *subvol)
 {
 	int ret = 0;
diff --git a/utils.c b/utils.c
index 110a4badb764..9aae7df04902 100644
--- a/utils.c
+++ b/utils.c
@@ -3156,3 +3156,15 @@ int test_issubvolume(const char *path)
 
 	return (int)stfs.f_type == BTRFS_SUPER_MAGIC;
 }
+
+char *get_subvol_name(char *mnt, char *full_path)
+{
+	int len = strlen(mnt);
+	if (!len)
+		return full_path;
+
+	if (mnt[len - 1] != '/')
+		len += 1;
+
+	return full_path + len;
+}
diff --git a/utils.h b/utils.h
index 32bb02005719..08543ff278ee 100644
--- a/utils.h
+++ b/utils.h
@@ -195,6 +195,7 @@ const char* group_profile_str(u64 flags);
 int test_minimum_size(const char *file, u32 leafsize);
 int test_issubvolname(const char *name);
 int test_issubvolume(const char *path);
+char *get_subvol_name(char *mnt, char *full_path);
 int test_isdir(const char *path);
 
 /*
-- 
2.7.0

[PATCH 5/6] btrfs-progs: create get_subvol_info()

[Anand Jain]

get_subvol_info() is useful as we are adding more features around
subvolume. This function was inline with the function
cmd_subvol_show().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c | 80 ++++++++++++++++++--------------------------------------
 utils.c          | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h          |  2 ++
 3 files changed, 100 insertions(+), 55 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index f62ab9584ce7..2319684e1500 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -908,21 +908,21 @@ static const char * const cmd_subvol_show_usage[] = {
 static int cmd_subvol_show(int argc, char **argv)
 {
 	struct root_info get_ri;
-	struct btrfs_list_filter_set *filter_set;
+	struct btrfs_list_filter_set *filter_set = NULL;
 	char tstr[256];
 	char uuidparse[BTRFS_UUID_UNPARSED_SIZE];
-	char *fullpath = NULL, *svpath = NULL, *mnt = NULL;
+	char *fullpath = NULL;
 	char raw_prefix[] = "\t\t\t\t";
-	u64 sv_id;
-	int fd = -1, mntfd = -1;
+	int fd = -1;
 	int ret = 1;
-	DIR *dirstream1 = NULL, *dirstream2 = NULL;
+	DIR *dirstream1 = NULL;
 
 	clean_args_no_options(argc, argv, cmd_subvol_show_usage);
 
 	if (check_argc_exact(argc - optind, 1))
 		usage(cmd_subvol_show_usage);
 
+	memset(&get_ri, 0, sizeof(get_ri));
 	fullpath = realpath(argv[optind], NULL);
 	if (!fullpath) {
 		error("cannot find real path for '%s': %s",
@@ -930,57 +930,23 @@ static int cmd_subvol_show(int argc, char **argv)
 		goto out;
 	}
 
-	ret = test_issubvolume(fullpath);
-	if (ret < 0) {
-		error("cannot access subvolume %s: %s", fullpath,
-			strerror(-ret));
-		goto out;
-	}
-	if (!ret) {
-		error("not a subvolume: %s", fullpath);
-		ret = 1;
-		goto out;
-	}
-
-	ret = find_mount_root(fullpath, &mnt);
-	if (ret < 0) {
-		error("find_mount_root failed on '%s': %s",
-			fullpath, strerror(-ret));
-		goto out;
-	}
-	if (ret > 0) {
-		error("%s doesn't belong to btrfs mount point", fullpath);
-		goto out;
-	}
-	ret = 1;
-	svpath = get_subvol_name(mnt, fullpath);
-
-	fd = btrfs_open_dir(fullpath, &dirstream1, 1);
-	if (fd < 0)
-		goto out;
-
-	ret = btrfs_list_get_path_rootid(fd, &sv_id);
-	if (ret) {
-		error("can't get rootid for '%s'", fullpath);
-		goto out;
-	}
-
-	mntfd = btrfs_open_dir(mnt, &dirstream2, 1);
-	if (mntfd < 0)
-		goto out;
-
-	if (sv_id == BTRFS_FS_TREE_OBJECTID) {
+	ret = get_subvol_info(fullpath, &get_ri);
+	if (ret == 2) {
+		/*
+		 * Since the top level btrfs was given don't
+		 * take that as error
+		 */
 		printf("%s is toplevel subvolume\n", fullpath);
+		ret = 0;
 		goto out;
 	}
-
-	memset(&get_ri, 0, sizeof(get_ri));
-	get_ri.root_id = sv_id;
-
-	ret = btrfs_get_subvol(mntfd, &get_ri);
 	if (ret) {
-		error("can't find '%s'", svpath);
-		goto out;
+		ret < 0 ?
+			error("Failed to get subvol info %s: %s\n",
+							fullpath, strerror(-ret)):
+			error("Failed to get subvol info %s: %d\n",
+							fullpath, ret);
+		return ret;
 	}
 
 	/* print the info */
@@ -1031,19 +997,23 @@ static int cmd_subvol_show(int argc, char **argv)
 	btrfs_list_setup_filter(&filter_set, BTRFS_LIST_FILTER_BY_PARENT,
 				(u64)(unsigned long)get_ri.uuid);
 	btrfs_list_setup_print_column(BTRFS_LIST_PATH);
+
+	fd = open_file_or_dir(fullpath, &dirstream1);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access '%s'\n", fullpath);
+		goto out;
+	}
 	btrfs_list_subvols_print(fd, filter_set, NULL, BTRFS_LIST_LAYOUT_RAW,
 			1, raw_prefix);
 
+out:
 	/* clean up */
 	free(get_ri.path);
 	free(get_ri.name);
 	free(get_ri.full_path);
 	btrfs_list_free_filter_set(filter_set);
 
-out:
 	close_file_or_dir(fd, dirstream1);
-	close_file_or_dir(mntfd, dirstream2);
-	free(mnt);
 	free(fullpath);
 	return !!ret;
 }
diff --git a/utils.c b/utils.c
index 9aae7df04902..e66314db21ed 100644
--- a/utils.c
+++ b/utils.c
@@ -3168,3 +3168,76 @@ char *get_subvol_name(char *mnt, char *full_path)
 
 	return full_path + len;
 }
+
+/*
+ * Returns
+ * <0: Std error
+ * 0: All fine
+ * 1: Error; and error info printed to the terminal. Fixme.
+ * 2: If the fullpath is root tree instead of subvol tree
+ */
+int get_subvol_info(char *fullpath, struct root_info *get_ri)
+{
+	u64 sv_id;
+	int ret = 1;
+	int fd = -1;
+	int mntfd = -1;
+	char *mnt = NULL;
+	char *svpath = NULL;
+	DIR *dirstream1 = NULL;
+	DIR *dirstream2 = NULL;
+
+	ret = test_issubvolume(fullpath);
+	if (ret < 0)
+		return ret;
+	if (!ret) {
+		error("not a subvolume: %s", fullpath);
+		return 1;
+	}
+
+	ret = find_mount_root(fullpath, &mnt);
+	if (ret < 0)
+		return ret;
+	if (ret > 0) {
+		error("%s doesn't belong to btrfs mount point", fullpath);
+		return 1;
+	}
+	ret = 1;
+	svpath = get_subvol_name(mnt, fullpath);
+
+	fd = btrfs_open_dir(fullpath, &dirstream1, 1);
+	if (fd < 0)
+		goto out;
+
+	ret = btrfs_list_get_path_rootid(fd, &sv_id);
+	if (ret) {
+		error("can't get rootid for '%s'", fullpath);
+		goto out;
+	}
+
+	mntfd = btrfs_open_dir(mnt, &dirstream2, 1);
+	if (mntfd < 0)
+		goto out;
+
+	if (sv_id == BTRFS_FS_TREE_OBJECTID) {
+		ret = 2;
+		/*
+		 * So that caller may decide if thats an error or just fine.
+		 */
+		goto out;
+	}
+
+	memset(get_ri, 0, sizeof(*get_ri));
+	get_ri->root_id = sv_id;
+
+	ret = btrfs_get_subvol(mntfd, get_ri);
+	if (ret)
+		error("can't find '%s': %d", svpath, ret);
+
+out:
+	close_file_or_dir(mntfd, dirstream2);
+	close_file_or_dir(mntfd, dirstream1);
+	free(mnt);
+
+	return ret;
+}
diff --git a/utils.h b/utils.h
index 08543ff278ee..843a1f4faa52 100644
--- a/utils.h
+++ b/utils.h
@@ -24,6 +24,7 @@
 #include <dirent.h>
 #include <stdarg.h>
 #include "internal.h"
+#include "btrfs-list.h"
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 #define BTRFS_MKFS_SMALL_VOLUME_SIZE (1024 * 1024 * 1024)
@@ -196,6 +197,7 @@ int test_minimum_size(const char *file, u32 leafsize);
 int test_issubvolname(const char *name);
 int test_issubvolume(const char *path);
 char *get_subvol_name(char *mnt, char *full_path);
+int get_subvol_info(char *fullpath, struct root_info *get_ri);
 int test_isdir(const char *path);
 
 /*
-- 
2.7.0

[PATCH 6/6] btrfs-progs: rename get_subvol_name() to subvol_minus_mnt()

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-send.c | 4 ++--
 utils.c     | 4 ++--
 utils.h     | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmds-send.c b/cmds-send.c
index 7605ed94cd64..339475927837 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -455,7 +455,7 @@ int cmd_send(int argc, char **argv)
 			if (ret < 0)
 				goto out;
 
-			ret = get_root_id(&send, get_subvol_name(send.root_path, subvol),
+			ret = get_root_id(&send, subvol_path_minus_mnt(send.root_path, subvol),
 					&root_id);
 			if (ret < 0) {
 				error("cannot resolve rootid for %s", subvol);
@@ -569,7 +569,7 @@ int cmd_send(int argc, char **argv)
 
 	if (snapshot_parent != NULL) {
 		ret = get_root_id(&send,
-				get_subvol_name(send.root_path, snapshot_parent),
+				subvol_path_minus_mnt(send.root_path, snapshot_parent),
 				&parent_root_id);
 		if (ret < 0) {
 			error("could not resolve rootid for %s", snapshot_parent);
diff --git a/utils.c b/utils.c
index e66314db21ed..965356df15c9 100644
--- a/utils.c
+++ b/utils.c
@@ -3157,7 +3157,7 @@ int test_issubvolume(const char *path)
 	return (int)stfs.f_type == BTRFS_SUPER_MAGIC;
 }
 
-char *get_subvol_name(char *mnt, char *full_path)
+char *subvol_path_minus_mnt(char *mnt, char *full_path)
 {
 	int len = strlen(mnt);
 	if (!len)
@@ -3203,7 +3203,7 @@ int get_subvol_info(char *fullpath, struct root_info *get_ri)
 		return 1;
 	}
 	ret = 1;
-	svpath = get_subvol_name(mnt, fullpath);
+	svpath = subvol_path_minus_mnt(mnt, fullpath);
 
 	fd = btrfs_open_dir(fullpath, &dirstream1, 1);
 	if (fd < 0)
diff --git a/utils.h b/utils.h
index 843a1f4faa52..abdd52095854 100644
--- a/utils.h
+++ b/utils.h
@@ -196,7 +196,7 @@ const char* group_profile_str(u64 flags);
 int test_minimum_size(const char *file, u32 leafsize);
 int test_issubvolname(const char *name);
 int test_issubvolume(const char *path);
-char *get_subvol_name(char *mnt, char *full_path);
+char *subvol_path_minus_mnt(char *mnt, char *full_path);
 int get_subvol_info(char *fullpath, struct root_info *get_ri);
 int test_isdir(const char *path);
 
-- 
2.7.0

Re: [PATCH 0/6 v3] btrfs-progs: subvolume functions reorg

[David Sterba]

On Mon, Mar 21, 2016 at 03:20:59PM +0800, Anand Jain wrote:
> We need subvolume helper functions easily accessible for features
> around subvolume. This patch set is just a cleanup of subvolume
> functions.
> 
> This is tested fine with fstests group subvol and subvol hand tests.
> 
> v3: Separate changes into 6 commits,
>     Drops the idea of creating subvolume.c, instead update utils.c.
>     Adds 1/5 a minor func mv and 3/5 remove of a duplicate func and
>     6/6 rename of get_subvol_name as per review comment.
> 
> v2: Update commit log. Separate this patch from the encryption
>     patch set.
> 
> 
> Anand Jain (6):
>   btrfs-progs: spatial rearrange subvolume functions together
>   btrfs-progs: move test_issubvolume() to utils.c
>   btrfs-progs: remove duplicate function __is_subvol()
>   btrfs-progs: move get_subvol_name() to utils.c
>   btrfs-progs: create get_subvol_info()
>   btrfs-progs: rename get_subvol_name() to subvol_minus_mnt()

Applied, thanks.