All of lore.kernel.org
 help / color / mirror / Atom feed
* [Request for review] [RFC] Add label support for snapshots and subvols
@ 2012-11-01 10:46 Anand jain
  2012-11-01 10:46 ` [PATCH 1/2] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
                   ` (8 more replies)
  0 siblings, 9 replies; 41+ messages in thread
From: Anand jain @ 2012-11-01 10:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Anand Jain

From: Anand Jain <anand.jain@oracle.com>

(This patch is for the review/test not yet for the integration).

 Here is an implementation of the feature to add label to the
 subvolume and snapshots. Which would help sysadmin to better manager
 the subvol and snapshots.

 This can be done in two ways, one - using attr which is user land
 only changes but drawback is able to change the label using the
 non btrfs cli. And the other way is to add a member to
 btrfs_root_item in the btrfs kernel to hold the label info for
 each snapshot and subvol. The drawback here is having to introduce
 V3 version of this structure. If there is any better way pls do share.
 The patch code is for the review.

Any comments/suggestion welcome.

Below is a demo of this new feature.
------------
 btrfs fi label -t /btrfs/sv1 "Prod-DB"

 btrfs fi label -t /btrfs/sv1
Prod-DB

 btrfs su snap /btrfs/sv1 /btrfs/snap1-sv1
Create a snapshot of '/btrfs/sv1' in '/btrfs/snap1-sv1'
 btrfs fi label -t /btrfs/snap1-sv1

 btrfs fi label -t /btrfs/snap1-sv1 "Prod-DB-sand-box-testing"
 
 btrfs fi label -t /btrfs/snap1-sv1
Prod-DB-sand-box-testing
----------------




Thanks, Anand
 

Anand Jain (2):
  Btrfs-progs: move open_file_or_dir() to utils.c
  Btrfs-progs: add feature to label subvol and snapshot

 Makefile          |    4 ++--
 btrfsctl.c        |    7 ++++---
 btrfslabel.c      |   40 ++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h      |    4 +++-
 cmds-balance.c    |    1 +
 cmds-filesystem.c |   34 +++++++++++++++++++++++++++++-----
 cmds-inspect.c    |    1 +
 cmds-qgroup.c     |    1 +
 cmds-quota.c      |    1 +
 cmds-subvolume.c  |    1 +
 commands.h        |    3 ---
 common.c          |   46 ----------------------------------------------
 ioctl.h           |    2 ++
 utils.c           |   30 ++++++++++++++++++++++++++++--
 utils.h           |    3 +++
 15 files changed, 116 insertions(+), 62 deletions(-)
 delete mode 100644 common.c


  Btrfs: add label to snapshot and subvol

 fs/btrfs/ctree.h       |   14 ++++++++++++++
 fs/btrfs/ioctl.c       |   32 ++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h       |    2 ++
 fs/btrfs/root-tree.c   |   44 +++++++++++++++++++++++---------------------
 fs/btrfs/transaction.c |    1 +
 5 files changed, 72 insertions(+), 21 deletions(-)



^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 1/2] Btrfs-progs: move open_file_or_dir() to utils.c
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
@ 2012-11-01 10:46 ` Anand jain
  2012-11-01 10:46 ` [PATCH 2/2] Btrfs-progs: add feature to label subvol and snapshot Anand jain
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-01 10:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Anand Jain

From: Anand Jain <anand.jain@oracle.com>

The definition of the function open_file_or_dir() is moved from common.c
to utils.c in order to be able to share some common code between scrub
and the device stats in the following step. That common code uses
open_file_or_dir(). Since open_file_or_dir() makes use of the function
dirfd(3), the required XOPEN version was raised from 6 to 7.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Original-Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |    1 +
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 11 files changed, 42 insertions(+), 56 deletions(-)
 delete mode 100644 common.c

diff --git a/Makefile b/Makefile
index 4894903..8576d90 100644
--- a/Makefile
+++ b/Makefile
@@ -41,8 +41,8 @@ all: version $(progs) manpages
 version:
 	bash version.sh
 
-btrfs: $(objects) btrfs.o help.o common.o $(cmds_objects)
-	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o common.o $(cmds_objects) \
+btrfs: $(objects) btrfs.o help.o $(cmds_objects)
+	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \
 		$(objects) $(LDFLAGS) $(LIBS) -lpthread
 
 calc-size: $(objects) calc-size.o
diff --git a/btrfsctl.c b/btrfsctl.c
index 518684c..049a5f3 100644
--- a/btrfsctl.c
+++ b/btrfsctl.c
@@ -63,7 +63,7 @@ static void print_usage(void)
 	exit(1);
 }
 
-static int open_file_or_dir(const char *fname)
+static int btrfsctl_open_file_or_dir(const char *fname)
 {
 	int ret;
 	struct stat st;
@@ -91,6 +91,7 @@ static int open_file_or_dir(const char *fname)
 	}
 	return fd;
 }
