diff mbox

[v2] btrfs-progs: subvolume functions reorg

Message ID 1458137673-464-1-git-send-email-anand.jain@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anand Jain March 16, 2016, 2:14 p.m. UTC
Makes few subvol related functions usable outside of subvol
command set.

This is a preparatory patch for encryption support. The changes
here are:

Created new file subvolume.c
Created btrfs_get_subvol_info() from parts of cmd_subvol_show()
Moved get_subvol_name() to subvolume.c
Moved test_issubvolume() to subvolume.c
Created subvolume.h to define the above functions.

Further we also need a new ioctl to get subvol info from the
kernel (instead of tree search ioctl) for non root users and
I am planning to add that in this new file subvolume.c.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---

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

 Makefile.in      |   2 +-
 cmds-qgroup.c    |   1 +
 cmds-send.c      |  12 +----
 cmds-subvolume.c | 102 +++++++------------------------------
 subvolume.c      | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 subvolume.h      |  20 ++++++++
 6 files changed, 193 insertions(+), 96 deletions(-)
 create mode 100644 subvolume.c
 create mode 100644 subvolume.h

Comments

David Sterba March 17, 2016, 2:19 p.m. UTC | #1
On Wed, Mar 16, 2016 at 10:14:33PM +0800, Anand Jain wrote:
> Makes few subvol related functions usable outside of subvol
> command set.
> 
> This is a preparatory patch for encryption support. The changes
> here are:
> 
> Created new file subvolume.c
> Created btrfs_get_subvol_info() from parts of cmd_subvol_show()
> Moved get_subvol_name() to subvolume.c
> Moved test_issubvolume() to subvolume.c
> Created subvolume.h to define the above functions.

Such helpers are in utils.[ch], eg. there's test_issubvolname.
get_subvol_name could be renamed,
factoring btrfs_get_subvol_info looks good.

So far the internal interfaces are not tidy, so everything goes to
utils. It should be easier for you if you need to do some code
rearrangements.
--
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
Anand Jain March 18, 2016, 8:54 a.m. UTC | #2
On 03/17/2016 10:19 PM, David Sterba wrote:
> On Wed, Mar 16, 2016 at 10:14:33PM +0800, Anand Jain wrote:
>> Makes few subvol related functions usable outside of subvol
>> command set.
>>
>> This is a preparatory patch for encryption support. The changes
>> here are:
>>
>> Created new file subvolume.c
>> Created btrfs_get_subvol_info() from parts of cmd_subvol_show()
>> Moved get_subvol_name() to subvolume.c
>> Moved test_issubvolume() to subvolume.c
>> Created subvolume.h to define the above functions.
>
> Such helpers are in utils.[ch], eg. there's test_issubvolname.

  You meant to move them/it to subvolume.c/h right ?

> get_subvol_name could be renamed,

  will do.

> factoring btrfs_get_subvol_info looks good.
>
> So far the internal interfaces are not tidy,
> so everything goes to utils.

  yes. very true.

> It should be easier for you if you need to do some code
> rearrangements.

  Sure. Thanks for suggesting. Will see what I can find.

- Anand

> --
> 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
>
--
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
David Sterba March 18, 2016, 1:31 p.m. UTC | #3
On Fri, Mar 18, 2016 at 04:54:24PM +0800, Anand Jain wrote:
> 
> 
> On 03/17/2016 10:19 PM, David Sterba wrote:
> > On Wed, Mar 16, 2016 at 10:14:33PM +0800, Anand Jain wrote:
> >> Makes few subvol related functions usable outside of subvol
> >> command set.
> >>
> >> This is a preparatory patch for encryption support. The changes
> >> here are:
> >>
> >> Created new file subvolume.c
> >> Created btrfs_get_subvol_info() from parts of cmd_subvol_show()
> >> Moved get_subvol_name() to subvolume.c
> >> Moved test_issubvolume() to subvolume.c
> >> Created subvolume.h to define the above functions.
> >
> > Such helpers are in utils.[ch], eg. there's test_issubvolname.
> 
>   You meant to move them/it to subvolume.c/h right ?