+
 int main(int ac, char **av)
 {
 	char *fname = NULL;
@@ -128,7 +129,7 @@ int main(int ac, char **av)
 			snap_location = strdup(fullpath);
 			snap_location = dirname(snap_location);
 
-			snap_fd = open_file_or_dir(snap_location);
+			snap_fd = btrfsctl_open_file_or_dir(snap_location);
 
 			name = strdup(fullpath);
 			name = basename(name);
@@ -238,7 +239,7 @@ int main(int ac, char **av)
 		}
 		name = fname;
 	 } else {
-		fd = open_file_or_dir(fname);
+		fd = btrfsctl_open_file_or_dir(fname);
 	 }
 
 	if (name) {
diff --git a/cmds-balance.c b/cmds-balance.c
index 38a7426..6268b61 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -28,6 +28,7 @@
 #include "volumes.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const balance_cmd_group_usage[] = {
 	"btrfs [filesystem] balance <command> [options] <path>",
diff --git a/cmds-inspect.c b/cmds-inspect.c
index edabff5..79e069b 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -22,6 +22,7 @@
 
 #include "kerncompat.h"
 #include "ioctl.h"
+#include "utils.h"
 
 #include "commands.h"
 #include "btrfs-list.h"
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 1525c11..cafc284 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -24,6 +24,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const qgroup_cmd_group_usage[] = {
 	"btrfs qgroup <command> [options] <path>",
diff --git a/cmds-quota.c b/cmds-quota.c
index cf9ad97..8481514 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -23,6 +23,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const quota_cmd_group_usage[] = {
 	"btrfs quota <command> [options] <path>",
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ac39f7b..e3cdb1e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -32,6 +32,7 @@
 #include "ctree.h"
 #include "commands.h"
 #include "btrfs-list.h"
+#include "utils.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
diff --git a/commands.h b/commands.h
index bb6d2dd..8114a73 100644
--- a/commands.h
+++ b/commands.h
@@ -79,9 +79,6 @@ void help_ambiguous_token(const char *arg, const struct cmd_group *grp);
 
 void help_command_group(const struct cmd_group *grp, int argc, char **argv);
 
-/* common.c */
-int open_file_or_dir(const char *fname);
-
 extern const struct cmd_group subvolume_cmd_group;
 extern const struct cmd_group filesystem_cmd_group;
 extern const struct cmd_group balance_cmd_group;
diff --git a/common.c b/common.c
deleted file mode 100644
index 03f6570..0000000
--- a/common.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * 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 <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
-#include <fcntl.h>
-
-int open_file_or_dir(const char *fname)
-{
-	int ret;
-	struct stat st;
-	DIR *dirstream;
-	int fd;
-
-	ret = stat(fname, &st);
-	if (ret < 0) {
-		return -1;
-	}
-	if (S_ISDIR(st.st_mode)) {
-		dirstream = opendir(fname);
-		if (!dirstream) {
-			return -2;
-		}
-		fd = dirfd(dirstream);
-	} else {
-		fd = open(fname, O_RDWR);
-	}
-	if (fd < 0) {
-		return -3;
-	}
-	return fd;
-}
diff --git a/utils.c b/utils.c
index 205e667..774f81d 100644
--- a/utils.c
+++ b/utils.c
@@ -16,8 +16,9 @@
  * Boston, MA 021110-1307, USA.
  */
 
-#define _XOPEN_SOURCE 600
-#define __USE_XOPEN2K
+#define _XOPEN_SOURCE 700
+#define __USE_XOPEN2K8
+#define __XOPEN2K8 /* due to an error in dirent.h, to get dirfd() */
 #include <stdio.h>
 #include <stdlib.h>
 #ifndef __CHECKER__
@@ -1220,3 +1221,28 @@ scan_again:
 	return 0;
 }
 
+int open_file_or_dir(const char *fname)
+{
+	int ret;
+	struct stat st;
+	DIR *dirstream;
+	int fd;
+
+	ret = stat(fname, &st);
+	if (ret < 0) {
+		return -1;
+	}
+	if (S_ISDIR(st.st_mode)) {
+		dirstream = opendir(fname);
+		if (!dirstream) {
+			return -2;
+		}
+		fd = dirfd(dirstream);
+	} else {
+		fd = open(fname, O_RDWR);
+	}
+	if (fd < 0) {
+		return -3;
+	}
+	return fd;
+}
diff --git a/utils.h b/utils.h
index 3a0368b..6975f10 100644
--- a/utils.h
+++ b/utils.h
@@ -19,6 +19,8 @@
 #ifndef __UTILS__
 #define __UTILS__
 
+#include "ctree.h"
+
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
 int make_btrfs(int fd, const char *device, const char *label,
@@ -46,4 +48,5 @@ int check_label(char *input);
 int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
+int open_file_or_dir(const char *fname);
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/2] Btrfs-progs: add feature to label subvol and snapshot
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
  2012-11-01 10:46 ` [PATCH 1/2] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
@ 2012-11-01 10:46 ` Anand jain
  2012-11-01 10:46 ` [PATCH] Btrfs: add label to snapshot and subvol Anand jain
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-01 10:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Anand Jain

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfslabel.c      |   40 ++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h      |    4 +++-
 cmds-filesystem.c |   34 +++++++++++++++++++++++++++++-----
 ioctl.h           |    2 ++
 4 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/btrfslabel.c b/btrfslabel.c
index cb142b0..90fe618 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -126,3 +126,43 @@ int set_label(char *btrfs_dev, char *nLabel)
 	change_label_unmounted(btrfs_dev, nLabel);
 	return 0;
 }
+
+int get_subvol_label(char *subvol)
+{
+	int fd, e=0;
+	char label[BTRFS_LABEL_SIZE+1];
+
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_GETLABEL, label) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	label[BTRFS_LABEL_SIZE] = '\0';
+	printf("%s\n",label);
+	close(fd);
+	return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+	char label[BTRFS_LABEL_SIZE];
+
+	fd = open_file_or_dir(subvol);
+
+	memset(label, 0, BTRFS_LABEL_SIZE);
+	strcpy(label, labelp);
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_SETLABEL, label) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	close(fd);	
+	return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..3db4180 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
 
 
 int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 9c43d35..718e70b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -22,6 +22,7 @@
 #include <errno.h>
 #include <uuid/uuid.h>
 #include <ctype.h>
+#include <sys/stat.h>
 
 #include "kerncompat.h"
 #include "ctree.h"
@@ -517,18 +518,41 @@ static int cmd_resize(int argc, char **argv)
 }
 
 static const char * const cmd_label_usage[] = {
-	"btrfs filesystem label <device> [<newlabel>]",
-	"Get or change the label of an unmounted filesystem",
-	"With one argument, get the label of filesystem on <device>.",
-	"If <newlabel> is passed, set the filesystem label to <newlabel>.",
+	"btrfs filesystem label [-t] <device>|<subvol> [<newlabel>]",
+	"Read or modify filesystem or subvol or snapshot label",
+	"-t  Read or modify label for the specified subvol or snapshot.",
 	NULL
 };
 
 static int cmd_label(int argc, char **argv)
 {
-	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+	struct stat st;
+	char subvol[BTRFS_PATH_NAME_MAX+1];
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
 		usage(cmd_label_usage);
 
+	if(getopt(argc, argv, "t:") != -1) {
+		if(optarg == NULL || strlen(optarg) > BTRFS_PATH_NAME_MAX)
+			return -1;
+		else {
+			strcpy(subvol,optarg);
+			if(stat(subvol, &st) < 0) {
+				fprintf(stderr, "Error: %s\n",strerror(errno));
+				return -errno;
+			}
+			if(!S_ISDIR(st.st_mode)) {
+				fprintf(stderr, "Error: Not a dir\n");
+				return -1;
+			}
+		}
+		if (argc > 3)
+			return set_subvol_label(argv[2], argv[3]);
+		else
+			return get_subvol_label(argv[2]);
+		return 0;
+	}
+
 	if (argc > 2)
 		return set_label(argv[1], argv[2]);
 	else
diff --git a/ioctl.h b/ioctl.h
index 6fda3a1..009fa6e 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -432,4 +432,6 @@ struct btrfs_ioctl_clone_range_args {
 					struct btrfs_ioctl_qgroup_create_args)
 #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
 					struct btrfs_ioctl_qgroup_limit_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH] Btrfs: add label to snapshot and subvol
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
  2012-11-01 10:46 ` [PATCH 1/2] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
  2012-11-01 10:46 ` [PATCH 2/2] Btrfs-progs: add feature to label subvol and snapshot Anand jain
@ 2012-11-01 10:46 ` Anand jain
  2012-11-05  7:39   ` Jan Schmidt
  2012-11-01 22:16 ` [Request for review] [RFC] Add label support for snapshots and subvols cwillu
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 41+ messages in thread
From: Anand jain @ 2012-11-01 10:46 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Anand Jain

From: Anand Jain <anand.jain@oracle.com>

This modifies the struct btrfs_root_item to hold the label,
and make it v3 of this structure.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ctree.h       |   14 ++++++++++++++
 fs/btrfs/ioctl.c       |   32 ++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h       |    2 ++
 fs/btrfs/root-tree.c   |   44 +++++++++++++++++++++++---------------------
 fs/btrfs/transaction.c |    1 +
 5 files changed, 72 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 994d255..b280256 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -759,6 +759,10 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
+
+	__le64 generation_v3;
+	char label[BTRFS_LABEL_SIZE]; /* Add label to subvol */
+	
 	__le64 reserved[8]; /* for future */
 } __attribute__ ((__packed__));
 
@@ -2428,6 +2432,8 @@ BTRFS_SETGET_STACK_FUNCS(root_last_snapshot, struct btrfs_root_item,
 			 last_snapshot, 64);
 BTRFS_SETGET_STACK_FUNCS(root_generation_v2, struct btrfs_root_item,
 			 generation_v2, 64);
+BTRFS_SETGET_STACK_FUNCS(root_generation_v3, struct btrfs_root_item,
+			 generation_v3, 64);
 BTRFS_SETGET_STACK_FUNCS(root_ctransid, struct btrfs_root_item,
 			 ctransid, 64);
 BTRFS_SETGET_STACK_FUNCS(root_otransid, struct btrfs_root_item,
@@ -2441,6 +2447,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
 {
 	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
 }
+static inline char * btrfs_root_label(struct btrfs_root *root)
+{
+	return (root->root_item.label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root *root, char *val)
+{
+	memcpy(root->root_item.label,val,BTRFS_LABEL_SIZE);
+}
 
 /* struct btrfs_root_backup */
 BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e58bd9d..cce0128 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3725,6 +3725,34 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
 	return 0;
 }
 
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char *label;
+
+	label = btrfs_root_label(root);
+	if (copy_to_user(arg, label, BTRFS_LABEL_SIZE))
+		return -EFAULT;
+	return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char label[BTRFS_LABEL_SIZE];
+	struct btrfs_trans_handle *trans;
+
+	if (copy_from_user(label, arg, BTRFS_LABEL_SIZE))
+		return -EFAULT;
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+	btrfs_root_set_label(root, label);
+	btrfs_commit_transaction(trans, root);
+
+	return 0;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -3827,6 +3855,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_label(root, argp);
 	case BTRFS_IOC_SET_LABEL:
 		return btrfs_ioctl_set_label(root, argp);
+	case BTRFS_IOC_SUBVOL_GETLABEL:
+		return btrfs_ioctl_subvol_getlabel(root, argp);
+	case BTRFS_IOC_SUBVOL_SETLABEL:
+		return btrfs_ioctl_subvol_setlabel(root, argp);
 	}
 
 	return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 0c60fcb..1009a0c 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
 #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index eb923d0..2a9ae5f 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -35,32 +35,34 @@ void btrfs_read_root_item(struct btrfs_root *root,
 {
 	uuid_le uuid;
 	int len;
-	int need_reset = 0;
 
 	len = btrfs_item_size_nr(eb, slot);
 	read_extent_buffer(eb, item, btrfs_item_ptr_offset(eb, slot),
 			min_t(int, len, (int)sizeof(*item)));
-	if (len < sizeof(*item))
-		need_reset = 1;
-	if (!need_reset && btrfs_root_generation(item)
-		!= btrfs_root_generation_v2(item)) {
-		if (btrfs_root_generation_v2(item) != 0) {
-			printk(KERN_WARNING "btrfs: mismatching "
-					"generation and generation_v2 "
-					"found in root item. This root "
-					"was probably mounted with an "
-					"older kernel. Resetting all "
-					"new fields.\n");
-		}
-		need_reset = 1;
-	}
-	if (need_reset) {
-		memset(&item->generation_v2, 0,
-			sizeof(*item) - offsetof(struct btrfs_root_item,
-					generation_v2));
 
-		uuid_le_gen(&uuid);
-		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
+	/* when len is gt sizeof(*item) that means its downgrade and
+	   the new members won't be touched anyways
+	   if len is lt sizeof(*item) its upgrade, we need to take
+	   care to set/reset newer members.
+	*/
+	if (len < sizeof(*item)) {
+		/* there is an upgrade need to set/reset the new members 
+		   assume v1 v2 v3 it can be upgrade from
+			v1 to v2 (set/reset only new members of v2)
+			v2 to v3 (set/reset only new members of v3)
+			v1 to v3 (set/reset new to v1)
+		*/
+		if (btrfs_root_generation(item) != btrfs_root_generation_v2(item)) {
+			//memset(&item->generation_v2, 0, 
+				//sizeof(*item) - offsetof(struct btrfs_root_item, generation_v2));
+			memset(&item->generation_v2, 0, sizeof(*item) - len);
+			uuid_le_gen(&uuid);
+			memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
+		} else if (btrfs_root_generation_v2(item) != btrfs_root_generation_v3(item)) {
+			//memset(&item->generation_v3, 0,
+			//	sizeof(*item) - offsetof(struct btrfs_root_item, generation_v3));
+			memset(&item->generation_v3, 0, sizeof(*item) - len);
+		}
 	}
 }
 
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 04bbfb1..45bff43 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1116,6 +1116,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	btrfs_set_root_otransid(new_root_item, trans->transid);
 	memset(&new_root_item->stime, 0, sizeof(new_root_item->stime));
 	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
+	memset(new_root_item->label, 0, BTRFS_LABEL_SIZE);
 	btrfs_set_root_stransid(new_root_item, 0);
 	btrfs_set_root_rtransid(new_root_item, 0);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (2 preceding siblings ...)
  2012-11-01 10:46 ` [PATCH] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-01 22:16 ` cwillu
  2012-11-01 22:28   ` Fajar A. Nugraha
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 41+ messages in thread
From: cwillu @ 2012-11-01 22:16 UTC (permalink / raw)
  To: Anand jain; +Cc: linux-btrfs

> Below is a demo of this new feature.
> ------------
>  btrfs fi label -t /btrfs/sv1 "Prod-DB"
>
>  btrfs fi label -t /btrfs/sv1
> Prod-DB
>
>  btrfs su snap /btrfs/sv1 /btrfs/snap1-sv1
> Create a snapshot of '/btrfs/sv1' in '/btrfs/snap1-sv1'
>  btrfs fi label -t /btrfs/snap1-sv1
>
>  btrfs fi label -t /btrfs/snap1-sv1 "Prod-DB-sand-box-testing"
>
>  btrfs fi label -t /btrfs/snap1-sv1
> Prod-DB-sand-box-testing

Why is this better than:

# btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
# mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
# ls /btrfs/
Prod-DB  Prod-DB-production-test

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 22:16 ` [Request for review] [RFC] Add label support for snapshots and subvols cwillu
@ 2012-11-01 22:28   ` Fajar A. Nugraha
  2012-11-01 22:32     ` Hugo Mills
  2012-11-01 23:04     ` Goffredo Baroncelli
  0 siblings, 2 replies; 41+ messages in thread
From: Fajar A. Nugraha @ 2012-11-01 22:28 UTC (permalink / raw)
  To: cwillu; +Cc: Anand jain, linux-btrfs

On Fri, Nov 2, 2012 at 5:16 AM, cwillu <cwillu@cwillu.com> wrote:
>>  btrfs fi label -t /btrfs/snap1-sv1
>> Prod-DB-sand-box-testing
>
> Why is this better than:
>
> # btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
> # mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
> # ls /btrfs/
> Prod-DB  Prod-DB-production-test


... because it would mean possibilty to decouple subvol name from
whatever-data-you-need (in this case, a label).

My request, though, is to just implement properties, and USER
properties, like what we have in zfs. This seems to be a cleaner,
saner approach. For example, this is on Ubutu + zfsonlinux:

# zfs create rpool/u
# zfs set user:label="Some test filesystem" rpool/u
# zfs get creation,user:label rpool/u
NAME     PROPERTY    VALUE                  SOURCE
rpool/u  creation    Fri Nov  2  5:24 2012  -
rpool/u  user:label  Some test filesystem   local

More info about zfs user properties here:
http://docs.oracle.com/cd/E19082-01/817-2271/gdrcw/index.html

--
Fajar

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 22:28   ` Fajar A. Nugraha
@ 2012-11-01 22:32     ` Hugo Mills
  2012-11-01 22:49       ` Fajar A. Nugraha
  2012-11-01 23:04     ` Goffredo Baroncelli
  1 sibling, 1 reply; 41+ messages in thread
From: Hugo Mills @ 2012-11-01 22:32 UTC (permalink / raw)
  To: Fajar A. Nugraha; +Cc: cwillu, Anand jain, linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Fri, Nov 02, 2012 at 05:28:01AM +0700, Fajar A. Nugraha wrote:
> On Fri, Nov 2, 2012 at 5:16 AM, cwillu <cwillu@cwillu.com> wrote:
> >>  btrfs fi label -t /btrfs/snap1-sv1
> >> Prod-DB-sand-box-testing
> >
> > Why is this better than:
> >
> > # btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
> > # mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
> > # ls /btrfs/
> > Prod-DB  Prod-DB-production-test
> 
> 
> ... because it would mean possibilty to decouple subvol name from
> whatever-data-you-need (in this case, a label).
> 
> My request, though, is to just implement properties, and USER
> properties, like what we have in zfs. This seems to be a cleaner,
> saner approach. For example, this is on Ubutu + zfsonlinux:
> 
> # zfs create rpool/u
> # zfs set user:label="Some test filesystem" rpool/u
> # zfs get creation,user:label rpool/u
> NAME     PROPERTY    VALUE                  SOURCE
> rpool/u  creation    Fri Nov  2  5:24 2012  -
> rpool/u  user:label  Some test filesystem   local

   Don't we already have an equivalent to that with user xattrs?

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
   --- I spent most of my money on drink, women and fast cars. The ---   
                      rest I wasted.  -- James Hunt                      

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 22:32     ` Hugo Mills
@ 2012-11-01 22:49       ` Fajar A. Nugraha
  2012-11-05  7:24         ` Anand Jain
  0 siblings, 1 reply; 41+ messages in thread
From: Fajar A. Nugraha @ 2012-11-01 22:49 UTC (permalink / raw)
  To: Hugo Mills, Fajar A. Nugraha, cwillu, Anand jain, linux-btrfs

On Fri, Nov 2, 2012 at 5:32 AM, Hugo Mills <hugo@carfax.org.uk> wrote:
> On Fri, Nov 02, 2012 at 05:28:01AM +0700, Fajar A. Nugraha wrote:
>> On Fri, Nov 2, 2012 at 5:16 AM, cwillu <cwillu@cwillu.com> wrote:
>> >>  btrfs fi label -t /btrfs/snap1-sv1
>> >> Prod-DB-sand-box-testing
>> >
>> > Why is this better than:
>> >
>> > # btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
>> > # mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
>> > # ls /btrfs/
>> > Prod-DB  Prod-DB-production-test
>>
>>
>> ... because it would mean possibilty to decouple subvol name from
>> whatever-data-you-need (in this case, a label).
>>
>> My request, though, is to just implement properties, and USER
>> properties, like what we have in zfs. This seems to be a cleaner,
>> saner approach. For example, this is on Ubutu + zfsonlinux:
>>
>> # zfs create rpool/u
>> # zfs set user:label="Some test filesystem" rpool/u
>> # zfs get creation,user:label rpool/u
>> NAME     PROPERTY    VALUE                  SOURCE
>> rpool/u  creation    Fri Nov  2  5:24 2012  -
>> rpool/u  user:label  Some test filesystem   local
>
>    Don't we already have an equivalent to that with user xattrs?
>
>    Hugo.


Anand did say one way to implement the label is by using attr, so +1
from me for that approach.

-- 
Fajar

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 22:28   ` Fajar A. Nugraha
  2012-11-01 22:32     ` Hugo Mills
@ 2012-11-01 23:04     ` Goffredo Baroncelli
  1 sibling, 0 replies; 41+ messages in thread
From: Goffredo Baroncelli @ 2012-11-01 23:04 UTC (permalink / raw)
  To: Fajar A. Nugraha; +Cc: cwillu, Anand jain, linux-btrfs

On 11/01/2012 11:28 PM, Fajar A. Nugraha wrote:
> On Fri, Nov 2, 2012 at 5:16 AM, cwillu <cwillu@cwillu.com> wrote:
>>>  btrfs fi label -t /btrfs/snap1-sv1
>>> Prod-DB-sand-box-testing
>>
>> Why is this better than:
>>
>> # btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
>> # mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
>> # ls /btrfs/
>> Prod-DB  Prod-DB-production-test
> 
> 
> ... because it would mean possibilty to decouple subvol name from
> whatever-data-you-need (in this case, a label).
> 

Could you elaborate how this solution is different from using xattr ?
I think also that these labels could be changed (like xattr). ?

These info should be associated to the inode of the subvolume root. We
could use a specific name in the system namespace, like
"system.btrfslabel" even tough I didn't see any advantage to using the
"user" namespace...


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-01 22:49       ` Fajar A. Nugraha
@ 2012-11-05  7:24         ` Anand Jain
  2012-11-05 23:22           ` David Sterba
  0 siblings, 1 reply; 41+ messages in thread
From: Anand Jain @ 2012-11-05  7:24 UTC (permalink / raw)
  To: Fajar A. Nugraha; +Cc: Hugo Mills, cwillu, linux-btrfs




  Thanks for all the comments and inputs.

  feature    xattr     btrfs-kernel-way
  [1]         No        Yes
  [2]         No        Yes
  [3]         Yes       No


  [1]. Ability to read subvol label without mount
  [2]. Full-ability to log and track the property
       when it is modified
  [3]. risk-free patch ?

  Any comments on why not the btrfs-kernel-way and a review
  of the patch ?

Thanks -Anand


On 11/02/2012 06:49 AM, Fajar A. Nugraha wrote:
> On Fri, Nov 2, 2012 at 5:32 AM, Hugo Mills <hugo@carfax.org.uk> wrote:
>> On Fri, Nov 02, 2012 at 05:28:01AM +0700, Fajar A. Nugraha wrote:
>>> On Fri, Nov 2, 2012 at 5:16 AM, cwillu <cwillu@cwillu.com> wrote:
>>>>>   btrfs fi label -t /btrfs/snap1-sv1
>>>>> Prod-DB-sand-box-testing
>>>>
>>>> Why is this better than:
>>>>
>>>> # btrfs su snap /btrfs/Prod-DB /btrfs/Prod-DB-sand-box-testing
>>>> # mv /btrfs/Prod-DB-sand-box-testing /btrfs/Prod-DB-production-test
>>>> # ls /btrfs/
>>>> Prod-DB  Prod-DB-production-test
>>>
>>>
>>> ... because it would mean possibilty to decouple subvol name from
>>> whatever-data-you-need (in this case, a label).
>>>
>>> My request, though, is to just implement properties, and USER
>>> properties, like what we have in zfs. This seems to be a cleaner,
>>> saner approach. For example, this is on Ubutu + zfsonlinux:
>>>
>>> # zfs create rpool/u
>>> # zfs set user:label="Some test filesystem" rpool/u
>>> # zfs get creation,user:label rpool/u
>>> NAME     PROPERTY    VALUE                  SOURCE
>>> rpool/u  creation    Fri Nov  2  5:24 2012  -
>>> rpool/u  user:label  Some test filesystem   local
>>
>>     Don't we already have an equivalent to that with user xattrs?
>>
>>     Hugo.
>
>
> Anand did say one way to implement the label is by using attr, so +1
> from me for that approach.
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH] Btrfs: add label to snapshot and subvol
  2012-11-01 10:46 ` [PATCH] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-05  7:39   ` Jan Schmidt
  2012-11-16  5:15     ` Anand Jain
  0 siblings, 1 reply; 41+ messages in thread
From: Jan Schmidt @ 2012-11-05  7:39 UTC (permalink / raw)
  To: Anand jain; +Cc: linux-btrfs

Hi Anand,

Some comments on your kernel patch from a quick jump through:

On 01.11.2012 11:46, Anand jain wrote:
> From: Anand Jain <anand.jain@oracle.com>
> 
> This modifies the struct btrfs_root_item to hold the label,
> and make it v3 of this structure.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/ctree.h       |   14 ++++++++++++++
>  fs/btrfs/ioctl.c       |   32 ++++++++++++++++++++++++++++++++
>  fs/btrfs/ioctl.h       |    2 ++
>  fs/btrfs/root-tree.c   |   44 +++++++++++++++++++++++---------------------
>  fs/btrfs/transaction.c |    1 +
>  5 files changed, 72 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 994d255..b280256 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -759,6 +759,10 @@ struct btrfs_root_item {
>  	struct btrfs_timespec otime;
>  	struct btrfs_timespec stime;
>  	struct btrfs_timespec rtime;
> +
> +	__le64 generation_v3;

What is that supposed to be?

> +	char label[BTRFS_LABEL_SIZE]; /* Add label to subvol */
> +	
>  	__le64 reserved[8]; /* for future */
>  } __attribute__ ((__packed__));

You can't do that. There's a reason why we've got reserved bytes for
future use. Never change the struct's size.

> @@ -2428,6 +2432,8 @@ BTRFS_SETGET_STACK_FUNCS(root_last_snapshot, struct btrfs_root_item,
>  			 last_snapshot, 64);
>  BTRFS_SETGET_STACK_FUNCS(root_generation_v2, struct btrfs_root_item,
>  			 generation_v2, 64);
> +BTRFS_SETGET_STACK_FUNCS(root_generation_v3, struct btrfs_root_item,
> +			 generation_v3, 64);
>  BTRFS_SETGET_STACK_FUNCS(root_ctransid, struct btrfs_root_item,
>  			 ctransid, 64);
>  BTRFS_SETGET_STACK_FUNCS(root_otransid, struct btrfs_root_item,
> @@ -2441,6 +2447,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>  {
>  	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>  }
> +static inline char * btrfs_root_label(struct btrfs_root *root)
> +{
> +	return (root->root_item.label);
> +}
> +static inline void btrfs_root_set_label(struct btrfs_root *root, char *val)
> +{
> +	memcpy(root->root_item.label,val,BTRFS_LABEL_SIZE);
> +}
>  
>  /* struct btrfs_root_backup */
>  BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index e58bd9d..cce0128 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3725,6 +3725,34 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>  	return 0;
>  }
>  
> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
> +						void __user *arg)
> +{
> +	char *label;
> +
> +	label = btrfs_root_label(root);
> +	if (copy_to_user(arg, label, BTRFS_LABEL_SIZE))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
> +						void __user *arg)
> +{
> +	char label[BTRFS_LABEL_SIZE];
> +	struct btrfs_trans_handle *trans;
> +
> +	if (copy_from_user(label, arg, BTRFS_LABEL_SIZE))
> +		return -EFAULT;
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans))
> +		return PTR_ERR(trans);
> +	btrfs_root_set_label(root, label);
> +	btrfs_commit_transaction(trans, root);

Why use start/commit instead of join/end here?

> +	return 0;
> +}
> +
>  long btrfs_ioctl(struct file *file, unsigned int
>  		cmd, unsigned long arg)
>  {
> @@ -3827,6 +3855,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_get_label(root, argp);
>  	case BTRFS_IOC_SET_LABEL:
>  		return btrfs_ioctl_set_label(root, argp);
> +	case BTRFS_IOC_SUBVOL_GETLABEL:
> +		return btrfs_ioctl_subvol_getlabel(root, argp);
> +	case BTRFS_IOC_SUBVOL_SETLABEL:
> +		return btrfs_ioctl_subvol_setlabel(root, argp);
>  	}
>  
>  	return -ENOTTY;
> diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
> index 0c60fcb..1009a0c 100644
> --- a/fs/btrfs/ioctl.h
> +++ b/fs/btrfs/ioctl.h
> @@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
>  				      struct btrfs_ioctl_get_dev_stats)
>  #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
>  #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
> +#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
> +#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
>  #endif
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index eb923d0..2a9ae5f 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -35,32 +35,34 @@ void btrfs_read_root_item(struct btrfs_root *root,
>  {
>  	uuid_le uuid;
>  	int len;
> -	int need_reset = 0;
>  
>  	len = btrfs_item_size_nr(eb, slot);
>  	read_extent_buffer(eb, item, btrfs_item_ptr_offset(eb, slot),
>  			min_t(int, len, (int)sizeof(*item)));
> -	if (len < sizeof(*item))
> -		need_reset = 1;
> -	if (!need_reset && btrfs_root_generation(item)
> -		!= btrfs_root_generation_v2(item)) {
> -		if (btrfs_root_generation_v2(item) != 0) {
> -			printk(KERN_WARNING "btrfs: mismatching "
> -					"generation and generation_v2 "
> -					"found in root item. This root "
> -					"was probably mounted with an "
> -					"older kernel. Resetting all "
> -					"new fields.\n");

Please argue why removing this without replacement should be a good idea.

> -		}
> -		need_reset = 1;
> -	}
> -	if (need_reset) {
> -		memset(&item->generation_v2, 0,
> -			sizeof(*item) - offsetof(struct btrfs_root_item,
> -					generation_v2));
>  
> -		uuid_le_gen(&uuid);
> -		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
> +	/* when len is gt sizeof(*item) that means its downgrade and
> +	   the new members won't be touched anyways
> +	   if len is lt sizeof(*item) its upgrade, we need to take
> +	   care to set/reset newer members.
> +	*/
> +	if (len < sizeof(*item)) {
> +		/* there is an upgrade need to set/reset the new members 
> +		   assume v1 v2 v3 it can be upgrade from
> +			v1 to v2 (set/reset only new members of v2)
> +			v2 to v3 (set/reset only new members of v3)
> +			v1 to v3 (set/reset new to v1)
> +		*/
> +		if (btrfs_root_generation(item) != btrfs_root_generation_v2(item)) {
> +			//memset(&item->generation_v2, 0, 
> +				//sizeof(*item) - offsetof(struct btrfs_root_item, generation_v2));
> +			memset(&item->generation_v2, 0, sizeof(*item) - len);
> +			uuid_le_gen(&uuid);
> +			memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
> +		} else if (btrfs_root_generation_v2(item) != btrfs_root_generation_v3(item)) {
> +			//memset(&item->generation_v3, 0,
> +			//	sizeof(*item) - offsetof(struct btrfs_root_item, generation_v3));
> +			memset(&item->generation_v3, 0, sizeof(*item) - len);
> +		}
>  	}
>  }
>  
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 04bbfb1..45bff43 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1116,6 +1116,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>  	btrfs_set_root_otransid(new_root_item, trans->transid);
>  	memset(&new_root_item->stime, 0, sizeof(new_root_item->stime));
>  	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
> +	memset(new_root_item->label, 0, BTRFS_LABEL_SIZE);
>  	btrfs_set_root_stransid(new_root_item, 0);
>  	btrfs_set_root_rtransid(new_root_item, 0);
>  
> 

-Jan

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Request for review] [RFC] Add label support for snapshots and subvols
  2012-11-05  7:24         ` Anand Jain
@ 2012-11-05 23:22           ` David Sterba
  0 siblings, 0 replies; 41+ messages in thread
From: David Sterba @ 2012-11-05 23:22 UTC (permalink / raw)
  To: Anand Jain; +Cc: Fajar A. Nugraha, Hugo Mills, cwillu, linux-btrfs

On Mon, Nov 05, 2012 at 03:24:48PM +0800, Anand Jain wrote:
>  feature    xattr     btrfs-kernel-way
>  [1]         No        Yes
>  [2]         No        Yes
>  [3]         Yes       No
> 
> 
>  [1]. Ability to read subvol label without mount

It is possible to read it offline, one can traverse the data structures
the same way as from kernel, ie root_tree -> subovlume fs_tree -> root
directory item -> xattr item.

>  [2]. Full-ability to log and track the property
>       when it is modified

What is expected to happen when the label changes? I understand that
somebody may change the xattr value silently, but let's say this is
changed through kernel -- do you intend to prohibit any changes or issue
some notification or whatever?

>  [3]. risk-free patch ?

No patch is risk free :) but yes, xattrs use an established and tested
infrastruture.

davdi

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [Request for review v2] [RFC] Add label support for snapshots and subvols
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (3 preceding siblings ...)
  2012-11-01 22:16 ` [Request for review] [RFC] Add label support for snapshots and subvols cwillu
@ 2012-11-16  4:52 ` Anand jain
  2012-11-16  4:52   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
                     ` (3 more replies)
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (3 subsequent siblings)
  8 siblings, 4 replies; 41+ messages in thread
From: Anand jain @ 2012-11-16  4:52 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

v1->v2:
	This v2 patch accepts the review comments on the btrfs
	kernel changes by Jan. and
	Moved the get and set subvol label to under subvol sub-cmd
	eg:
		btrfs subvolume label /btrfs/ss5
		btrfs su la /btrfs/ss5 "ss5-label"
		btrfs su la /btrfs/ss5
		ss5-label

v1:
 (This patch is for the review/test not yet for the integration).

 Here is an implementation of the feature to add label to the
 subvolume and snapshots. Which would help sysadmin to better manager
 the subvol and snapshots.

 This can be done in two ways, one - using attr which is user land
 only changes but drawback is able to change the label using the
 non btrfs cli. And the other way is to add a member to
 btrfs_root_item in the btrfs kernel to hold the label info for
 each snapshot and subvol. The drawback here is having to introduce
 V3 version of this structure. If there is any better way pls do share.
 The patch code is for the review.

Any comments/suggestion welcome.

Below is a demo of this new feature.
------------
 btrfs fi label -t /btrfs/sv1 "Prod-DB"

 btrfs fi label -t /btrfs/sv1
Prod-DB

 btrfs su snap /btrfs/sv1 /btrfs/snap1-sv1
Create a snapshot of '/btrfs/sv1' in '/btrfs/snap1-sv1'
 btrfs fi label -t /btrfs/snap1-sv1

 btrfs fi label -t /btrfs/snap1-sv1 "Prod-DB-sand-box-testing"
 
 btrfs fi label -t /btrfs/snap1-sv1
Prod-DB-sand-box-testing
----------------

Anand Jain (3):
  Btrfs-progs: move open_file_or_dir() to utils.c
  Btrfs-progs: add feature to label subvol and snapshot
  Btrfs-progs: cmd option to show or set the subvol label

 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 btrfslabel.c     |   45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h     |    4 +++-
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |   38 ++++++++++++++++++++++++++++++++++++++
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 ctree.h          |    4 +++-
 ioctl.h          |    2 ++
 man/btrfs.8.in   |    6 ++++++
 print-tree.c     |    2 ++
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 17 files changed, 140 insertions(+), 58 deletions(-)
 delete mode 100644 common.c


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
@ 2012-11-16  4:52   ` Anand jain
  2012-11-16  4:52   ` [PATCH v2] Btrfs: add label to snapshot and subvol Anand jain
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-16  4:52 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

The definition of the function open_file_or_dir() is moved from common.c
to utils.c in order to be able to share some common code between scrub
and the device stats in the following step. That common code uses
open_file_or_dir(). Since open_file_or_dir() makes use of the function
dirfd(3), the required XOPEN version was raised from 6 to 7.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Original-Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |    1 +
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 11 files changed, 42 insertions(+), 56 deletions(-)
 delete mode 100644 common.c

diff --git a/Makefile b/Makefile
index 4894903..8576d90 100644
--- a/Makefile
+++ b/Makefile
@@ -41,8 +41,8 @@ all: version $(progs) manpages
 version:
 	bash version.sh
 
-btrfs: $(objects) btrfs.o help.o common.o $(cmds_objects)
-	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o common.o $(cmds_objects) \
+btrfs: $(objects) btrfs.o help.o $(cmds_objects)
+	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \
 		$(objects) $(LDFLAGS) $(LIBS) -lpthread
 
 calc-size: $(objects) calc-size.o
diff --git a/btrfsctl.c b/btrfsctl.c
index 518684c..049a5f3 100644
--- a/btrfsctl.c
+++ b/btrfsctl.c
@@ -63,7 +63,7 @@ static void print_usage(void)
 	exit(1);
 }
 
-static int open_file_or_dir(const char *fname)
+static int btrfsctl_open_file_or_dir(const char *fname)
 {
 	int ret;
 	struct stat st;
@@ -91,6 +91,7 @@ static int open_file_or_dir(const char *fname)
 	}
 	return fd;
 }
+
 int main(int ac, char **av)
 {
 	char *fname = NULL;
@@ -128,7 +129,7 @@ int main(int ac, char **av)
 			snap_location = strdup(fullpath);
 			snap_location = dirname(snap_location);
 
-			snap_fd = open_file_or_dir(snap_location);
+			snap_fd = btrfsctl_open_file_or_dir(snap_location);
 
 			name = strdup(fullpath);
 			name = basename(name);
@@ -238,7 +239,7 @@ int main(int ac, char **av)
 		}
 		name = fname;
 	 } else {
-		fd = open_file_or_dir(fname);
+		fd = btrfsctl_open_file_or_dir(fname);
 	 }
 
 	if (name) {
diff --git a/cmds-balance.c b/cmds-balance.c
index 38a7426..6268b61 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -28,6 +28,7 @@
 #include "volumes.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const balance_cmd_group_usage[] = {
 	"btrfs [filesystem] balance <command> [options] <path>",
diff --git a/cmds-inspect.c b/cmds-inspect.c
index edabff5..79e069b 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -22,6 +22,7 @@
 
 #include "kerncompat.h"
 #include "ioctl.h"
+#include "utils.h"
 
 #include "commands.h"
 #include "btrfs-list.h"
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 1525c11..cafc284 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -24,6 +24,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const qgroup_cmd_group_usage[] = {
 	"btrfs qgroup <command> [options] <path>",
diff --git a/cmds-quota.c b/cmds-quota.c
index cf9ad97..8481514 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -23,6 +23,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const quota_cmd_group_usage[] = {
 	"btrfs quota <command> [options] <path>",
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ac39f7b..e3cdb1e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -32,6 +32,7 @@
 #include "ctree.h"
 #include "commands.h"
 #include "btrfs-list.h"
+#include "utils.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
diff --git a/commands.h b/commands.h
index bb6d2dd..8114a73 100644
--- a/commands.h
+++ b/commands.h
@@ -79,9 +79,6 @@ void help_ambiguous_token(const char *arg, const struct cmd_group *grp);
 
 void help_command_group(const struct cmd_group *grp, int argc, char **argv);
 
-/* common.c */
-int open_file_or_dir(const char *fname);
-
 extern const struct cmd_group subvolume_cmd_group;
 extern const struct cmd_group filesystem_cmd_group;
 extern const struct cmd_group balance_cmd_group;
diff --git a/common.c b/common.c
deleted file mode 100644
index 03f6570..0000000
--- a/common.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * 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 <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
-#include <fcntl.h>
-
-int open_file_or_dir(const char *fname)
-{
-	int ret;
-	struct stat st;
-	DIR *dirstream;
-	int fd;
-
-	ret = stat(fname, &st);
-	if (ret < 0) {
-		return -1;
-	}
-	if (S_ISDIR(st.st_mode)) {
-		dirstream = opendir(fname);
-		if (!dirstream) {
-			return -2;
-		}
-		fd = dirfd(dirstream);
-	} else {
-		fd = open(fname, O_RDWR);
-	}
-	if (fd < 0) {
-		return -3;
-	}
-	return fd;
-}
diff --git a/utils.c b/utils.c
index 205e667..774f81d 100644
--- a/utils.c
+++ b/utils.c
@@ -16,8 +16,9 @@
  * Boston, MA 021110-1307, USA.
  */
 
-#define _XOPEN_SOURCE 600
-#define __USE_XOPEN2K
+#define _XOPEN_SOURCE 700
+#define __USE_XOPEN2K8
+#define __XOPEN2K8 /* due to an error in dirent.h, to get dirfd() */
 #include <stdio.h>
 #include <stdlib.h>
 #ifndef __CHECKER__
@@ -1220,3 +1221,28 @@ scan_again:
 	return 0;
 }
 
+int open_file_or_dir(const char *fname)
+{
+	int ret;
+	struct stat st;
+	DIR *dirstream;
+	int fd;
+
+	ret = stat(fname, &st);
+	if (ret < 0) {
+		return -1;
+	}
+	if (S_ISDIR(st.st_mode)) {
+		dirstream = opendir(fname);
+		if (!dirstream) {
+			return -2;
+		}
+		fd = dirfd(dirstream);
+	} else {
+		fd = open(fname, O_RDWR);
+	}
+	if (fd < 0) {
+		return -3;
+	}
+	return fd;
+}
diff --git a/utils.h b/utils.h
index 3a0368b..6975f10 100644
--- a/utils.h
+++ b/utils.h
@@ -19,6 +19,8 @@
 #ifndef __UTILS__
 #define __UTILS__
 
+#include "ctree.h"
+
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
 int make_btrfs(int fd, const char *device, const char *label,
@@ -46,4 +48,5 @@ int check_label(char *input);
 int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
+int open_file_or_dir(const char *fname);
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2] Btrfs: add label to snapshot and subvol
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
  2012-11-16  4:52   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
@ 2012-11-16  4:52   ` Anand jain
  2012-11-16  6:33     ` Miao Xie
  2012-11-16  4:52   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
  2012-11-16  4:52   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  3 siblings, 1 reply; 41+ messages in thread
From: Anand jain @ 2012-11-16  4:52 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

v1->v2:
This adds a new member label in the btrfs_root_item struct,
which uses 32 bytes of the reserved 64 bytes. So that 
btrfs_root_item remains same.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ctree.h       |   12 +++++++++++-
 fs/btrfs/ioctl.c       |   32 ++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h       |    2 ++
 fs/btrfs/transaction.c |    1 +
 4 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 994d255..039e11c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -383,6 +383,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -759,7 +760,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-	__le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
@@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
 {
 	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
 }
+static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
+{
+	return (root_item->label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
+{
+	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
+}
 
 /* struct btrfs_root_backup */
 BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e58bd9d..aa4eb23 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3725,6 +3725,34 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
 	return 0;
 }
 
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char *label;
+
+	label = btrfs_root_label(&root->root_item);
+	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
+		return -EFAULT;
+	return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	struct btrfs_trans_handle *trans;
+
+	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE))
+		return -EFAULT;
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+	btrfs_root_set_label(&root->root_item, label);
+	btrfs_end_transaction(trans, root);
+
+	return 0;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -3827,6 +3855,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_label(root, argp);
 	case BTRFS_IOC_SET_LABEL:
 		return btrfs_ioctl_set_label(root, argp);
+	case BTRFS_IOC_SUBVOL_GETLABEL:
+		return btrfs_ioctl_subvol_getlabel(root, argp);
+	case BTRFS_IOC_SUBVOL_SETLABEL:
+		return btrfs_ioctl_subvol_setlabel(root, argp);
 	}
 
 	return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 0c60fcb..1009a0c 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
 #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 04bbfb1..0a3f8b3 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1118,6 +1118,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
 	btrfs_set_root_stransid(new_root_item, 0);
 	btrfs_set_root_rtransid(new_root_item, 0);