No, please keep using utils.[ch] for now.
--
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
Anand Jain March 21, 2016, 7:20 a.m. UTC | #4
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(-)
Anand Jain March 21, 2016, 7:24 a.m. UTC | #5
On 03/18/2016 09:31 PM, David Sterba wrote:
> On Fri, Mar 18, 2016 at 04:54:24PM +0800, Anand Jain wrote:
>>
>>
>> On 03/17/2016 10:19 PM, David Sterba wrote:
>>> On Wed, Mar 16, 2016 at 10:14:33PM +0800, Anand Jain wrote:
>>>> Makes few subvol related functions usable outside of subvol
>>>> command set.
>>>>
>>>> This is a preparatory patch for encryption support. The changes
>>>> here are:
>>>>
>>>> Created new file subvolume.c
>>>> Created btrfs_get_subvol_info() from parts of cmd_subvol_show()
>>>> Moved get_subvol_name() to subvolume.c
>>>> Moved test_issubvolume() to subvolume.c
>>>> Created subvolume.h to define the above functions.
>>>
>>> Such helpers are in utils.[ch], eg. there's test_issubvolname.
>>
>>    You meant to move them/it to subvolume.c/h right ?
>
> No, please keep using utils.[ch] for now.

Agreed.

V3 was sent out. (and now its a patch set of 6).

Thanks, Anand

--
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
David Sterba March 22, 2016, 4:11 p.m. UTC | #6
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.
--
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
diff mbox

Patch

diff --git a/Makefile.in b/Makefile.in
index dd306862aa91..46dd8d25ddfd 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -70,7 +70,7 @@  objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  extent-cache.o extent_io.o volumes.o utils.o repair.o \
 	  qgroup.o raid6.o free-space-cache.o list_sort.o props.o \
 	  ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
-	  inode.o file.o find-root.o free-space-tree.o help.o
+	  inode.o file.o find-root.o free-space-tree.o help.o subvolume.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o \
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index c4a0c6b05ce8..5f9c4fb66ca3 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -26,6 +26,7 @@ 
 #include "commands.h"
 #include "qgroup.h"
 #include "utils.h"
+#include "subvolume.h"
 
 static const char * const qgroup_cmd_group_usage[] = {
 	"btrfs qgroup <command> [options] <path>",
diff --git a/cmds-send.c b/cmds-send.c
index 3e34d75bb834..000fd5638039 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -43,6 +43,7 @@ 
 
 #include "send.h"
 #include "send-utils.h"
+#include "subvolume.h"
 
 static int g_verbose = 0;
 
@@ -335,17 +336,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/cmds-subvolume.c b/cmds-subvolume.c
index 02e1dec18ed2..e9d0a4a40ea1 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -37,6 +37,7 @@ 
 #include "utils.h"
 #include "btrfs-list.h"
 #include "utils.h"
+#include "subvolume.h"
 
 static int is_subvolume_cleaned(int fd, u64 subvolid)
 {
@@ -224,33 +225,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;
@@ -938,12 +912,11 @@  static int cmd_subvol_show(int argc, char **argv)
 	struct btrfs_list_filter_set *filter_set;
 	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);
 
@@ -957,57 +930,14 @@  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);
+	ret = btrfs_get_subvol_info(fullpath, &get_ri);
 	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) {
-		printf("%s is toplevel subvolume\n", fullpath);
-		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 */
@@ -1058,19 +988,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/subvolume.c b/subvolume.c
new file mode 100644
index 000000000000..866285e02611
--- /dev/null
+++ b/subvolume.c
@@ -0,0 +1,152 @@ 
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <libgen.h>
+#include <limits.h>
+#include <getopt.h>
+#include <uuid/uuid.h>
+#include <linux/magic.h>
+#include <sys/vfs.h>
+
+#include "kerncompat.h"
+#include "ioctl.h"
+#include "qgroup.h"
+
+#include "ctree.h"
+#include "commands.h"
+#include "utils.h"
+#include "btrfs-list.h"
+#include "utils.h"
+#include "subvolume.h"
+
+/*
+ * 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;
+}
+
+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;
+}
+
+/*
+ * fixme: remove the error being printed here, move it to the
+ * leaf function
+ */
+int btrfs_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) {
+		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) {
+		printf("%s is toplevel subvolume\n", fullpath);
+		goto out;
+	}
+
+	memset(get_ri, 0, sizeof(*get_ri));
+	get_ri->root_id = sv_id;
+
+	ret = btrfs_get_subvol(mntfd, get_ri);
+	if (ret) {
+		fprintf(stderr, "ERROR: can't find '%s'\n",
+			svpath);
+		goto out;
+	}
+
+out:
+	return ret;
+}
diff --git a/subvolume.h b/subvolume.h
new file mode 100644
index 000000000000..ceacf603e01c
--- /dev/null
+++ b/subvolume.h
@@ -0,0 +1,20 @@ 
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+#include "btrfs-list.h"
+
+int btrfs_get_subvol_info(char *fullpath, struct root_info *get_ri);
+int test_issubvolume(const char *path);
+char *get_subvol_name(char *mnt, char *full_path);