+	memset(&new_root_item->label, 0, BTRFS_SUBVOL_LABEL_SIZE);
 
 	old = btrfs_lock_root_node(root);
 	ret = btrfs_cow_block(trans, root, old, NULL, 0, &old);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
  2012-11-16  4:52   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
  2012-11-16  4:52   ` [PATCH v2] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-16  4:52   ` Anand jain
  2012-11-16  4:52   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-16  4:52 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfslabel.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h |    4 +++-
 ctree.h      |    4 +++-
 ioctl.h      |    2 ++
 print-tree.c |    2 ++
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/btrfslabel.c b/btrfslabel.c
index cb142b0..e161af1 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -126,3 +126,48 @@ int set_label(char *btrfs_dev, char *nLabel)
 	change_label_unmounted(btrfs_dev, nLabel);
 	return 0;
 }
+
+int get_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_GETLABEL, labelp) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	labelp[BTRFS_LABEL_SIZE] = '\0';
+	if (!strlen(labelp))
+		return -1;
+	close(fd);
+	return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+
+	if (strlen(labelp) > BTRFS_SUBVOL_LABEL_SIZE) {
+		fprintf(stderr, "ERROR: Subvol label is more than max length %d\n",
+			BTRFS_SUBVOL_LABEL_SIZE);
+		return -1;
+	}
+	memset(label, 0, BTRFS_SUBVOL_LABEL_SIZE+1);
+	strcpy(label, labelp);
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_SETLABEL, label) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	close(fd);
+	return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..1abc483 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
 
 
 int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol, char *labelp);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/ctree.h b/ctree.h
index 293b24f..68bae28 100644
--- a/ctree.h
+++ b/ctree.h
@@ -325,6 +325,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -702,7 +703,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-        __le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
diff --git a/ioctl.h b/ioctl.h
index 6fda3a1..009fa6e 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -432,4 +432,6 @@ struct btrfs_ioctl_clone_range_args {
 					struct btrfs_ioctl_qgroup_create_args)
 #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
 					struct btrfs_ioctl_qgroup_limit_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/print-tree.c b/print-tree.c
index 7c615dd..5979109 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -330,6 +330,8 @@ static void print_root(struct extent_buffer *leaf, int slot)
 				btrfs_root_stransid(&root_item),
 				btrfs_root_rtransid(&root_item));
 		}
+		if (strlen(root_item.label))
+			printf("\t\tlabel %s\n", root_item.label);
 	}
 	if (btrfs_root_refs(&root_item) == 0) {
 		struct btrfs_key drop_key;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
                     ` (2 preceding siblings ...)
  2012-11-16  4:52   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
@ 2012-11-16  4:52   ` Anand jain
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-16  4:52 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c |   37 +++++++++++++++++++++++++++++++++++++
 man/btrfs.8.in   |    6 ++++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e3cdb1e..759eade 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -33,6 +33,7 @@
 #include "commands.h"
 #include "btrfs-list.h"
 #include "utils.h"
+#include "btrfslabel.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
@@ -700,6 +701,41 @@ static int cmd_find_new(int argc, char **argv)
 	return 0;
 }
 
+static const char * const cmd_subvol_label_usage[] = {
+	"btrfs subvolume label <path> [label]",
+	"Show or set label for the subvol or snapshot",
+	NULL
+};
+
+static int cmd_subvol_label(int argc, char **argv)
+{
+	struct stat st;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	int ret;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+		usage(cmd_subvol_label_usage);
+
+	if (stat(argv[1], &st) < 0) {
+		fprintf(stderr, "Error: %s\n",strerror(errno));
+		return -errno;
+	}
+	if (!S_ISDIR(st.st_mode)) {
+		fprintf(stderr, "Error: Not a dir\n");
+		return -1;
+	}
+	if (argc > 2)
+		return set_subvol_label(argv[1], argv[2]);
+	else {
+		ret = get_subvol_label(argv[1], label);
+		if (ret)
+			return ret;
+		label[BTRFS_SUBVOL_LABEL_SIZE]=0;
+		printf("%s\n",label);
+	}
+	return 0;
+}
+
 const struct cmd_group subvolume_cmd_group = {
 	subvolume_cmd_group_usage, NULL, {
 		{ "create", cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 },
@@ -711,6 +747,7 @@ const struct cmd_group subvolume_cmd_group = {
 		{ "set-default", cmd_subvol_set_default,
 			cmd_subvol_set_default_usage, NULL, 0 },
 		{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
+		{ "label", cmd_subvol_label, cmd_subvol_label_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }
 	}
 };
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 9222580..aa225d9 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -17,6 +17,8 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBsubvolume get-default\fP\fI <path>\fP
 .PP
+\fBbtrfs\fP \fBsubvolume label\fP\fI <path> [label]\fP
+.PP
 \fBbtrfs\fP \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] \
 [-s \fIstart\fR] [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> \
 [<\fIfile\fR>|<\fIdir\fR>...]
@@ -160,6 +162,10 @@ Get the default subvolume of the filesystem \fI<path>\fR. The output format
 is similar to \fBsubvolume list\fR command.
 .TP
 
+\fBsubvolume label\fR\fI <path> [label]\fR
+Show or set \fI[label]\fR for the subvolume or the snapshot \fI<path>\fR.
+.TP
+
 \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] [-s \fIstart\fR] \
 [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> [<\fIfile\fR>|<\fIdir\fR>...]
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH] Btrfs: add label to snapshot and subvol
  2012-11-05  7:39   ` Jan Schmidt
@ 2012-11-16  5:15     ` Anand Jain
  0 siblings, 0 replies; 41+ messages in thread
From: Anand Jain @ 2012-11-16  5:15 UTC (permalink / raw)
  To: Jan Schmidt; +Cc: linux-btrfs


Hi Jan,

  Thanks for the review. Just posted the v2 patch for the
  btrfs kernel patch.
  Which, used the btrfs_root_item reserved space and
  used join_transaction

-Anand

On 05/11/12 15:39, Jan Schmidt wrote:
> Hi Anand,
>
> Some comments on your kernel patch from a quick jump through:
>
> On 01.11.2012 11:46, Anand jain wrote:
>> From: Anand Jain <anand.jain@oracle.com>
>>
>> This modifies the struct btrfs_root_item to hold the label,
>> and make it v3 of this structure.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/ctree.h       |   14 ++++++++++++++
>>   fs/btrfs/ioctl.c       |   32 ++++++++++++++++++++++++++++++++
>>   fs/btrfs/ioctl.h       |    2 ++
>>   fs/btrfs/root-tree.c   |   44 +++++++++++++++++++++++---------------------
>>   fs/btrfs/transaction.c |    1 +
>>   5 files changed, 72 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 994d255..b280256 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -759,6 +759,10 @@ struct btrfs_root_item {
>>   	struct btrfs_timespec otime;
>>   	struct btrfs_timespec stime;
>>   	struct btrfs_timespec rtime;
>> +
>> +	__le64 generation_v3;
>
> What is that supposed to be?
>
>> +	char label[BTRFS_LABEL_SIZE]; /* Add label to subvol */
>> +	
>>   	__le64 reserved[8]; /* for future */
>>   } __attribute__ ((__packed__));
>
> You can't do that. There's a reason why we've got reserved bytes for
> future use. Never change the struct's size.
>
>> @@ -2428,6 +2432,8 @@ BTRFS_SETGET_STACK_FUNCS(root_last_snapshot, struct btrfs_root_item,
>>   			 last_snapshot, 64);
>>   BTRFS_SETGET_STACK_FUNCS(root_generation_v2, struct btrfs_root_item,
>>   			 generation_v2, 64);
>> +BTRFS_SETGET_STACK_FUNCS(root_generation_v3, struct btrfs_root_item,
>> +			 generation_v3, 64);
>>   BTRFS_SETGET_STACK_FUNCS(root_ctransid, struct btrfs_root_item,
>>   			 ctransid, 64);
>>   BTRFS_SETGET_STACK_FUNCS(root_otransid, struct btrfs_root_item,
>> @@ -2441,6 +2447,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>>   {
>>   	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>>   }
>> +static inline char * btrfs_root_label(struct btrfs_root *root)
>> +{
>> +	return (root->root_item.label);
>> +}
>> +static inline void btrfs_root_set_label(struct btrfs_root *root, char *val)
>> +{
>> +	memcpy(root->root_item.label,val,BTRFS_LABEL_SIZE);
>> +}
>>
>>   /* struct btrfs_root_backup */
>>   BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index e58bd9d..cce0128 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3725,6 +3725,34 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>>   	return 0;
>>   }
>>
>> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
>> +						void __user *arg)
>> +{
>> +	char *label;
>> +
>> +	label = btrfs_root_label(root);
>> +	if (copy_to_user(arg, label, BTRFS_LABEL_SIZE))
>> +		return -EFAULT;
>> +	return 0;
>> +}
>> +
>> +static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
>> +						void __user *arg)
>> +{
>> +	char label[BTRFS_LABEL_SIZE];
>> +	struct btrfs_trans_handle *trans;
>> +
>> +	if (copy_from_user(label, arg, BTRFS_LABEL_SIZE))
>> +		return -EFAULT;
>> +	trans = btrfs_start_transaction(root, 1);
>> +	if (IS_ERR(trans))
>> +		return PTR_ERR(trans);
>> +	btrfs_root_set_label(root, label);
>> +	btrfs_commit_transaction(trans, root);
>
> Why use start/commit instead of join/end here?
>
>> +	return 0;
>> +}
>> +
>>   long btrfs_ioctl(struct file *file, unsigned int
>>   		cmd, unsigned long arg)
>>   {
>> @@ -3827,6 +3855,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>>   		return btrfs_ioctl_get_label(root, argp);
>>   	case BTRFS_IOC_SET_LABEL:
>>   		return btrfs_ioctl_set_label(root, argp);
>> +	case BTRFS_IOC_SUBVOL_GETLABEL:
>> +		return btrfs_ioctl_subvol_getlabel(root, argp);
>> +	case BTRFS_IOC_SUBVOL_SETLABEL:
>> +		return btrfs_ioctl_subvol_setlabel(root, argp);
>>   	}
>>
>>   	return -ENOTTY;
>> diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
>> index 0c60fcb..1009a0c 100644
>> --- a/fs/btrfs/ioctl.h
>> +++ b/fs/btrfs/ioctl.h
>> @@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
>>   				      struct btrfs_ioctl_get_dev_stats)
>>   #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
>>   #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
>> +#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
>> +#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
>>   #endif
>> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
>> index eb923d0..2a9ae5f 100644
>> --- a/fs/btrfs/root-tree.c
>> +++ b/fs/btrfs/root-tree.c
>> @@ -35,32 +35,34 @@ void btrfs_read_root_item(struct btrfs_root *root,
>>   {
>>   	uuid_le uuid;
>>   	int len;
>> -	int need_reset = 0;
>>
>>   	len = btrfs_item_size_nr(eb, slot);
>>   	read_extent_buffer(eb, item, btrfs_item_ptr_offset(eb, slot),
>>   			min_t(int, len, (int)sizeof(*item)));
>> -	if (len < sizeof(*item))
>> -		need_reset = 1;
>> -	if (!need_reset && btrfs_root_generation(item)
>> -		!= btrfs_root_generation_v2(item)) {
>> -		if (btrfs_root_generation_v2(item) != 0) {
>> -			printk(KERN_WARNING "btrfs: mismatching "
>> -					"generation and generation_v2 "
>> -					"found in root item. This root "
>> -					"was probably mounted with an "
>> -					"older kernel. Resetting all "
>> -					"new fields.\n");
>
> Please argue why removing this without replacement should be a good idea.
>
>> -		}
>> -		need_reset = 1;
>> -	}
>> -	if (need_reset) {
>> -		memset(&item->generation_v2, 0,
>> -			sizeof(*item) - offsetof(struct btrfs_root_item,
>> -					generation_v2));
>>
>> -		uuid_le_gen(&uuid);
>> -		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
>> +	/* when len is gt sizeof(*item) that means its downgrade and
>> +	   the new members won't be touched anyways
>> +	   if len is lt sizeof(*item) its upgrade, we need to take
>> +	   care to set/reset newer members.
>> +	*/
>> +	if (len < sizeof(*item)) {
>> +		/* there is an upgrade need to set/reset the new members
>> +		   assume v1 v2 v3 it can be upgrade from
>> +			v1 to v2 (set/reset only new members of v2)
>> +			v2 to v3 (set/reset only new members of v3)
>> +			v1 to v3 (set/reset new to v1)
>> +		*/
>> +		if (btrfs_root_generation(item) != btrfs_root_generation_v2(item)) {
>> +			//memset(&item->generation_v2, 0,
>> +				//sizeof(*item) - offsetof(struct btrfs_root_item, generation_v2));
>> +			memset(&item->generation_v2, 0, sizeof(*item) - len);
>> +			uuid_le_gen(&uuid);
>> +			memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
>> +		} else if (btrfs_root_generation_v2(item) != btrfs_root_generation_v3(item)) {
>> +			//memset(&item->generation_v3, 0,
>> +			//	sizeof(*item) - offsetof(struct btrfs_root_item, generation_v3));
>> +			memset(&item->generation_v3, 0, sizeof(*item) - len);
>> +		}
>>   	}
>>   }
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index 04bbfb1..45bff43 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -1116,6 +1116,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>>   	btrfs_set_root_otransid(new_root_item, trans->transid);
>>   	memset(&new_root_item->stime, 0, sizeof(new_root_item->stime));
>>   	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
>> +	memset(new_root_item->label, 0, BTRFS_LABEL_SIZE);
>>   	btrfs_set_root_stransid(new_root_item, 0);
>>   	btrfs_set_root_rtransid(new_root_item, 0);
>>
>>
>
> -Jan
> --
> 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
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2] Btrfs: add label to snapshot and subvol
  2012-11-16  4:52   ` [PATCH v2] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-16  6:33     ` Miao Xie
  2012-11-27 18:26       ` Anand Jain
  0 siblings, 1 reply; 41+ messages in thread
From: Miao Xie @ 2012-11-16  6:33 UTC (permalink / raw)
  To: Anand jain; +Cc: linux-btrfs

Hi, Anand

On fri, 16 Nov 2012 12:52:25 +0800, Anand jain wrote:
> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
> +						void __user *arg)
> +{
> +	char *label;
> +
> +	label = btrfs_root_label(&root->root_item);
> +	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
> +						void __user *arg)
> +{
> +	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
> +	struct btrfs_trans_handle *trans;
> +
> +	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE))
> +		return -EFAULT;
> +	trans = btrfs_join_transaction(root);
> +	if (IS_ERR(trans))
> +		return PTR_ERR(trans);
> +	btrfs_root_set_label(&root->root_item, label);
> +	btrfs_end_transaction(trans, root);

We need a lock to protect the label, and besides that, we need to get the write
access before we set the label for the subvolume.

Thanks
Miao


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2] Btrfs: add label to snapshot and subvol
  2012-11-16  6:33     ` Miao Xie
@ 2012-11-27 18:26       ` Anand Jain
  0 siblings, 0 replies; 41+ messages in thread
From: Anand Jain @ 2012-11-27 18:26 UTC (permalink / raw)
  To: miaox; +Cc: linux-btrfs


Hi Maio,

  Thanks for the review.
  Was away for sometime sorry for delay, have sent out the
  patch with review comments accepted.

-Anand

On 11/16/2012 02:33 PM, Miao Xie wrote:
> Hi, Anand
>
> On fri, 16 Nov 2012 12:52:25 +0800, Anand jain wrote:
>> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
>> +						void __user *arg)
>> +{
>> +	char *label;
>> +
>> +	label = btrfs_root_label(&root->root_item);
>> +	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
>> +		return -EFAULT;
>> +	return 0;
>> +}
>> +
>> +static int btrfs_ioctl_subvol_setlabel(struct btrfs_root *root,
>> +						void __user *arg)
>> +{
>> +	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
>> +	struct btrfs_trans_handle *trans;
>> +
>> +	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE))
>> +		return -EFAULT;
>> +	trans = btrfs_join_transaction(root);
>> +	if (IS_ERR(trans))
>> +		return PTR_ERR(trans);
>> +	btrfs_root_set_label(&root->root_item, label);
>> +	btrfs_end_transaction(trans, root);
>
> We need a lock to protect the label, and besides that, we need to get the write
> access before we set the label for the subvolume.
>
> Thanks
> Miao
>
> --
> 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
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [Request for review v3] [RFC] Add label support for snapshots and subvols
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (4 preceding siblings ...)
  2012-11-16  4:52 ` [Request for review v2] " Anand jain
@ 2012-11-27 18:29 ` Anand jain
  2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
                     ` (3 more replies)
  2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
                   ` (2 subsequent siblings)
  8 siblings, 4 replies; 41+ messages in thread
From: Anand jain @ 2012-11-27 18:29 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

v2->v3:
	Changes are in the btrfs kernel patch only, which accepts
	review comments by Miao.
v1->v2:
	This v2 patch accepts the review comments on the btrfs
	kernel changes by Jan. and
	Moved the get and set subvol label to under subvol sub-cmd
	eg:
		btrfs subvolume label /btrfs/ss5
		btrfs su la /btrfs/ss5 "ss5-label"
		btrfs su la /btrfs/ss5
		ss5-label

v1:
 (This patch is for the review/test not yet for the integration).

 Here is an implementation of the feature to add label to the
 subvolume and snapshots. Which would help sysadmin to better manager
 the subvol and snapshots.

 This can be done in two ways, one - using attr which is user land
 only changes but drawback is able to change the label using the
 non btrfs cli. And the other way is to add a member to
 btrfs_root_item in the btrfs kernel to hold the label info for
 each snapshot and subvol. The drawback here is having to introduce
 V3 version of this structure. If there is any better way pls do share.
 The patch code is for the review.

Any comments/suggestion welcome.

Below is a demo of this new feature.
------------
 btrfs fi label -t /btrfs/sv1 "Prod-DB"

 btrfs fi label -t /btrfs/sv1
Prod-DB

 btrfs su snap /btrfs/sv1 /btrfs/snap1-sv1
Create a snapshot of '/btrfs/sv1' in '/btrfs/snap1-sv1'
 btrfs fi label -t /btrfs/snap1-sv1

 btrfs fi label -t /btrfs/snap1-sv1 "Prod-DB-sand-box-testing"
 
 btrfs fi label -t /btrfs/snap1-sv1
Prod-DB-sand-box-testing
----------------

Anand Jain (3):
  Btrfs-progs: move open_file_or_dir() to utils.c
  Btrfs-progs: add feature to label subvol and snapshot
  Btrfs-progs: cmd option to show or set the subvol label

 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 btrfslabel.c     |   45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h     |    4 +++-
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |   38 ++++++++++++++++++++++++++++++++++++++
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 ctree.h          |    4 +++-
 ioctl.h          |    2 ++
 man/btrfs.8.in   |    6 ++++++
 print-tree.c     |    2 ++
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 17 files changed, 140 insertions(+), 58 deletions(-)
 delete mode 100644 common.c


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
@ 2012-11-27 18:29   ` Anand jain
  2012-11-28  3:05     ` Miao Xie
  2012-11-28 12:15     ` Anand jain
  2012-11-27 18:29   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 41+ messages in thread
From: Anand jain @ 2012-11-27 18:29 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

v3->v2:
accepts Miao' review comment.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h       |   12 +++++++++-
 fs/btrfs/ioctl.c       |   55 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h       |    2 +
 fs/btrfs/transaction.c |    1 +
 4 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 994d255..039e11c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -383,6 +383,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -759,7 +760,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-	__le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
@@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
 {
 	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
 }
+static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
+{
+	return (root_item->label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
+{
+	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
+}
 
 /* struct btrfs_root_backup */
 BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e58bd9d..f0b3d9d 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3725,6 +3725,57 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
 	return 0;
 }
 
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char *label;
+
+	label = btrfs_root_label(&root->root_item);
+	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
+		return -EFAULT;
+	return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct file *file,
+						void __user *arg)
+{
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	int ret;
+
+	if (btrfs_root_readonly(root))
+		return -EROFS;
+
+	if (!inode_owner_or_capable(inode))
+		return -EACCES;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	mutex_lock(&inode->i_mutex);
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		mutex_unlock(&inode->i_mutex);
+		goto out;
+	}
+	btrfs_root_set_label(&root->root_item, label);
+	btrfs_end_transaction(trans, root);
+	mutex_unlock(&inode->i_mutex);
+
+out:
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -3827,6 +3878,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_label(root, argp);
 	case BTRFS_IOC_SET_LABEL:
 		return btrfs_ioctl_set_label(root, argp);
+	case BTRFS_IOC_SUBVOL_GETLABEL:
+		return btrfs_ioctl_subvol_getlabel(root, argp);
+	case BTRFS_IOC_SUBVOL_SETLABEL:
+		return btrfs_ioctl_subvol_setlabel(file, argp);
 	}
 
 	return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 0c60fcb..1009a0c 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
 #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 04bbfb1..0a3f8b3 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1118,6 +1118,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
 	btrfs_set_root_stransid(new_root_item, 0);
 	btrfs_set_root_rtransid(new_root_item, 0);
+	memset(&new_root_item->label, 0, BTRFS_SUBVOL_LABEL_SIZE);
 
 	old = btrfs_lock_root_node(root);
 	ret = btrfs_cow_block(trans, root, old, NULL, 0, &old);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
  2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-27 18:29   ` Anand jain
  2012-11-27 18:29   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
  2012-11-27 18:29   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-27 18:29 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

The definition of the function open_file_or_dir() is moved from common.c
to utils.c in order to be able to share some common code between scrub
and the device stats in the following step. That common code uses
open_file_or_dir(). Since open_file_or_dir() makes use of the function
dirfd(3), the required XOPEN version was raised from 6 to 7.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Original-Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |    1 +
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 11 files changed, 42 insertions(+), 56 deletions(-)
 delete mode 100644 common.c

diff --git a/Makefile b/Makefile
index 4894903..8576d90 100644
--- a/Makefile
+++ b/Makefile
@@ -41,8 +41,8 @@ all: version $(progs) manpages
 version:
 	bash version.sh
 
-btrfs: $(objects) btrfs.o help.o common.o $(cmds_objects)
-	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o common.o $(cmds_objects) \
+btrfs: $(objects) btrfs.o help.o $(cmds_objects)
+	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \
 		$(objects) $(LDFLAGS) $(LIBS) -lpthread
 
 calc-size: $(objects) calc-size.o
diff --git a/btrfsctl.c b/btrfsctl.c
index 518684c..049a5f3 100644
--- a/btrfsctl.c
+++ b/btrfsctl.c
@@ -63,7 +63,7 @@ static void print_usage(void)
 	exit(1);
 }
 
-static int open_file_or_dir(const char *fname)
+static int btrfsctl_open_file_or_dir(const char *fname)
 {
 	int ret;
 	struct stat st;
@@ -91,6 +91,7 @@ static int open_file_or_dir(const char *fname)
 	}
 	return fd;
 }
+
 int main(int ac, char **av)
 {
 	char *fname = NULL;
@@ -128,7 +129,7 @@ int main(int ac, char **av)
 			snap_location = strdup(fullpath);
 			snap_location = dirname(snap_location);
 
-			snap_fd = open_file_or_dir(snap_location);
+			snap_fd = btrfsctl_open_file_or_dir(snap_location);
 
 			name = strdup(fullpath);
 			name = basename(name);
@@ -238,7 +239,7 @@ int main(int ac, char **av)
 		}
 		name = fname;
 	 } else {
-		fd = open_file_or_dir(fname);
+		fd = btrfsctl_open_file_or_dir(fname);
 	 }
 
 	if (name) {
diff --git a/cmds-balance.c b/cmds-balance.c
index 38a7426..6268b61 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -28,6 +28,7 @@
 #include "volumes.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const balance_cmd_group_usage[] = {
 	"btrfs [filesystem] balance <command> [options] <path>",
diff --git a/cmds-inspect.c b/cmds-inspect.c
index edabff5..79e069b 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -22,6 +22,7 @@
 
 #include "kerncompat.h"
 #include "ioctl.h"
+#include "utils.h"
 
 #include "commands.h"
 #include "btrfs-list.h"
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 1525c11..cafc284 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -24,6 +24,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const qgroup_cmd_group_usage[] = {
 	"btrfs qgroup <command> [options] <path>",
diff --git a/cmds-quota.c b/cmds-quota.c
index cf9ad97..8481514 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -23,6 +23,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const quota_cmd_group_usage[] = {
 	"btrfs quota <command> [options] <path>",
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ac39f7b..e3cdb1e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -32,6 +32,7 @@
 #include "ctree.h"
 #include "commands.h"
 #include "btrfs-list.h"
+#include "utils.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
diff --git a/commands.h b/commands.h
index bb6d2dd..8114a73 100644
--- a/commands.h
+++ b/commands.h
@@ -79,9 +79,6 @@ void help_ambiguous_token(const char *arg, const struct cmd_group *grp);
 
 void help_command_group(const struct cmd_group *grp, int argc, char **argv);
 
-/* common.c */
-int open_file_or_dir(const char *fname);
-
 extern const struct cmd_group subvolume_cmd_group;
 extern const struct cmd_group filesystem_cmd_group;
 extern const struct cmd_group balance_cmd_group;
diff --git a/common.c b/common.c
deleted file mode 100644
index 03f6570..0000000
--- a/common.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * 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 <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
-#include <fcntl.h>
-
-int open_file_or_dir(const char *fname)
-{
-	int ret;
-	struct stat st;
-	DIR *dirstream;
-	int fd;
-
-	ret = stat(fname, &st);
-	if (ret < 0) {
-		return -1;
-	}
-	if (S_ISDIR(st.st_mode)) {
-		dirstream = opendir(fname);
-		if (!dirstream) {
-			return -2;
-		}
-		fd = dirfd(dirstream);
-	} else {
-		fd = open(fname, O_RDWR);
-	}
-	if (fd < 0) {
-		return -3;
-	}
-	return fd;
-}
diff --git a/utils.c b/utils.c
index 205e667..774f81d 100644
--- a/utils.c
+++ b/utils.c
@@ -16,8 +16,9 @@
  * Boston, MA 021110-1307, USA.
  */
 
-#define _XOPEN_SOURCE 600
-#define __USE_XOPEN2K
+#define _XOPEN_SOURCE 700
+#define __USE_XOPEN2K8
+#define __XOPEN2K8 /* due to an error in dirent.h, to get dirfd() */
 #include <stdio.h>
 #include <stdlib.h>
 #ifndef __CHECKER__
@@ -1220,3 +1221,28 @@ scan_again:
 	return 0;
 }
 
+int open_file_or_dir(const char *fname)
+{
+	int ret;
+	struct stat st;
+	DIR *dirstream;
+	int fd;
+
+	ret = stat(fname, &st);
+	if (ret < 0) {
+		return -1;
+	}
+	if (S_ISDIR(st.st_mode)) {
+		dirstream = opendir(fname);
+		if (!dirstream) {
+			return -2;
+		}
+		fd = dirfd(dirstream);
+	} else {
+		fd = open(fname, O_RDWR);
+	}
+	if (fd < 0) {
+		return -3;
+	}
+	return fd;
+}
diff --git a/utils.h b/utils.h
index 3a0368b..6975f10 100644
--- a/utils.h
+++ b/utils.h
@@ -19,6 +19,8 @@
 #ifndef __UTILS__
 #define __UTILS__
 
+#include "ctree.h"
+
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
 int make_btrfs(int fd, const char *device, const char *label,
@@ -46,4 +48,5 @@ int check_label(char *input);
 int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
+int open_file_or_dir(const char *fname);
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
  2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
  2012-11-27 18:29   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
@ 2012-11-27 18:29   ` Anand jain
  2012-11-27 18:29   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-27 18:29 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfslabel.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h |    4 +++-
 ctree.h      |    4 +++-
 ioctl.h      |    2 ++
 print-tree.c |    2 ++
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/btrfslabel.c b/btrfslabel.c
index cb142b0..e161af1 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -126,3 +126,48 @@ int set_label(char *btrfs_dev, char *nLabel)
 	change_label_unmounted(btrfs_dev, nLabel);
 	return 0;
 }
+
+int get_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_GETLABEL, labelp) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	labelp[BTRFS_LABEL_SIZE] = '\0';
+	if (!strlen(labelp))
+		return -1;
+	close(fd);
+	return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+
+	if (strlen(labelp) > BTRFS_SUBVOL_LABEL_SIZE) {
+		fprintf(stderr, "ERROR: Subvol label is more than max length %d\n",
+			BTRFS_SUBVOL_LABEL_SIZE);
+		return -1;
+	}
+	memset(label, 0, BTRFS_SUBVOL_LABEL_SIZE+1);
+	strcpy(label, labelp);
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_SETLABEL, label) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	close(fd);
+	return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..1abc483 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
 
 
 int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol, char *labelp);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/ctree.h b/ctree.h
index 293b24f..68bae28 100644
--- a/ctree.h
+++ b/ctree.h
@@ -325,6 +325,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -702,7 +703,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-        __le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
diff --git a/ioctl.h b/ioctl.h
index 6fda3a1..009fa6e 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -432,4 +432,6 @@ struct btrfs_ioctl_clone_range_args {
 					struct btrfs_ioctl_qgroup_create_args)
 #define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
 					struct btrfs_ioctl_qgroup_limit_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/print-tree.c b/print-tree.c
index 7c615dd..5979109 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -330,6 +330,8 @@ static void print_root(struct extent_buffer *leaf, int slot)
 				btrfs_root_stransid(&root_item),
 				btrfs_root_rtransid(&root_item));
 		}
+		if (strlen(root_item.label))
+			printf("\t\tlabel %s\n", root_item.label);
 	}
 	if (btrfs_root_refs(&root_item) == 0) {
 		struct btrfs_key drop_key;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
                     ` (2 preceding siblings ...)
  2012-11-27 18:29   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
@ 2012-11-27 18:29   ` Anand jain
  3 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-27 18:29 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c |   37 +++++++++++++++++++++++++++++++++++++
 man/btrfs.8.in   |    6 ++++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e3cdb1e..759eade 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -33,6 +33,7 @@
 #include "commands.h"
 #include "btrfs-list.h"
 #include "utils.h"
+#include "btrfslabel.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
@@ -700,6 +701,41 @@ static int cmd_find_new(int argc, char **argv)
 	return 0;
 }
 
+static const char * const cmd_subvol_label_usage[] = {
+	"btrfs subvolume label <path> [label]",
+	"Show or set label for the subvol or snapshot",
+	NULL
+};
+
+static int cmd_subvol_label(int argc, char **argv)
+{
+	struct stat st;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	int ret;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+		usage(cmd_subvol_label_usage);
+
+	if (stat(argv[1], &st) < 0) {
+		fprintf(stderr, "Error: %s\n",strerror(errno));
+		return -errno;
+	}
+	if (!S_ISDIR(st.st_mode)) {
+		fprintf(stderr, "Error: Not a dir\n");
+		return -1;
+	}
+	if (argc > 2)
+		return set_subvol_label(argv[1], argv[2]);
+	else {
+		ret = get_subvol_label(argv[1], label);
+		if (ret)
+			return ret;
+		label[BTRFS_SUBVOL_LABEL_SIZE]=0;
+		printf("%s\n",label);
+	}
+	return 0;
+}
+
 const struct cmd_group subvolume_cmd_group = {
 	subvolume_cmd_group_usage, NULL, {
 		{ "create", cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 },
@@ -711,6 +747,7 @@ const struct cmd_group subvolume_cmd_group = {
 		{ "set-default", cmd_subvol_set_default,
 			cmd_subvol_set_default_usage, NULL, 0 },
 		{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
+		{ "label", cmd_subvol_label, cmd_subvol_label_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }
 	}
 };
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 9222580..aa225d9 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -17,6 +17,8 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBsubvolume get-default\fP\fI <path>\fP
 .PP
+\fBbtrfs\fP \fBsubvolume label\fP\fI <path> [label]\fP
+.PP
 \fBbtrfs\fP \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] \
 [-s \fIstart\fR] [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> \
 [<\fIfile\fR>|<\fIdir\fR>...]
@@ -160,6 +162,10 @@ Get the default subvolume of the filesystem \fI<path>\fR. The output format
 is similar to \fBsubvolume list\fR command.
 .TP
 
+\fBsubvolume label\fR\fI <path> [label]\fR
+Show or set \fI[label]\fR for the subvolume or the snapshot \fI<path>\fR.
+.TP
+
 \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] [-s \fIstart\fR] \
 [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> [<\fIfile\fR>|<\fIdir\fR>...]
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
@ 2012-11-28  3:05     ` Miao Xie
  2012-11-28 10:02       ` Anand Jain
  2012-11-28 12:15     ` Anand jain
  1 sibling, 1 reply; 41+ messages in thread
From: Miao Xie @ 2012-11-28  3:05 UTC (permalink / raw)
  To: Anand jain; +Cc: linux-btrfs

On wed, 28 Nov 2012 02:29:17 +0800, Anand jain wrote:
>  /*
> @@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>  {
>  	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>  }
> +static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
> +{
> +	return (root_item->label);
> +}
> +static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
> +{
> +	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
> +}
>  
>  /* struct btrfs_root_backup */
>  BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index e58bd9d..f0b3d9d 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3725,6 +3725,57 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>  	return 0;
>  }
>  
> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
> +						void __user *arg)
> +{
> +	char *label;
> +
> +	label = btrfs_root_label(&root->root_item);
> +	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
> +		return -EFAULT;

we also need lock here.

> +	return 0;
> +}
> +
> +static int btrfs_ioctl_subvol_setlabel(struct file *file,
> +						void __user *arg)
> +{
> +	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
> +	struct inode *inode = file->f_path.dentry->d_inode;
> +	int ret;
> +
> +	if (btrfs_root_readonly(root))
> +		return -EROFS;
> +
> +	if (!inode_owner_or_capable(inode))
> +		return -EACCES;
> +
> +	ret = mnt_want_write_file(file);
> +	if (ret)
> +		return ret;
> +
> +	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	mutex_lock(&inode->i_mutex);

we should use a lock which belongs to the root not inode.

Thanks
Miao

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-28  3:05     ` Miao Xie
@ 2012-11-28 10:02       ` Anand Jain
  2012-11-28 10:25         ` Miao Xie
  0 siblings, 1 reply; 41+ messages in thread
From: Anand Jain @ 2012-11-28 10:02 UTC (permalink / raw)
  To: miaox; +Cc: linux-btrfs



On 11/28/2012 11:05 AM, Miao Xie wrote:
> On wed, 28 Nov 2012 02:29:17 +0800, Anand jain wrote:
>>   /*
>> @@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>>   {
>>   	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>>   }
>> +static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
>> +{
>> +	return (root_item->label);
>> +}
>> +static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
>> +{
>> +	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
>> +}
>>
>>   /* struct btrfs_root_backup */
>>   BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index e58bd9d..f0b3d9d 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3725,6 +3725,57 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>>   	return 0;
>>   }
>>
>> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
>> +						void __user *arg)
>> +{
>> +	char *label;
>> +
>> +	label = btrfs_root_label(&root->root_item);
>> +	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
>> +		return -EFAULT;
>
> we also need lock here.

  yes. wish memcpy is atomic.

>> +	return 0;
>> +}
>> +
>> +static int btrfs_ioctl_subvol_setlabel(struct file *file,
>> +						void __user *arg)
>> +{
>> +	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
>> +	struct inode *inode = file->f_path.dentry->d_inode;
>> +	int ret;
>> +
>> +	if (btrfs_root_readonly(root))
>> +		return -EROFS;
>> +
>> +	if (!inode_owner_or_capable(inode))
>> +		return -EACCES;
>> +
>> +	ret = mnt_want_write_file(file);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
>> +		ret = -EFAULT;
>> +		goto out;
>> +	}
>> +
>> +	mutex_lock(&inode->i_mutex);
>
> we should use a lock which belongs to the root not inode.


  I couldn't find an already defined lock which would exactly
  fit the purpose here. Do you have any idea ?


Thanks, Anand



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-28 10:02       ` Anand Jain
@ 2012-11-28 10:25         ` Miao Xie
  2012-11-28 12:17           ` Anand Jain
  0 siblings, 1 reply; 41+ messages in thread
From: Miao Xie @ 2012-11-28 10:25 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On 	wed, 28 Nov 2012 18:02:56 +0800, Anand Jain wrote:
> 
> 
> On 11/28/2012 11:05 AM, Miao Xie wrote:
>> On wed, 28 Nov 2012 02:29:17 +0800, Anand jain wrote:
>>>   /*
>>> @@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>>>   {
>>>       return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>>>   }
>>> +static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
>>> +{
>>> +    return (root_item->label);
>>> +}
>>> +static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
>>> +{
>>> +    memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
>>> +}
>>>
>>>   /* struct btrfs_root_backup */
>>>   BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>> index e58bd9d..f0b3d9d 100644
>>> --- a/fs/btrfs/ioctl.c
>>> +++ b/fs/btrfs/ioctl.c
>>> @@ -3725,6 +3725,57 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>>>       return 0;
>>>   }
>>>
>>> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
>>> +                        void __user *arg)
>>> +{
>>> +    char *label;
>>> +
>>> +    label = btrfs_root_label(&root->root_item);
>>> +    if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
>>> +        return -EFAULT;
>>
>> we also need lock here.
> 
>  yes. wish memcpy is atomic.
> 
>>> +    return 0;
>>> +}
>>> +
>>> +static int btrfs_ioctl_subvol_setlabel(struct file *file,
>>> +                        void __user *arg)
>>> +{
>>> +    char label[BTRFS_SUBVOL_LABEL_SIZE+1];
>>> +    struct btrfs_trans_handle *trans;
>>> +    struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
>>> +    struct inode *inode = file->f_path.dentry->d_inode;
>>> +    int ret;
>>> +
>>> +    if (btrfs_root_readonly(root))
>>> +        return -EROFS;
>>> +
>>> +    if (!inode_owner_or_capable(inode))
>>> +        return -EACCES;
>>> +
>>> +    ret = mnt_want_write_file(file);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
>>> +        ret = -EFAULT;
>>> +        goto out;
>>> +    }
>>> +
>>> +    mutex_lock(&inode->i_mutex);
>>
>> we should use a lock which belongs to the root not inode.
> 
> 
>  I couldn't find an already defined lock which would exactly
>  fit the purpose here. Do you have any idea ?

I thinks we may use ->root_times_lock, but we need change the name
to make it suitable for its role.

Thanks
Miao

> 
> 
> 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
> 


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
  2012-11-28  3:05     ` Miao Xie
@ 2012-11-28 12:15     ` Anand jain
  1 sibling, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-28 12:15 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Jan Schmidt <list.btrfs@jan-o-sch.net>
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ctree.h       |   12 +++++++++-
 fs/btrfs/ioctl.c       |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/ioctl.h       |    2 +
 fs/btrfs/transaction.c |    1 +
 4 files changed, 72 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 994d255..039e11c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -383,6 +383,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -759,7 +760,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-	__le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
@@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
 {
 	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
 }
+static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
+{
+	return (root_item->label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
+{
+	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
+}
 
 /* struct btrfs_root_backup */
 BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e58bd9d..abbac27 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3725,6 +3725,60 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
 	return 0;
 }
 
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char *label;
+
+	spin_lock(&root->root_times_lock);
+	label = btrfs_root_label(&root->root_item);
+	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE)) {
+		spin_unlock(&root->root_times_lock);
+		return -EFAULT;
+	}
+	spin_unlock(&root->root_times_lock);
+	return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct file *file,
+						void __user *arg)
+{
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	int ret;
+
+	if (btrfs_root_readonly(root))
+		return -EROFS;
+
+	if (!inode_owner_or_capable(inode))
+		return -EACCES;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
+	spin_lock(&root->root_times_lock);
+	btrfs_root_set_label(&root->root_item, label);
+	spin_unlock(&root->root_times_lock);
+	btrfs_end_transaction(trans, root);
+
+out:
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -3827,6 +3881,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_label(root, argp);
 	case BTRFS_IOC_SET_LABEL:
 		return btrfs_ioctl_set_label(root, argp);
+	case BTRFS_IOC_SUBVOL_GETLABEL:
+		return btrfs_ioctl_subvol_getlabel(root, argp);
+	case BTRFS_IOC_SUBVOL_SETLABEL:
+		return btrfs_ioctl_subvol_setlabel(file, argp);
 	}
 
 	return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 0c60fcb..1009a0c 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -455,4 +455,6 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_GET_LABEL _IOR(BTRFS_IOCTL_MAGIC, 53, __u64)
 #define BTRFS_IOC_SET_LABEL _IOW(BTRFS_IOCTL_MAGIC, 54, __u64)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 #endif
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 04bbfb1..0a3f8b3 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1118,6 +1118,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
 	btrfs_set_root_stransid(new_root_item, 0);
 	btrfs_set_root_rtransid(new_root_item, 0);
+	memset(&new_root_item->label, 0, BTRFS_SUBVOL_LABEL_SIZE);
 
 	old = btrfs_lock_root_node(root);
 	ret = btrfs_cow_block(trans, root, old, NULL, 0, &old);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH v3] Btrfs: add label to snapshot and subvol
  2012-11-28 10:25         ` Miao Xie
@ 2012-11-28 12:17           ` Anand Jain
  0 siblings, 0 replies; 41+ messages in thread
From: Anand Jain @ 2012-11-28 12:17 UTC (permalink / raw)
  To: miaox; +Cc: linux-btrfs


Miao,
  Thanks for the review.
  A new patch was sent.

Anand

On 11/28/2012 06:25 PM, Miao Xie wrote:
> On 	wed, 28 Nov 2012 18:02:56 +0800, Anand Jain wrote:
>>
>>
>> On 11/28/2012 11:05 AM, Miao Xie wrote:
>>> On wed, 28 Nov 2012 02:29:17 +0800, Anand jain wrote:
>>>>    /*
>>>> @@ -2441,6 +2443,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
>>>>    {
>>>>        return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
>>>>    }
>>>> +static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
>>>> +{
>>>> +    return (root_item->label);
>>>> +}
>>>> +static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
>>>> +{
>>>> +    memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
>>>> +}
>>>>
>>>>    /* struct btrfs_root_backup */
>>>>    BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
>>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>>> index e58bd9d..f0b3d9d 100644
>>>> --- a/fs/btrfs/ioctl.c
>>>> +++ b/fs/btrfs/ioctl.c
>>>> @@ -3725,6 +3725,57 @@ static int btrfs_ioctl_set_label(struct btrfs_root *root, void __user *arg)
>>>>        return 0;
>>>>    }
>>>>
>>>> +static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
>>>> +                        void __user *arg)
>>>> +{
>>>> +    char *label;
>>>> +
>>>> +    label = btrfs_root_label(&root->root_item);
>>>> +    if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE))
>>>> +        return -EFAULT;
>>>
>>> we also need lock here.
>>
>>   yes. wish memcpy is atomic.
>>
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int btrfs_ioctl_subvol_setlabel(struct file *file,
>>>> +                        void __user *arg)
>>>> +{
>>>> +    char label[BTRFS_SUBVOL_LABEL_SIZE+1];
>>>> +    struct btrfs_trans_handle *trans;
>>>> +    struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
>>>> +    struct inode *inode = file->f_path.dentry->d_inode;
>>>> +    int ret;
>>>> +
>>>> +    if (btrfs_root_readonly(root))
>>>> +        return -EROFS;
>>>> +
>>>> +    if (!inode_owner_or_capable(inode))
>>>> +        return -EACCES;
>>>> +
>>>> +    ret = mnt_want_write_file(file);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
>>>> +        ret = -EFAULT;
>>>> +        goto out;
>>>> +    }
>>>> +
>>>> +    mutex_lock(&inode->i_mutex);
>>>
>>> we should use a lock which belongs to the root not inode.
>>
>>
>>   I couldn't find an already defined lock which would exactly
>>   fit the purpose here. Do you have any idea ?
>
> I thinks we may use ->root_times_lock, but we need change the name
> to make it suitable for its role.
>
> Thanks
> Miao
>
>>
>>
>> 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
>>
>
> --
> 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
>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 0/3] Add subvol and snapshot attribute label
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (5 preceding siblings ...)
  2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
@ 2012-11-30 15:47 ` Anand jain
  2012-11-30 15:48   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
                     ` (2 more replies)
  2013-02-25  5:31 ` [PATCH 0/2 v4] Btrfs-progs: Add support for " Anand Jain
  2013-02-25  5:31 ` [PATCH v4] Btrfs: Add support for " Anand Jain
  8 siblings, 3 replies; 41+ messages in thread
From: Anand jain @ 2012-11-30 15:47 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

This patch set creates the attribute user.label in the root
space, for the subvol and snapshots. Command to use will be
	btrfs subvol label <subvol> [label]
This patch is only for your kind review. And to evaluate both
the approaches, as found during the survey.
Thanks

Anand Jain (3):
  Btrfs-progs: move open_file_or_dir() to utils.c
  Btrfs-progs: add attribute label for subvol and snapshot
  Btrfs-progs: cmd option to show or set the subvol label

 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 btrfslabel.c     |   38 ++++++++++++++++++++++++++++++++++++++
 btrfslabel.h     |    4 +++-
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |   38 ++++++++++++++++++++++++++++++++++++++
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 ctree.h          |    1 +
 man/btrfs.8.in   |    6 ++++++
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 15 files changed, 127 insertions(+), 57 deletions(-)
 delete mode 100644 common.c


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c
  2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
@ 2012-11-30 15:48   ` Anand jain
  2012-11-30 15:48   ` [PATCH 2/3] Btrfs-progs: add attribute label for subvol and snapshot Anand jain
  2012-11-30 15:48   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  2 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-30 15:48 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

The definition of the function open_file_or_dir() is moved from common.c
to utils.c in order to be able to share some common code between scrub
and the device stats in the following step. That common code uses
open_file_or_dir(). Since open_file_or_dir() makes use of the function
dirfd(3), the required XOPEN version was raised from 6 to 7.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Original-Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
 Makefile         |    4 ++--
 btrfsctl.c       |    7 ++++---
 cmds-balance.c   |    1 +
 cmds-inspect.c   |    1 +
 cmds-qgroup.c    |    1 +
 cmds-quota.c     |    1 +
 cmds-subvolume.c |    1 +
 commands.h       |    3 ---
 common.c         |   46 ----------------------------------------------
 utils.c          |   30 ++++++++++++++++++++++++++++--
 utils.h          |    3 +++
 11 files changed, 42 insertions(+), 56 deletions(-)
 delete mode 100644 common.c

diff --git a/Makefile b/Makefile
index 4894903..8576d90 100644
--- a/Makefile
+++ b/Makefile
@@ -41,8 +41,8 @@ all: version $(progs) manpages
 version:
 	bash version.sh
 
-btrfs: $(objects) btrfs.o help.o common.o $(cmds_objects)
-	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o common.o $(cmds_objects) \
+btrfs: $(objects) btrfs.o help.o $(cmds_objects)
+	$(CC) $(CFLAGS) -o btrfs btrfs.o help.o $(cmds_objects) \
 		$(objects) $(LDFLAGS) $(LIBS) -lpthread
 
 calc-size: $(objects) calc-size.o
diff --git a/btrfsctl.c b/btrfsctl.c
index 518684c..049a5f3 100644
--- a/btrfsctl.c
+++ b/btrfsctl.c
@@ -63,7 +63,7 @@ static void print_usage(void)
 	exit(1);
 }
 
-static int open_file_or_dir(const char *fname)
+static int btrfsctl_open_file_or_dir(const char *fname)
 {
 	int ret;
 	struct stat st;
@@ -91,6 +91,7 @@ static int open_file_or_dir(const char *fname)
 	}
 	return fd;
 }
+
 int main(int ac, char **av)
 {
 	char *fname = NULL;
@@ -128,7 +129,7 @@ int main(int ac, char **av)
 			snap_location = strdup(fullpath);
 			snap_location = dirname(snap_location);
 
-			snap_fd = open_file_or_dir(snap_location);
+			snap_fd = btrfsctl_open_file_or_dir(snap_location);
 
 			name = strdup(fullpath);
 			name = basename(name);
@@ -238,7 +239,7 @@ int main(int ac, char **av)
 		}
 		name = fname;
 	 } else {
-		fd = open_file_or_dir(fname);
+		fd = btrfsctl_open_file_or_dir(fname);
 	 }
 
 	if (name) {
diff --git a/cmds-balance.c b/cmds-balance.c
index 38a7426..6268b61 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -28,6 +28,7 @@
 #include "volumes.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const balance_cmd_group_usage[] = {
 	"btrfs [filesystem] balance <command> [options] <path>",
diff --git a/cmds-inspect.c b/cmds-inspect.c
index edabff5..79e069b 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -22,6 +22,7 @@
 
 #include "kerncompat.h"
 #include "ioctl.h"
+#include "utils.h"
 
 #include "commands.h"
 #include "btrfs-list.h"
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 1525c11..cafc284 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -24,6 +24,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const qgroup_cmd_group_usage[] = {
 	"btrfs qgroup <command> [options] <path>",
diff --git a/cmds-quota.c b/cmds-quota.c
index cf9ad97..8481514 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -23,6 +23,7 @@
 #include "ioctl.h"
 
 #include "commands.h"
+#include "utils.h"
 
 static const char * const quota_cmd_group_usage[] = {
 	"btrfs quota <command> [options] <path>",
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ac39f7b..e3cdb1e 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -32,6 +32,7 @@
 #include "ctree.h"
 #include "commands.h"
 #include "btrfs-list.h"
+#include "utils.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
diff --git a/commands.h b/commands.h
index bb6d2dd..8114a73 100644
--- a/commands.h
+++ b/commands.h
@@ -79,9 +79,6 @@ void help_ambiguous_token(const char *arg, const struct cmd_group *grp);
 
 void help_command_group(const struct cmd_group *grp, int argc, char **argv);
 
-/* common.c */
-int open_file_or_dir(const char *fname);
-
 extern const struct cmd_group subvolume_cmd_group;
 extern const struct cmd_group filesystem_cmd_group;
 extern const struct cmd_group balance_cmd_group;
diff --git a/common.c b/common.c
deleted file mode 100644
index 03f6570..0000000
--- a/common.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * 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 <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
-#include <fcntl.h>
-
-int open_file_or_dir(const char *fname)
-{
-	int ret;
-	struct stat st;
-	DIR *dirstream;
-	int fd;
-
-	ret = stat(fname, &st);
-	if (ret < 0) {
-		return -1;
-	}
-	if (S_ISDIR(st.st_mode)) {
-		dirstream = opendir(fname);
-		if (!dirstream) {
-			return -2;
-		}
-		fd = dirfd(dirstream);
-	} else {
-		fd = open(fname, O_RDWR);
-	}
-	if (fd < 0) {
-		return -3;
-	}
-	return fd;
-}
diff --git a/utils.c b/utils.c
index 205e667..774f81d 100644
--- a/utils.c
+++ b/utils.c
@@ -16,8 +16,9 @@
  * Boston, MA 021110-1307, USA.
  */
 
-#define _XOPEN_SOURCE 600
-#define __USE_XOPEN2K
+#define _XOPEN_SOURCE 700
+#define __USE_XOPEN2K8
+#define __XOPEN2K8 /* due to an error in dirent.h, to get dirfd() */
 #include <stdio.h>
 #include <stdlib.h>
 #ifndef __CHECKER__
@@ -1220,3 +1221,28 @@ scan_again:
 	return 0;
 }
 
+int open_file_or_dir(const char *fname)
+{
+	int ret;
+	struct stat st;
+	DIR *dirstream;
+	int fd;
+
+	ret = stat(fname, &st);
+	if (ret < 0) {
+		return -1;
+	}
+	if (S_ISDIR(st.st_mode)) {
+		dirstream = opendir(fname);
+		if (!dirstream) {
+			return -2;
+		}
+		fd = dirfd(dirstream);
+	} else {
+		fd = open(fname, O_RDWR);
+	}
+	if (fd < 0) {
+		return -3;
+	}
+	return fd;
+}
diff --git a/utils.h b/utils.h
index 3a0368b..6975f10 100644
--- a/utils.h
+++ b/utils.h
@@ -19,6 +19,8 @@
 #ifndef __UTILS__
 #define __UTILS__
 
+#include "ctree.h"
+
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
 int make_btrfs(int fd, const char *device, const char *label,
@@ -46,4 +48,5 @@ int check_label(char *input);
 int get_mountpt(char *dev, char *mntpt, size_t size);
 
 int btrfs_scan_block_devices(int run_ioctl);
+int open_file_or_dir(const char *fname);
 #endif
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/3] Btrfs-progs: add attribute label for subvol and snapshot
  2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
  2012-11-30 15:48   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
@ 2012-11-30 15:48   ` Anand jain
  2012-11-30 15:48   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
  2 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-30 15:48 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfslabel.c |   38 ++++++++++++++++++++++++++++++++++++++
 btrfslabel.h |    4 +++-
 ctree.h      |    1 +
 3 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/btrfslabel.c b/btrfslabel.c
index cb142b0..8c424e5 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -34,6 +34,8 @@
 #include <linux/fs.h>
 #include <linux/limits.h>
 #include <ctype.h>
+#include <attr/xattr.h>
+#include <attr/attributes.h>
 #include "kerncompat.h"
 #include "ctree.h"
 #include "utils.h"
@@ -126,3 +128,39 @@ int set_label(char *btrfs_dev, char *nLabel)
 	change_label_unmounted(btrfs_dev, nLabel);
 	return 0;
 }
+
+int get_subvol_label(char *subvol, char *labelp)
+{
+	int ret;
+	ret = getxattr(subvol, "user.label", labelp, BTRFS_SUBVOL_LABEL_SIZE);
+	if(ret < 0) {
+		labelp = "";
+		fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+			strerror(errno));
+		return -errno;
+	}
+	labelp[ret] = '\0';
+	return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+	int e=0;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+
+	if (strlen(labelp) > BTRFS_SUBVOL_LABEL_SIZE) {
+		fprintf(stderr, "ERROR: Subvol label is more than max length %d\n",
+			BTRFS_SUBVOL_LABEL_SIZE);
+		return -1;
+	}
+	memset(label, 0, BTRFS_SUBVOL_LABEL_SIZE+1);
+	strcpy(label, labelp);
+	if(setxattr(subvol, "user.label", label, BTRFS_SUBVOL_LABEL_SIZE,
+		ATTR_ROOT) == -1) {
+		e = errno;
+		fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+			strerror(e));
+		return -e;
+	}
+	return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..1abc483 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
 
 
 int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol, char *labelp);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/ctree.h b/ctree.h
index 293b24f..993dbcf 100644
--- a/ctree.h
+++ b/ctree.h
@@ -325,6 +325,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label
  2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
  2012-11-30 15:48   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
  2012-11-30 15:48   ` [PATCH 2/3] Btrfs-progs: add attribute label for subvol and snapshot Anand jain
@ 2012-11-30 15:48   ` Anand jain
  2 siblings, 0 replies; 41+ messages in thread
From: Anand jain @ 2012-11-30 15:48 UTC (permalink / raw)
  To: linux-btrfs

From: Anand Jain <anand.jain@oracle.com>

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c |   37 +++++++++++++++++++++++++++++++++++++
 man/btrfs.8.in   |    6 ++++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e3cdb1e..759eade 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -33,6 +33,7 @@
 #include "commands.h"
 #include "btrfs-list.h"
 #include "utils.h"
+#include "btrfslabel.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
@@ -700,6 +701,41 @@ static int cmd_find_new(int argc, char **argv)
 	return 0;
 }
 
+static const char * const cmd_subvol_label_usage[] = {
+	"btrfs subvolume label <path> [label]",
+	"Show or set label for the subvol or snapshot",
+	NULL
+};
+
+static int cmd_subvol_label(int argc, char **argv)
+{
+	struct stat st;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	int ret;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+		usage(cmd_subvol_label_usage);
+
+	if (stat(argv[1], &st) < 0) {
+		fprintf(stderr, "Error: %s\n",strerror(errno));
+		return -errno;
+	}
+	if (!S_ISDIR(st.st_mode)) {
+		fprintf(stderr, "Error: Not a dir\n");
+		return -1;
+	}
+	if (argc > 2)
+		return set_subvol_label(argv[1], argv[2]);
+	else {
+		ret = get_subvol_label(argv[1], label);
+		if (ret)
+			return ret;
+		label[BTRFS_SUBVOL_LABEL_SIZE]=0;
+		printf("%s\n",label);
+	}
+	return 0;
+}
+
 const struct cmd_group subvolume_cmd_group = {
 	subvolume_cmd_group_usage, NULL, {
 		{ "create", cmd_subvol_create, cmd_subvol_create_usage, NULL, 0 },
@@ -711,6 +747,7 @@ const struct cmd_group subvolume_cmd_group = {
 		{ "set-default", cmd_subvol_set_default,
 			cmd_subvol_set_default_usage, NULL, 0 },
 		{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
+		{ "label", cmd_subvol_label, cmd_subvol_label_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }
 	}
 };
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 9222580..aa225d9 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -17,6 +17,8 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBsubvolume get-default\fP\fI <path>\fP
 .PP
+\fBbtrfs\fP \fBsubvolume label\fP\fI <path> [label]\fP
+.PP
 \fBbtrfs\fP \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] \
 [-s \fIstart\fR] [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> \
 [<\fIfile\fR>|<\fIdir\fR>...]
@@ -160,6 +162,10 @@ Get the default subvolume of the filesystem \fI<path>\fR. The output format
 is similar to \fBsubvolume list\fR command.
 .TP
 
+\fBsubvolume label\fR\fI <path> [label]\fR
+Show or set \fI[label]\fR for the subvolume or the snapshot \fI<path>\fR.
+.TP
+
 \fBfilesystem defragment\fP -c[zlib|lzo] [-l \fIlen\fR] [-s \fIstart\fR] \
 [-t \fIsize\fR] -[vf] <\fIfile\fR>|<\fIdir\fR> [<\fIfile\fR>|<\fIdir\fR>...]
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 0/2 v4] Btrfs-progs: Add support for subvol label
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (6 preceding siblings ...)
  2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
@ 2013-02-25  5:31 ` Anand Jain
  2013-02-25  5:31   ` [PATCH 1/2 v4] Btrfs-progs: add feature to label subvol and snapshot Anand Jain
  2013-02-25  5:31   ` [PATCH 2/2 v4] Btrfs-progs: cmd option to show or set the subvol label Anand Jain
  2013-02-25  5:31 ` [PATCH v4] Btrfs: Add support for " Anand Jain
  8 siblings, 2 replies; 41+ messages in thread
From: Anand Jain @ 2013-02-25  5:31 UTC (permalink / raw)
  To: linux-btrfs

This is the Btrfs-progs changes to add support for the subvol label.
As discussed in the mailing-list there are two ways we could get the
label attached to the subvol, using kernel-ioctl method and the
other method is by using attributes.

I preferred kernel-ioctl method over the attribute since
there might be more than just the btrfs-progs which is reading and
writing the label, if used attributes, the keyword is defined and
used at the application level, which will be difficult to publicize
and maintain the consistent keyword across any application in the
long run.  In the kernel-ioctl method we don't have this problem.

For kernel-ioctl method you need the below two Btrfs-progs 
patches and the following kernel patch which is posted to
mailing-list

 --------
  Btrfs: ability to add label to snapshot and subvol

 fs/btrfs/ctree.h           | 12 +++++++++-
 fs/btrfs/ioctl.c           | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/transaction.c     |  1 +
 include/uapi/linux/btrfs.h |  2 ++
 4 files changed, 72 insertions(+), 1 deletion(-)
 --------

Still in any case if we decide to use the attributes method
over the kernel-ioctl method, then following btrfs-progs patch
  Btrfs-progs: add feature to label subvol and snapshot
should be replaced with
  Btrfs-progs: add attribute label for subvol and snapshot
  (also posted in the mailing-list)
and the above kernel patch should be removed.

v4: rebased to David's integration-20130219 branch

Anand Jain (2):
  Btrfs-progs: add feature to label subvol and snapshot
  Btrfs-progs: cmd option to show or set the subvol label

 btrfslabel.c     | 45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h     |  4 +++-
 cmds-subvolume.c | 37 +++++++++++++++++++++++++++++++++++++
 ctree.h          |  4 +++-
 ioctl.h          |  2 ++
 man/btrfs.8.in   |  6 ++++++
 print-tree.c     |  2 ++
 7 files changed, 98 insertions(+), 2 deletions(-)

-- 
1.8.1.227.g44fe835


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 1/2 v4] Btrfs-progs: add feature to label subvol and snapshot
  2013-02-25  5:31 ` [PATCH 0/2 v4] Btrfs-progs: Add support for " Anand Jain
@ 2013-02-25  5:31   ` Anand Jain
  2013-02-25  5:31   ` [PATCH 2/2 v4] Btrfs-progs: cmd option to show or set the subvol label Anand Jain
  1 sibling, 0 replies; 41+ messages in thread
From: Anand Jain @ 2013-02-25  5:31 UTC (permalink / raw)
  To: linux-btrfs

btrfs-progs framework to add support to label the subvol and
snapshots. This patch is for the ioctl-way to write label
to the subvol and snapshot. (the other way is to use attribute
if then only this patch has to be backed out in btrfs-progs
and replace with the patch titled
 Btrfs-progs: add attribute label for subvol and snapshot)

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfslabel.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 btrfslabel.h |  4 +++-
 ctree.h      |  4 +++-
 ioctl.h      |  2 ++
 print-tree.c |  2 ++
 5 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/btrfslabel.c b/btrfslabel.c
index a421a8b..b3ad540 100644
--- a/btrfslabel.c
+++ b/btrfslabel.c
@@ -126,3 +126,48 @@ int set_label(char *btrfs_dev, char *nLabel)
 	}
 	return change_label_unmounted(btrfs_dev, nLabel);
 }
+
+int get_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_GETLABEL, labelp) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: get subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	labelp[BTRFS_LABEL_SIZE] = '\0';
+	if (!strlen(labelp))
+		return -1;
+	close(fd);
+	return 0;
+}
+
+int set_subvol_label(char *subvol, char *labelp)
+{
+	int fd, e=0;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+
+	if (strlen(labelp) > BTRFS_SUBVOL_LABEL_SIZE) {
+		fprintf(stderr, "ERROR: Subvol label is more than max length %d\n",
+			BTRFS_SUBVOL_LABEL_SIZE);
+		return -1;
+	}
+	memset(label, 0, BTRFS_SUBVOL_LABEL_SIZE+1);
+	strcpy(label, labelp);
+	fd = open_file_or_dir(subvol);
+
+	if(ioctl(fd, BTRFS_IOC_SUBVOL_SETLABEL, label) < 0) {
+		e = errno;
+		fprintf(stderr, "ERROR: set subvol label failed, %s\n",
+			strerror(e));
+		close(fd);
+		return -e;
+	}
+	close(fd);
+	return 0;
+}
diff --git a/btrfslabel.h b/btrfslabel.h
index abf43ad..1abc483 100644
--- a/btrfslabel.h
+++ b/btrfslabel.h
@@ -2,4 +2,6 @@
 
 
 int get_label(char *btrfs_dev);
-int set_label(char *btrfs_dev, char *nLabel);
\ No newline at end of file
+int set_label(char *btrfs_dev, char *nLabel);
+int get_subvol_label(char *subvol, char *labelp);
+int set_subvol_label(char *subvol, char *labelp);
diff --git a/ctree.h b/ctree.h
index 12f8fe3..bacb7d1 100644
--- a/ctree.h
+++ b/ctree.h
@@ -332,6 +332,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -721,7 +722,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-        __le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+        __le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
diff --git a/ioctl.h b/ioctl.h
index 3e7e451..4cbae3a 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -524,6 +524,8 @@ struct btrfs_ioctl_clone_range_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 
 #ifdef __cplusplus
 }
diff --git a/print-tree.c b/print-tree.c
index c9e891b..3d097dd 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -366,6 +366,8 @@ static void print_root(struct extent_buffer *leaf, int slot)
 				btrfs_root_stransid(&root_item),
 				btrfs_root_rtransid(&root_item));
 		}
+		if (strlen(root_item.label))
+			printf("\t\tlabel %s\n", root_item.label);
 	}
 	if (btrfs_root_refs(&root_item) == 0) {
 		struct btrfs_key drop_key;
-- 
1.8.1.227.g44fe835


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 2/2 v4] Btrfs-progs: cmd option to show or set the subvol label
  2013-02-25  5:31 ` [PATCH 0/2 v4] Btrfs-progs: Add support for " Anand Jain
  2013-02-25  5:31   ` [PATCH 1/2 v4] Btrfs-progs: add feature to label subvol and snapshot Anand Jain
@ 2013-02-25  5:31   ` Anand Jain
  1 sibling, 0 replies; 41+ messages in thread
From: Anand Jain @ 2013-02-25  5:31 UTC (permalink / raw)
  To: linux-btrfs

This adds the command option label to the subvol sub-command,
this is a generic patch which will stay irrespective of which
approach we take, that is the ioctl-way or the attributes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-subvolume.c | 37 +++++++++++++++++++++++++++++++++++++
 man/btrfs.8.in   |  6 ++++++
 2 files changed, 43 insertions(+)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ea128fc..1951c51 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -35,12 +35,48 @@
 #include "utils.h"
 #include "btrfs-list.h"
 #include "utils.h"
+#include "btrfslabel.h"
 
 static const char * const subvolume_cmd_group_usage[] = {
 	"btrfs subvolume <command> <args>",
 	NULL
 };
 
+static const char * const cmd_subvol_label_usage[] = {
+	"btrfs subvolume label <path> [label]",
+	"Show or set label for the subvol or snapshot",
+	NULL
+};
+
+static int cmd_subvol_label(int argc, char **argv)
+{
+	struct stat st;
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	int ret;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+		usage(cmd_subvol_label_usage);
+
+	if (stat(argv[1], &st) < 0) {
+		fprintf(stderr, "Error: %s\n",strerror(errno));
+		return -errno;
+	}
+	if (!S_ISDIR(st.st_mode)) {
+		fprintf(stderr, "Error: Not a dir\n");
+		return -1;
+	}
+	if (argc > 2)
+		return set_subvol_label(argv[1], argv[2]);
+	else {
+		ret = get_subvol_label(argv[1], label);
+		if (ret)
+			return ret;
+		label[BTRFS_SUBVOL_LABEL_SIZE]=0;
+		printf("%s\n",label);
+	}
+	return 0;
+}
+
 /*
  * test if path is a directory
  * this function return
@@ -933,6 +969,7 @@ const struct cmd_group subvolume_cmd_group = {
 			cmd_subvol_set_default_usage, NULL, 0 },
 		{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
 		{ "show", cmd_subvol_show, cmd_subvol_show_usage, NULL, 0 },
+		{ "label", cmd_subvol_label, cmd_subvol_label_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }
 	}
 };
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 94f4ffe..cd2e4e5 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -17,6 +17,8 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBsubvolume get-default\fP\fI <path>\fP
 .PP
+\fBbtrfs\fP \fBsubvolume label\fP\fI <path> [label]\fP
+.PP
 \fBbtrfs\fP \fBsubvolume find-new\fP\fI <subvolume> <last_gen>\fP
 .PP
 \fBbtrfs\fP \fBsubvolume show\fP\fI <path>\fP
@@ -177,6 +179,10 @@ Get the default subvolume of the filesystem \fI<path>\fR. The output format
 is similar to \fBsubvolume list\fR command.
 .TP
 
+\fBsubvolume label\fR\fI <path> [label]\fR
+Show or set \fI[label]\fR for the subvolume or the snapshot \fI<path>\fR.
+.TP
+
 \fBsubvolume find-new\fR\fI <subvolume> <last_gen>\fR
 List the recently modified files in a subvolume, after \fI<last_gen>\fR ID.
 .TP
-- 
1.8.1.227.g44fe835


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v4] Btrfs: Add support for subvol label
  2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
                   ` (7 preceding siblings ...)
  2013-02-25  5:31 ` [PATCH 0/2 v4] Btrfs-progs: Add support for " Anand Jain
@ 2013-02-25  5:31 ` Anand Jain
  2013-02-25  5:31   ` [PATCH v4] Btrfs: ability to add label to snapshot and subvol Anand Jain
  8 siblings, 1 reply; 41+ messages in thread
From: Anand Jain @ 2013-02-25  5:31 UTC (permalink / raw)
  To: linux-btrfs

This is the btrfs kernel changes to add supprt for the
subvol label.

The related (v4) Btrfs-progs patches are 

  Btrfs-progs: add feature to label subvol and snapshot
  Btrfs-progs: cmd option to show or set the subvol label

v4: rebased to Josef Bacik btrfs-next

Anand Jain (1):
  Btrfs: ability to add label to snapshot and subvol

 fs/btrfs/ctree.h           | 12 +++++++++-
 fs/btrfs/ioctl.c           | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/transaction.c     |  1 +
 include/uapi/linux/btrfs.h |  2 ++
 4 files changed, 72 insertions(+), 1 deletion(-)

-- 
1.8.1.227.g44fe835


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v4] Btrfs: ability to add label to snapshot and subvol
  2013-02-25  5:31 ` [PATCH v4] Btrfs: Add support for " Anand Jain
@ 2013-02-25  5:31   ` Anand Jain
  2013-02-25 20:45     ` Hugo Mills
  0 siblings, 1 reply; 41+ messages in thread
From: Anand Jain @ 2013-02-25  5:31 UTC (permalink / raw)
  To: linux-btrfs

Generally snapshots are machine generated, so at any point in time
if a sysadmin looks at a list of snapshots there should be some
info about the snapshots to indicate purpose of it being created.
With this patch and along with the corresponding btrfs-progs patch
a 32byte info can be added to the snapshots/subvol.

Further, ioctl is preferred over the attribute to write the label
since btrfs-progs is not only the application which might be
interacting with the subvol to write the label, for example
btrfs-gui might as well write the label, so that needs an
additional efforts to maintain a consistent keyword across the
applications is difficult in the long run.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ctree.h           | 12 +++++++++-
 fs/btrfs/ioctl.c           | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/transaction.c     |  1 +
 include/uapi/linux/btrfs.h |  2 ++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1679051..6b527bc 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -392,6 +392,7 @@ struct btrfs_header {
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
 #define BTRFS_LABEL_SIZE 256
+#define BTRFS_SUBVOL_LABEL_SIZE 32
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
@@ -769,7 +770,8 @@ struct btrfs_root_item {
 	struct btrfs_timespec otime;
 	struct btrfs_timespec stime;
 	struct btrfs_timespec rtime;
-	__le64 reserved[8]; /* for future */
+	char label[BTRFS_SUBVOL_LABEL_SIZE];
+	__le64 reserved[4]; /* for future */
 } __attribute__ ((__packed__));
 
 /*
@@ -2546,6 +2548,14 @@ static inline bool btrfs_root_readonly(struct btrfs_root *root)
 {
 	return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_RDONLY)) != 0;
 }
+static inline char * btrfs_root_label(struct btrfs_root_item *root_item)
+{
+	return (root_item->label);
+}
+static inline void btrfs_root_set_label(struct btrfs_root_item *root_item, char *val)
+{
+	memcpy(root_item->label, val, BTRFS_SUBVOL_LABEL_SIZE);
+}
 
 /* struct btrfs_root_backup */
 BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 2bbbed5..211b319 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3982,6 +3982,60 @@ out_unlock:
 	return ret;
 }
 
+static int btrfs_ioctl_subvol_getlabel(struct btrfs_root *root,
+						void __user *arg)
+{
+	char *label;
+
+	spin_lock(&root->root_item_lock);
+	label = btrfs_root_label(&root->root_item);
+	if (copy_to_user(arg, label, BTRFS_SUBVOL_LABEL_SIZE)) {
+		spin_unlock(&root->root_item_lock);
+		return -EFAULT;
+	}
+	spin_unlock(&root->root_item_lock);
+	return 0;
+}
+
+static int btrfs_ioctl_subvol_setlabel(struct file *file,
+						void __user *arg)
+{
+	char label[BTRFS_SUBVOL_LABEL_SIZE+1];
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root;
+	struct inode *inode = file->f_path.dentry->d_inode;
+	int ret;
+
+	if (btrfs_root_readonly(root))
+		return -EROFS;
+
+	if (!inode_owner_or_capable(inode))
+		return -EACCES;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	if (copy_from_user(label, arg, BTRFS_SUBVOL_LABEL_SIZE)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	trans = btrfs_join_transaction(root);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out;
+	}
+	spin_lock(&root->root_item_lock);
+	btrfs_root_set_label(&root->root_item, label);
+	spin_unlock(&root->root_item_lock);
+	btrfs_end_transaction(trans, root);
+
+out:
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 long btrfs_ioctl(struct file *file, unsigned int
 		cmd, unsigned long arg)
 {
@@ -4086,6 +4140,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_fslabel(file, argp);
 	case BTRFS_IOC_SET_FSLABEL:
 		return btrfs_ioctl_set_fslabel(file, argp);
+	case BTRFS_IOC_SUBVOL_GETLABEL:
+		return btrfs_ioctl_subvol_getlabel(root, argp);
+	case BTRFS_IOC_SUBVOL_SETLABEL:
+		return btrfs_ioctl_subvol_setlabel(file, argp);
 	}
 
 	return -ENOTTY;
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 955204c..955097e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1181,6 +1181,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	memset(&new_root_item->rtime, 0, sizeof(new_root_item->rtime));
 	btrfs_set_root_stransid(new_root_item, 0);
 	btrfs_set_root_rtransid(new_root_item, 0);
+	memset(&new_root_item->label, 0, BTRFS_SUBVOL_LABEL_SIZE);
 
 	old = btrfs_lock_root_node(root);
 	ret = btrfs_cow_block(trans, root, old, NULL, 0, &old);
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index fa3a5f9..38d0f56 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -510,5 +510,7 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_SUBVOL_GETLABEL _IOWR(BTRFS_IOCTL_MAGIC, 55, __u64)
+#define BTRFS_IOC_SUBVOL_SETLABEL _IOW(BTRFS_IOCTL_MAGIC, 56, __u64)
 
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
1.8.1.227.g44fe835


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH v4] Btrfs: ability to add label to snapshot and subvol
  2013-02-25  5:31   ` [PATCH v4] Btrfs: ability to add label to snapshot and subvol Anand Jain
@ 2013-02-25 20:45     ` Hugo Mills
  2013-03-01 10:36       ` Anand Jain
  0 siblings, 1 reply; 41+ messages in thread
From: Hugo Mills @ 2013-02-25 20:45 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

On Mon, Feb 25, 2013 at 01:31:45PM +0800, Anand Jain wrote:
> Generally snapshots are machine generated, so at any point in time
> if a sysadmin looks at a list of snapshots there should be some
> info about the snapshots to indicate purpose of it being created.

   I still can't see what benefits you get from having this extra
metadata that you couldn't put in a filename -- particularly in the
case of a scripted snapshot. However, assuming that you can bend the
argument to needing this feature...

> With this patch and along with the corresponding btrfs-progs patch
> a 32byte info can be added to the snapshots/subvol.

   How is this any different to using extended attributes to store the
same information? With the exception that xattrs:

 - can store more than 32 bytes
 - can store an arbitrary number of items 
 - have already been implemented
 - don't introduce more FS-specific ioctls
 - don't take up valuable extra space in metadata structures

> Further, ioctl is preferred over the attribute to write the label
> since btrfs-progs is not only the application which might be
> interacting with the subvol to write the label, for example
> btrfs-gui might as well write the label, so that needs an
> additional efforts to maintain a consistent keyword across the
> applications is difficult in the long run.

   As long as there's a well-known xattr name that's written by
btrfs-progs, that'll become the de-facto standard. If any other system
fails to use that attribute name, it becomes non-interoperable with
the default tools. This is probably enough to ensure that it'll get
fixed fairly quickly in this case (because the users that care about
the feature will complain it's not working right).

   My conclusion: go with user xattrs.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
         --- Welcome to Hollywood, a land just off the coast ---         
                            of Planet Earth.                             

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v4] Btrfs: ability to add label to snapshot and subvol
  2013-02-25 20:45     ` Hugo Mills
@ 2013-03-01 10:36       ` Anand Jain
  0 siblings, 0 replies; 41+ messages in thread
From: Anand Jain @ 2013-03-01 10:36 UTC (permalink / raw)
  To: Hugo Mills, linux-btrfs



  As long as we integrate, broadcast and use single
  keyword for a purpose I am fine with using xattr.
  Patch using xattr was posted as well.

  Just a note, potential applications using snapshot
  label are:
    - Yum, btrfs-progs, snapper, btrfs-gui,
    Gnome-Nautilus-snapshot-plugin,
    enterprise backup-scripts, PHP-scripts using MYsql.


>     As long as there's a well-known xattr name that's written by
> btrfs-progs, that'll become the de-facto standard. If any other system
> fails to use that attribute name, it becomes non-interoperable with
> the default tools. This is probably enough to ensure that it'll get
> fixed fairly quickly in this case (because the users that care about
> the feature will complain it's not working right).
>
>     My conclusion: go with user xattrs.


Thanks.

  Any comment on the patch titled
    Btrfs-progs: add attribute label for subvol and snapshot

will help integration.

-Anand

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2013-03-01 10:36 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-01 10:46 [Request for review] [RFC] Add label support for snapshots and subvols Anand jain
2012-11-01 10:46 ` [PATCH 1/2] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
2012-11-01 10:46 ` [PATCH 2/2] Btrfs-progs: add feature to label subvol and snapshot Anand jain
2012-11-01 10:46 ` [PATCH] Btrfs: add label to snapshot and subvol Anand jain
2012-11-05  7:39   ` Jan Schmidt
2012-11-16  5:15     ` Anand Jain
2012-11-01 22:16 ` [Request for review] [RFC] Add label support for snapshots and subvols cwillu
2012-11-01 22:28   ` Fajar A. Nugraha
2012-11-01 22:32     ` Hugo Mills
2012-11-01 22:49       ` Fajar A. Nugraha
2012-11-05  7:24         ` Anand Jain
2012-11-05 23:22           ` David Sterba
2012-11-01 23:04     ` Goffredo Baroncelli
2012-11-16  4:52 ` [Request for review v2] " Anand jain
2012-11-16  4:52   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
2012-11-16  4:52   ` [PATCH v2] Btrfs: add label to snapshot and subvol Anand jain
2012-11-16  6:33     ` Miao Xie
2012-11-27 18:26       ` Anand Jain
2012-11-16  4:52   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
2012-11-16  4:52   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
2012-11-27 18:29 ` [Request for review v3] [RFC] Add label support for snapshots and subvols Anand jain
2012-11-27 18:29   ` [PATCH v3] Btrfs: add label to snapshot and subvol Anand jain
2012-11-28  3:05     ` Miao Xie
2012-11-28 10:02       ` Anand Jain
2012-11-28 10:25         ` Miao Xie
2012-11-28 12:17           ` Anand Jain
2012-11-28 12:15     ` Anand jain
2012-11-27 18:29   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
2012-11-27 18:29   ` [PATCH 2/3] Btrfs-progs: add feature to label subvol and snapshot Anand jain
2012-11-27 18:29   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
2012-11-30 15:47 ` [PATCH 0/3] Add subvol and snapshot attribute label Anand jain
2012-11-30 15:48   ` [PATCH 1/3] Btrfs-progs: move open_file_or_dir() to utils.c Anand jain
2012-11-30 15:48   ` [PATCH 2/3] Btrfs-progs: add attribute label for subvol and snapshot Anand jain
2012-11-30 15:48   ` [PATCH 3/3] Btrfs-progs: cmd option to show or set the subvol label Anand jain
2013-02-25  5:31 ` [PATCH 0/2 v4] Btrfs-progs: Add support for " Anand Jain
2013-02-25  5:31   ` [PATCH 1/2 v4] Btrfs-progs: add feature to label subvol and snapshot Anand Jain
2013-02-25  5:31   ` [PATCH 2/2 v4] Btrfs-progs: cmd option to show or set the subvol label Anand Jain
2013-02-25  5:31 ` [PATCH v4] Btrfs: Add support for " Anand Jain
2013-02-25  5:31   ` [PATCH v4] Btrfs: ability to add label to snapshot and subvol Anand Jain
2013-02-25 20:45     ` Hugo Mills
2013-03-01 10:36       ` Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.