All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Add support for object properties
@ 2013-11-12 13:41 Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it Filipe David Borba Manana
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

This is a revised version of the original proposal/work from Alexander Block
to introduce a generic framework to set properties on btrfs filesystem objects
(inodes, subvolumes, filesystems, devices).

I preserved most of Alexander's work, rebasing only his main patch against
latest integration branch, adding some missing static qualifiers, malloc failure
checks and fixing some checkpatch.pl warnings. On top of his work, I added a few
fixes and enhancements and added support for 1 inode property, named compression,
which requires the corresponding kernel patch.

Once there's agreement on this feature and patchset, I'll send my xfstests patch
that tests this feature.

Alexander's original cover letter:

"
        This patchset introduces the btrfs property subgroup. It is the
        result of a discussion we had on IRC. I tried to make the properties
        interface as generic and extensible as possible. Comments are welcome.

        Currently the command group looks like this:
        btrfs prop set [-t <type>] /path/to/object <name> <value>
        btrfs prop get [-t <type>] /path/to/object [<name>] (omitting name dumps all)
        btrfs prop list [-t <type>] /path/to/object (lists properties with description)

        The type is used to explicitly specify what type of object you mean. This is 
        necessary in case the object+property combination is ambiguous.  For example
        '/path/to/fs/root' could mean the root subvolume, the directory inode or the 
        filesystem itself. Normally, btrfs-progs will try to detect the type 
        automatically.

        David suggested that it should also be possible to specify objects by
        their id/uuid/fsid. I like that idea, but would be happy if someone else
        could take over that part :)

        For now, I've implemented two properties:

        1. read-only. Usable on subvolumes to toggle the read-only flags.
        2. label. I looked through btrfs to find good examples of things that
           could be moved to the new properties interface and the filesystem
           label looked like a good one. There are for sure more, but that is
           something for later (and maybe for someone else). I would suggest 
           to move everthing that makes sense over to the props interface and 
           mark the old interfaces as deprecated. Comments on this are welcome.

        Patch version history:
        v1
          Initial version.
        v2
          - Removed the filesystem prefix and implemented it as new command group
          - Switched from the <name>[=<value>] form to the set/get <name> [<value>] 
            form.
          - Removed patches "Btrfs-progs: make filesystem_cmd_group non const"
            and "Btrfs-progs: move skip_prefix and prefixcmp to utils.c". They
            are not needed anymore due to the 'btrfs prop list' command.
          - Udjusted the subvol flags patch to be compatible to the "Btrfs: use 
            _IOR for BTRFS_IOC_SUBVOL_GETFLAGS" patch.
          - Using -t <type> instead of <type>: prefix now.
          - Changes are based on feedback from Ilya and David.

        Alex.
"

Thanks.


Alexander Block (1):
  Btrfs-progs: introduce btrfs property subgroup

Filipe David Borba Manana (4):
  Btrfs-progs: let get_label return the label instead of printing it
  Btrfs-progs: fix detection of root objects in cmds-property.c
  Btrfs-progs: add type root to label property
  Btrfs-progs: add support for the compression property

 Makefile          |    4 +-
 btrfs.c           |    1 +
 cmds-filesystem.c |   14 +-
 cmds-property.c   |  467 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 commands.h        |    2 +
 ioctl.h           |   12 ++
 props.c           |  184 +++++++++++++++++++++
 props.h           |   43 +++++
 utils.c           |   15 +-
 utils.h           |    2 +-
 10 files changed, 729 insertions(+), 15 deletions(-)
 create mode 100644 cmds-property.c
 create mode 100644 props.c
 create mode 100644 props.h

-- 
1.7.9.5


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

* [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
@ 2013-11-12 13:41 ` Filipe David Borba Manana
  2013-11-13 15:43   ` David Sterba
  2013-11-12 13:41 ` [PATCH 2/5] Btrfs-progs: introduce btrfs property subgroup Filipe David Borba Manana
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

get_label prints the label at the moment. Change this so that
the label is returned and printing is done by the caller.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 cmds-filesystem.c |   14 +++++++++++---
 utils.c           |   15 ++++++---------
 utils.h           |    2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index d2cad81..9fda41a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -810,10 +810,18 @@ static int cmd_label(int argc, char **argv)
 	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
 		usage(cmd_label_usage);
 
-	if (argc > 2)
+	if (argc > 2) {
 		return set_label(argv[1], argv[2]);
-	else
-		return get_label(argv[1]);
+	} else {
+		char label[BTRFS_LABEL_SIZE];
+		int ret;
+
+		ret = get_label(argv[1], label);
+		if (!ret)
+			fprintf(stdout, "%s\n", label);
+
+		return ret;
+	}
 }
 
 const struct cmd_group filesystem_cmd_group = {
diff --git a/utils.c b/utils.c
index 85682d9..9105af1 100644
--- a/utils.c
+++ b/utils.c
@@ -1343,7 +1343,7 @@ static int set_label_mounted(const char *mount_path, const char *label)
 	return 0;
 }
 
-static int get_label_unmounted(const char *dev)
+static int get_label_unmounted(const char *dev, char *label)
 {
 	struct btrfs_root *root;
 	int ret;
@@ -1366,7 +1366,7 @@ static int get_label_unmounted(const char *dev)
 	if(!root)
 		return -1;
 
-	fprintf(stdout, "%s\n", root->fs_info->super_copy->label);
+	memcpy(label, root->fs_info->super_copy->label, BTRFS_LABEL_SIZE);
 
 	/* Now we close it since we are done. */
 	close_ctree(root);
@@ -1401,18 +1401,15 @@ int get_label_mounted(const char *mount_path, char *labelp)
 	return 0;
 }
 
-int get_label(const char *btrfs_dev)
+int get_label(const char *btrfs_dev, char *label)
 {
 	int ret;
-	char label[BTRFS_LABEL_SIZE];
 
 	if (is_existing_blk_or_reg_file(btrfs_dev))
-		ret = get_label_unmounted(btrfs_dev);
-	else {
+		ret = get_label_unmounted(btrfs_dev, label);
+	else
 		ret = get_label_mounted(btrfs_dev, label);
-		if (!ret)
-			fprintf(stdout, "%s\n", label);
-	}
+
 	return ret;
 }
 
diff --git a/utils.h b/utils.h
index 6f4b10c..b38bd3a 100644
--- a/utils.h
+++ b/utils.h
@@ -72,7 +72,7 @@ int open_file_or_dir(const char *fname, DIR **dirstream);
 void close_file_or_dir(int fd, DIR *dirstream);
 int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 		struct btrfs_ioctl_dev_info_args **di_ret);
-int get_label(const char *btrfs_dev);
+int get_label(const char *btrfs_dev, char *label);
 int set_label(const char *btrfs_dev, const char *label);
 
 char *__strncpy__null(char *dest, const char *src, size_t n);
-- 
1.7.9.5


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

* [PATCH 2/5] Btrfs-progs: introduce btrfs property subgroup
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it Filipe David Borba Manana
@ 2013-11-12 13:41 ` Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 3/5] Btrfs-progs: fix detection of root objects in cmds-property.c Filipe David Borba Manana
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Alexander Block, Filipe David Borba Manana

From: Alexander Block <ablock84@googlemail.com>

"btrfs filesystem property" is a generic interface to set/get
properties on filesystem objects (inodes/subvolumes/filesystems
/devs).

This patch adds the generic framework for properties and also
implements two properties. The first is the read-only property
for subvolumes and the second is the label property for devices.

Signed-off-by: Alexander Block <ablock84@googlemail.com>
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 Makefile        |    4 +-
 btrfs.c         |    1 +
 cmds-property.c |  459 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 commands.h      |    2 +
 props.c         |  110 +++++++++++++
 props.h         |   43 ++++++
 6 files changed, 617 insertions(+), 2 deletions(-)
 create mode 100644 cmds-property.c
 create mode 100644 props.c
 create mode 100644 props.h

diff --git a/Makefile b/Makefile
index 039fd81..eef0060 100644
--- a/Makefile
+++ b/Makefile
@@ -9,12 +9,12 @@ CFLAGS = -g -O1
 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  root-tree.o dir-item.o file-item.o inode-item.o inode-map.o \
 	  extent-cache.o extent_io.o volumes.o utils.o repair.o \
-	  qgroup.o raid6.o free-space-cache.o uuid-tree.o list_sort.o
+	  qgroup.o raid6.o free-space-cache.o uuid-tree.o list_sort.o props.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o \
 	       cmds-restore.o cmds-rescue.o chunk-recover.o super-recover.o \
-	       cmds-dedup.o
+	       cmds-dedup.o cmds-property.o
 libbtrfs_objects = send-stream.o send-utils.o rbtree.o btrfs-list.o crc32c.o \
 		   uuid-tree.o
 libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \
diff --git a/btrfs.c b/btrfs.c
index dfae35f..16458ef 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -250,6 +250,7 @@ static const struct cmd_group btrfs_cmd_group = {
 		{ "rescue", cmd_rescue, NULL, &rescue_cmd_group, 0 },
 		{ "restore", cmd_restore, cmd_restore_usage, NULL, 0 },
 		{ "inspect-internal", cmd_inspect, NULL, &inspect_cmd_group, 0 },
+		{ "property", cmd_property, NULL, &property_cmd_group, 0 },
 		{ "send", cmd_send, cmd_send_usage, NULL, 0 },
 		{ "receive", cmd_receive, cmd_receive_usage, NULL, 0 },
 		{ "quota", cmd_quota, NULL, &quota_cmd_group, 0 },
diff --git a/cmds-property.c b/cmds-property.c
new file mode 100644
index 0000000..6699e61
--- /dev/null
+++ b/cmds-property.c
@@ -0,0 +1,459 @@
+/*
+ * 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 <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+
+#include "commands.h"
+#include "props.h"
+#include "ctree.h"
+
+static const char * const property_cmd_group_usage[] = {
+	"btrfs property get/set/list [-t <type>] <object> [<name>] [value]",
+	NULL
+};
+
+static const char * const cmd_get_usage[] = {
+	"btrfs property get [-t <type>] <object> [<name>]",
+	"Gets a property from a btrfs object.",
+	"If no name is specified, all properties for the given object are",
+	"printed.",
+	"A filesystem object can be a the filesystem itself, a subvolume,",
+	"an inode or a device. The '-t <type>' option can be used to explicitly",
+	"specify what type of object you meant. This is only needed when a",
+	"property could be set for more then one object type. Possible types",
+	"are s[ubvol], f[ilesystem], i[node] and d[evice].",
+	NULL
+};
+
+static const char * const cmd_set_usage[] = {
+	"btrfs property set [-t <type>] <object> <name> <value>",
+	"Sets a property on a btrfs object.",
+	"Please see the help of 'btrfs property get' for a description of",
+	"objects and object types.",
+	NULL
+};
+
+static const char * const cmd_list_usage[] = {
+	"btrfs property list [-t <type>] <object>",
+	"Lists available properties with their descriptions for the given object.",
+	"Please see the help of 'btrfs property get' for a description of",
+	"objects and object types.",
+	NULL
+};
+
+static int parse_prop(const char *arg, const struct prop_handler *props,
+		      const struct prop_handler **prop_ret)
+{
+	const struct prop_handler *prop = props;
+
+	for (; prop->name; prop++) {
+		if (!strcmp(prop->name, arg)) {
+			*prop_ret = prop;
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+static int get_fsid(const char *path, u8 *fsid)
+{
+	int ret;
+	int fd;
+	struct btrfs_ioctl_fs_info_args args;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: open %s failed. %s\n", path,
+				strerror(-ret));
+		goto out;
+	}
+
+	ret = ioctl(fd, BTRFS_IOC_FS_INFO, &args);
+	if (ret < 0) {
+		ret = -errno;
+		goto out;
+	}
+
+	memcpy(fsid, args.fsid, BTRFS_FSID_SIZE);
+	ret = 0;
+
+out:
+	if (fd != -1)
+		close(fd);
+	return ret;
+}
+
+static int check_btrfs_object(const char *object)
+{
+	int ret;
+	u8 fsid[BTRFS_FSID_SIZE];
+
+	ret = get_fsid(object, fsid);
+	if (ret < 0)
+		ret = 0;
+	else
+		ret = 1;
+	return ret;
+}
+
+static int check_is_root(const char *object)
+{
+	int ret;
+	u8 fsid[BTRFS_FSID_SIZE];
+	u8 fsid2[BTRFS_FSID_SIZE];
+	char *tmp;
+
+	tmp = malloc(strlen(object) + 5);
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	strcpy(tmp, object);
+	if (tmp[strlen(tmp) - 1] != '/')
+		strcat(tmp, "/");
+	strcat(tmp, "..");
+
+	ret = get_fsid(object, fsid);
+	if (ret < 0) {
+		fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", object,
+				strerror(-ret));
+		goto out;
+	}
+
+	ret = get_fsid(tmp, fsid2);
+	if (ret < 0) {
+		ret = 0;
+		goto out;
+	}
+
+	if (!memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
+		ret = 0;
+		goto out;
+	}
+
+	ret = 1;
+
+out:
+	free(tmp);
+	return ret;
+}
+
+static int count_bits(int v)
+{
+	unsigned int tmp = (unsigned int)v;
+	int cnt = 0;
+
+	while (tmp) {
+		if (tmp & 1)
+			cnt++;
+		tmp >>= 1;
+	}
+	return cnt;
+}
+
+static int autodetect_object_types(const char *object, int *types_out)
+{
+	int ret;
+	int is_btrfs_object;
+	int types = 0;
+	struct stat st;
+
+	is_btrfs_object = check_btrfs_object(object);
+
+	ret = lstat(object, &st);
+	if (ret < 0) {
+		ret = -errno;
+		goto out;
+	}
+
+	if (is_btrfs_object) {
+		types |= prop_object_inode;
+		if (st.st_ino == BTRFS_FIRST_FREE_OBJECTID)
+			types |= prop_object_subvol;
+
+		ret = check_is_root(object);
+		if (ret < 0)
+			goto out;
+		if (ret)
+			types |= prop_object_root;
+	}
+
+	if (S_ISBLK(st.st_mode))
+		types |= prop_object_dev;
+
+	ret = 0;
+	*types_out = types;
+
+out:
+	return ret;
+}
+
+static int print_prop_help(const struct prop_handler *prop)
+{
+	fprintf(stdout, "%-20s%s\n", prop->name, prop->desc);
+	return 0;
+}
+
+static int dump_prop(const struct prop_handler *prop,
+		     const char *object,
+		     int types,
+		     int type,
+		     int name_and_help)
+{
+	int ret = 0;
+
+	if ((types & type) && (prop->types & type)) {
+		if (!name_and_help)
+			ret = prop->handler(type, object, prop->name, NULL);
+		else
+			ret = print_prop_help(prop);
+	}
+	return ret;
+}
+
+static int dump_props(int types, const char *object, int name_and_help)
+{
+	int ret;
+	int i;
+	int j;
+	const struct prop_handler *prop;
+
+	for (i = 0; prop_handlers[i].name; i++) {
+		prop = &prop_handlers[i];
+		for (j = 1; j < __prop_object_max; j <<= 1) {
+			ret = dump_prop(prop, object, types, j, name_and_help);
+			if (ret < 0) {
+				ret = 50;
+				goto out;
+			}
+		}
+	}
+
+	ret = 0;
+
+out:
+	return ret;
+}
+
+static int setget_prop(int types, const char *object,
+		       const char *name, const char *value)
+{
+	int ret;
+	const struct prop_handler *prop = NULL;
+
+	ret = parse_prop(name, prop_handlers, &prop);
+	if (ret == -1) {
+		fprintf(stderr, "ERROR: property is unknown\n");
+		ret = 40;
+		goto out;
+	} else if (ret) {
+		fprintf(stderr, "ERROR: parse_prop reported unknown error\n");
+		ret = 42;
+		goto out;
+	}
+
+	types &= prop->types;
+	if (!types) {
+		fprintf(stderr,
+			"ERROR: object is not compatible with property\n");
+		ret = 47;
+		goto out;
+	}
+
+	if (count_bits(types) > 1) {
+		fprintf(stderr,
+			"ERROR: type of object is ambiguous. Please specify a type by hand.\n");
+		ret = 48;
+		goto out;
+	}
+
+	if (value && prop->read_only) {
+		fprintf(stderr, "ERROR: %s is a read-only property.\n",
+				prop->name);
+		ret = 51;
+		goto out;
+	}
+
+	ret = prop->handler(types, object, name, value);
+
+	if (ret < 0)
+		ret = 50;
+	else
+		ret = 0;
+
+out:
+	return ret;
+
+}
+
+static void parse_args(int argc, char **argv,
+		       const char * const *usage_str,
+		       int *types, char **object,
+		       char **name, char **value)
+{
+	int ret;
+	char *type_str = NULL;
+
+	optind = 1;
+	while (1) {
+		int c = getopt(argc, argv, "t:");
+		if (c < 0)
+			break;
+
+		switch (c) {
+		case 't':
+			type_str = optarg;
+			break;
+		default:
+			usage(usage_str);
+			break;
+		}
+	}
+
+	*types = 0;
+	if (type_str) {
+		if (!strcmp(type_str, "s") || !strcmp(type_str, "subvol")) {
+			*types = prop_object_subvol;
+		} else if (!strcmp(type_str, "f") ||
+			   !strcmp(type_str, "filesystem")) {
+			*types = prop_object_root;
+		} else if (!strcmp(type_str, "i") ||
+			   !strcmp(type_str, "inode")) {
+			*types = prop_object_inode;
+		} else if (!strcmp(type_str, "d") ||
+			   !strcmp(type_str, "device")) {
+			*types = prop_object_dev;
+		} else {
+			fprintf(stderr, "ERROR: invalid object type.\n");
+			usage(usage_str);
+		}
+	}
+
+	if (object && optind < argc)
+		*object = argv[optind++];
+	if (name && optind < argc)
+		*name = argv[optind++];
+	if (value && optind < argc)
+		*value = argv[optind++];
+
+	if (optind != argc) {
+		fprintf(stderr, "ERROR: invalid arguments.\n");
+		usage(usage_str);
+	}
+
+	if (!*types && object && *object) {
+		ret = autodetect_object_types(*object, types);
+		if (ret < 0) {
+			fprintf(stderr,
+				"ERROR: failed to detect object type. %s\n",
+				strerror(-ret));
+			usage(usage_str);
+		}
+		if (!*types) {
+			fprintf(stderr,
+				"ERROR: object is not a btrfs object.\n");
+			usage(usage_str);
+		}
+	}
+}
+
+static int cmd_get(int argc, char **argv)
+{
+	int ret;
+	char *object;
+	char *name = NULL;
+	int types = 0;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 4))
+		usage(cmd_get_usage);
+
+	parse_args(argc, argv, cmd_get_usage, &types, &object, &name, NULL);
+	if (!object) {
+		fprintf(stderr, "ERROR: invalid arguments.\n");
+		usage(cmd_set_usage);
+	}
+
+	if (name)
+		ret = setget_prop(types, object, name, NULL);
+	else
+		ret = dump_props(types, object, 0);
+
+	return ret;
+}
+
+static int cmd_set(int argc, char **argv)
+{
+	int ret;
+	char *object;
+	char *name;
+	char *value;
+	int types = 0;
+
+	if (check_argc_min(argc, 4) || check_argc_max(argc, 5))
+		usage(cmd_set_usage);
+
+	parse_args(argc, argv, cmd_set_usage, &types, &object, &name, &value);
+	if (!object || !name || !value) {
+		fprintf(stderr, "ERROR: invalid arguments.\n");
+		usage(cmd_set_usage);
+	}
+
+	ret = setget_prop(types, object, name, value);
+
+	return ret;
+}
+
+static int cmd_list(int argc, char **argv)
+{
+	int ret;
+	char *object = NULL;
+	int types = 0;
+
+	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
+		usage(cmd_list_usage);
+
+	parse_args(argc, argv, cmd_list_usage, &types, &object, NULL, NULL);
+	if (!object) {
+		fprintf(stderr, "ERROR: invalid arguments.\n");
+		usage(cmd_set_usage);
+	}
+
+	ret = dump_props(types, object, 1);
+
+	return ret;
+}
+
+const struct cmd_group property_cmd_group = {
+	property_cmd_group_usage, NULL, {
+		{ "get", cmd_get, cmd_get_usage, NULL, 0 },
+		{ "set", cmd_set, cmd_set_usage, NULL, 0 },
+		{ "list", cmd_list, cmd_list_usage, NULL, 0 },
+		{ 0, 0, 0, 0, 0 },
+	}
+};
+
+int cmd_property(int argc, char **argv)
+{
+	return handle_command_group(&property_cmd_group, argc, argv);
+}
diff --git a/commands.h b/commands.h
index 6fccc15..9ae2e10 100644
--- a/commands.h
+++ b/commands.h
@@ -87,6 +87,7 @@ extern const struct cmd_group balance_cmd_group;
 extern const struct cmd_group device_cmd_group;
 extern const struct cmd_group scrub_cmd_group;
 extern const struct cmd_group inspect_cmd_group;
+extern const struct cmd_group property_cmd_group;
 extern const struct cmd_group quota_cmd_group;
 extern const struct cmd_group qgroup_cmd_group;
 extern const struct cmd_group replace_cmd_group;
@@ -110,6 +111,7 @@ int cmd_check(int argc, char **argv);
 int cmd_chunk_recover(int argc, char **argv);
 int cmd_super_recover(int argc, char **argv);
 int cmd_inspect(int argc, char **argv);
+int cmd_property(int argc, char **argv);
 int cmd_send(int argc, char **argv);
 int cmd_receive(int argc, char **argv);
 int cmd_quota(int argc, char **argv);
diff --git a/props.c b/props.c
new file mode 100644
index 0000000..763fd57
--- /dev/null
+++ b/props.c
@@ -0,0 +1,110 @@
+/*
+ * 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/stat.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "ctree.h"
+#include "commands.h"
+#include "utils.h"
+#include "props.h"
+
+static int prop_read_only(enum prop_object_type type,
+			  const char *object,
+			  const char *name,
+			  const char *value)
+{
+	int ret = 0;
+	int fd = -1;
+	u64 flags = 0;
+
+	fd = open(object, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: open %s failed. %s\n",
+				object, strerror(-ret));
+		goto out;
+	}
+
+	ret = ioctl(fd, BTRFS_IOC_SUBVOL_GETFLAGS, &flags);
+	if (ret < 0) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: failed to get flags for %s. %s\n",
+				object, strerror(-ret));
+		goto out;
+	}
+
+	if (!value) {
+		if (flags & BTRFS_SUBVOL_RDONLY)
+			fprintf(stdout, "ro=true\n");
+		else
+			fprintf(stdout, "ro=false\n");
+		ret = 0;
+		goto out;
+	}
+
+	if (!strcmp(value, "true")) {
+		flags |= BTRFS_SUBVOL_RDONLY;
+	} else if (!strcmp(value, "false")) {
+		flags = flags & ~BTRFS_SUBVOL_RDONLY;
+	} else {
+		ret = -EINVAL;
+		fprintf(stderr, "ERROR: invalid value for property.\n");
+		goto out;
+	}
+
+	ret = ioctl(fd, BTRFS_IOC_SUBVOL_SETFLAGS, &flags);
+	if (ret < 0) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: failed to set flags for %s. %s\n",
+				object, strerror(-ret));
+		goto out;
+	}
+
+out:
+	if (fd != -1)
+		close(fd);
+	return ret;
+}
+
+static int prop_label(enum prop_object_type type,
+		      const char *object,
+		      const char *name,
+		      const char *value)
+{
+	int ret;
+
+	if (value) {
+		ret = set_label((char *) object, (char *) value);
+	} else {
+		char label[BTRFS_LABEL_SIZE];
+
+		ret = get_label((char *) object, label);
+		if (!ret)
+			fprintf(stdout, "label=%s\n", label);
+	}
+
+	return ret;
+}
+
+const struct prop_handler prop_handlers[] = {
+	{"ro", "Set/get read-only flag of subvolume.", 0, prop_object_subvol,
+	 prop_read_only},
+	{"label", "Set/get label of device.", 0, prop_object_dev, prop_label},
+	{0, 0, 0, 0, 0}
+};
diff --git a/props.h b/props.h
new file mode 100644
index 0000000..faa4410
--- /dev/null
+++ b/props.h
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+
+#ifndef PROPS_H_
+#define PROPS_H_
+
+enum prop_object_type {
+	prop_object_dev		= (1 << 0),
+	prop_object_root	= (1 << 1),
+	prop_object_subvol	= (1 << 2),
+	prop_object_inode	= (1 << 3),
+	__prop_object_max,
+};
+
+typedef int (*prop_handler_t)(enum prop_object_type type,
+			      const char *object,
+			      const char *name,
+			      const char *value);
+
+struct prop_handler {
+	const char *name;
+	const char *desc;
+	int read_only;
+	int types;
+	prop_handler_t handler;
+};
+
+extern const struct prop_handler prop_handlers[];
+
+#endif /* PROPS_H_ */
-- 
1.7.9.5


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

* [PATCH 3/5] Btrfs-progs: fix detection of root objects in cmds-property.c
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 2/5] Btrfs-progs: introduce btrfs property subgroup Filipe David Borba Manana
@ 2013-11-12 13:41 ` Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 4/5] Btrfs-progs: add type root to label property Filipe David Borba Manana
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

Several fixes:

1) The function check_is_root() returns 0 if the object is root;

2) Don't treat any error from get fsid ioctl as meaning the target
   is root. Only -ENOTTY means it's a root (parent directory is
   not a btrfs fs) and a -ENOTDIR means our target object is not a
   directory, therefore it can be the root;

3) Fix the comparison of the target and target's parent fs ids. If
   they are different, it means the target is a mount point in a
   btrfs fs, therefore it's a root, otherwise it isn't.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 cmds-property.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/cmds-property.c b/cmds-property.c
index 6699e61..3d1f18c 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -75,7 +75,7 @@ static int parse_prop(const char *arg, const struct prop_handler *props,
 	return -1;
 }
 
-static int get_fsid(const char *path, u8 *fsid)
+static int get_fsid(const char *path, u8 *fsid, int silent)
 {
 	int ret;
 	int fd;
@@ -84,7 +84,8 @@ static int get_fsid(const char *path, u8 *fsid)
 	fd = open(path, O_RDONLY);
 	if (fd < 0) {
 		ret = -errno;
-		fprintf(stderr, "ERROR: open %s failed. %s\n", path,
+		if (!silent)
+			fprintf(stderr, "ERROR: open %s failed. %s\n", path,
 				strerror(-ret));
 		goto out;
 	}
@@ -109,7 +110,7 @@ static int check_btrfs_object(const char *object)
 	int ret;
 	u8 fsid[BTRFS_FSID_SIZE];
 
-	ret = get_fsid(object, fsid);
+	ret = get_fsid(object, fsid, 0);
 	if (ret < 0)
 		ret = 0;
 	else
@@ -134,20 +135,27 @@ static int check_is_root(const char *object)
 		strcat(tmp, "/");
 	strcat(tmp, "..");
 
-	ret = get_fsid(object, fsid);
+	ret = get_fsid(object, fsid, 0);
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", object,
 				strerror(-ret));
 		goto out;
 	}
 
-	ret = get_fsid(tmp, fsid2);
-	if (ret < 0) {
+	ret = get_fsid(tmp, fsid2, 1);
+	if (ret == -ENOTTY) {
 		ret = 0;
 		goto out;
+	} else if (ret == -ENOTDIR) {
+		ret = 1;
+		goto out;
+	} else if (ret < 0) {
+		fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", tmp,
+			strerror(-ret));
+		goto out;
 	}
 
-	if (!memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
+	if (memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
 		ret = 0;
 		goto out;
 	}
@@ -195,7 +203,7 @@ static int autodetect_object_types(const char *object, int *types_out)
 		ret = check_is_root(object);
 		if (ret < 0)
 			goto out;
-		if (ret)
+		if (!ret)
 			types |= prop_object_root;
 	}
 
-- 
1.7.9.5


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

* [PATCH 4/5] Btrfs-progs: add type root to label property
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
                   ` (2 preceding siblings ...)
  2013-11-12 13:41 ` [PATCH 3/5] Btrfs-progs: fix detection of root objects in cmds-property.c Filipe David Borba Manana
@ 2013-11-12 13:41 ` Filipe David Borba Manana
  2013-11-12 13:41 ` [PATCH 5/5] Btrfs-progs: add support for the compression property Filipe David Borba Manana
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

So that we can get the label of a mounted filesystem.

Before this change:

        $ btrfs prop get /mnt/btrfs label
        ERROR: object is not compatible with property

        $ btrfs prop get /dev/sdb3 label
        ERROR: dev /dev/sdb3 is mounted, use mount point
        ERROR: failed to set/get property for object.

After this change:

        $ btrfs prop get /mnt/btrfs label
        label=foobar

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 props.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/props.c b/props.c
index 763fd57..12fdc52 100644
--- a/props.c
+++ b/props.c
@@ -105,6 +105,7 @@ static int prop_label(enum prop_object_type type,
 const struct prop_handler prop_handlers[] = {
 	{"ro", "Set/get read-only flag of subvolume.", 0, prop_object_subvol,
 	 prop_read_only},
-	{"label", "Set/get label of device.", 0, prop_object_dev, prop_label},
+	{"label", "Set/get label of device.", 0,
+	 prop_object_dev | prop_object_root, prop_label},
 	{0, 0, 0, 0, 0}
 };
-- 
1.7.9.5


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

* [PATCH 5/5] Btrfs-progs: add support for the compression property
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
                   ` (3 preceding siblings ...)
  2013-11-12 13:41 ` [PATCH 4/5] Btrfs-progs: add type root to label property Filipe David Borba Manana
@ 2013-11-12 13:41 ` Filipe David Borba Manana
  2013-11-13  1:22 ` [PATCH 5/5 V2] " Filipe David Borba Manana
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-12 13:41 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

With this property, one can enable compression for individual files
without the need to mount the filesystem with the compress or
compress-force options, and specify the compression algorithm.

When applied against a directory, files created under that directory
will inherit the compression property.

This requires the corresponding kernel patch, which adds the
support for the property get and set ioctls and implementation
of the compression property.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 ioctl.h |   12 +++++++++++
 props.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/ioctl.h b/ioctl.h
index d21413f..b1f7928 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -465,6 +465,14 @@ struct btrfs_ioctl_qgroup_create_args {
 	__u64 qgroupid;
 };
 
+struct btrfs_ioctl_getset_prop_args {
+	__u16 name_len;   /* in     */
+	__u16 value_len;  /* in/out */
+	__u8  unused[4];
+	__u64 name_ptr;   /* in     */
+	__u64 value_ptr;  /* in/out */
+};
+
 /* Error codes as returned by the kernel */
 enum btrfs_err_code {
 	notused,
@@ -551,6 +559,10 @@ struct btrfs_ioctl_clone_range_args {
 #define BTRFS_IOC_DEFAULT_SUBVOL _IOW(BTRFS_IOCTL_MAGIC, 19, u64)
 #define BTRFS_IOC_SPACE_INFO _IOWR(BTRFS_IOCTL_MAGIC, 20, \
 				    struct btrfs_ioctl_space_args)
+#define BTRFS_IOC_GET_PROP _IOR(BTRFS_IOCTL_MAGIC, 21, \
+				struct btrfs_ioctl_getset_prop_args)
+#define BTRFS_IOC_SET_PROP _IOW(BTRFS_IOCTL_MAGIC, 21, \
+				struct btrfs_ioctl_getset_prop_args)
 #define BTRFS_IOC_SNAP_CREATE_V2 _IOW(BTRFS_IOCTL_MAGIC, 23, \
 				   struct btrfs_ioctl_vol_args_v2)
 #define BTRFS_IOC_SUBVOL_CREATE_V2 _IOW(BTRFS_IOCTL_MAGIC, 24, \
diff --git a/props.c b/props.c
index 12fdc52..4a7d7ef 100644
--- a/props.c
+++ b/props.c
@@ -17,6 +17,7 @@
 #include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
+#include <stdint.h>
 #include <unistd.h>
 
 #include "ctree.h"
@@ -102,10 +103,82 @@ static int prop_label(enum prop_object_type type,
 	return ret;
 }
 
+static int prop_compression(enum prop_object_type type,
+			    const char *object,
+			    const char *name,
+			    const char *value)
+{
+	int ret;
+	DIR *dirstream = NULL;
+	int fd = -1;
+	struct btrfs_ioctl_getset_prop_args args;
+	char *buf = NULL;
+
+	args.name_len = strlen(name);
+	args.name_ptr = (uintptr_t) name;
+
+	if (value) {
+		args.value_len = strlen(value);
+		args.value_ptr = (uintptr_t) value;
+	} else {
+		args.value_len = 0;
+		args.value_ptr = (uintptr_t) NULL;
+	}
+
+	fd = open_file_or_dir(object, &dirstream);
+	if (fd == -1) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: open %s failed. %s\n",
+			object, strerror(-ret));
+		goto out;
+	}
+
+	if (value)
+		ret = ioctl(fd, BTRFS_IOC_SET_PROP, &args);
+	else
+		ret = ioctl(fd, BTRFS_IOC_GET_PROP, &args);
+	if (ret < 0) {
+		ret = -errno;
+		if (ret != -ENODATA)
+			fprintf(stderr,
+				"ERROR: failed to %s compression for %s. %s\n",
+				value ? "set" : "get", object, strerror(-ret));
+		goto out;
+	}
+	if (!value) {
+		buf = malloc(args.value_len);
+		if (!buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		args.value_ptr = (uintptr_t) buf;
+		ret = ioctl(fd, BTRFS_IOC_GET_PROP, &args);
+		if (ret < 0) {
+			ret = -errno;
+			fprintf(stderr,
+				"ERROR: failed to get compression for %s. %s\n",
+				object, strerror(-ret));
+			goto out;
+		}
+		fprintf(stdout, "compression=%.*s\n", args.value_len, buf);
+	}
+
+	ret = 0;
+out:
+	free(buf);
+	if (fd >= 0)
+		close_file_or_dir(fd, dirstream);
+
+	return ret;
+}
+
+
 const struct prop_handler prop_handlers[] = {
 	{"ro", "Set/get read-only flag of subvolume.", 0, prop_object_subvol,
 	 prop_read_only},
 	{"label", "Set/get label of device.", 0,
 	 prop_object_dev | prop_object_root, prop_label},
+	{"compression", "Set/get compression for a file or directory", 0,
+	 prop_object_inode, prop_compression},
 	{0, 0, 0, 0, 0}
 };
-- 
1.7.9.5


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

* [PATCH 5/5 V2] Btrfs-progs: add support for the compression property
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
                   ` (4 preceding siblings ...)
  2013-11-12 13:41 ` [PATCH 5/5] Btrfs-progs: add support for the compression property Filipe David Borba Manana
@ 2013-11-13  1:22 ` Filipe David Borba Manana
  2013-11-13 16:15 ` [PATCH 0/5] Add support for object properties David Sterba
  2013-11-23  0:52 ` David Sterba
  7 siblings, 0 replies; 13+ messages in thread
From: Filipe David Borba Manana @ 2013-11-13  1:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Filipe David Borba Manana

With this property, one can enable compression for individual files
without the need to mount the filesystem with the compress or
compress-force options, and specify the compression algorithm.

When applied against a directory, files created under that directory
will inherit the compression property.

This requires the corresponding kernel patch, which adds the support
for setting and getting properties and implements the compression
property.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---

V2: Update to match latest kernel change - use regular set/get xattr
    path to set/get compression property.

 props.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/props.c b/props.c
index 12fdc52..4d0aeea 100644
--- a/props.c
+++ b/props.c
@@ -16,6 +16,8 @@
 
 #include <sys/stat.h>
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <attr/xattr.h>
 #include <fcntl.h>
 #include <unistd.h>
 
@@ -24,6 +26,10 @@
 #include "utils.h"
 #include "props.h"
 
+#define XATTR_BTRFS_PREFIX     "btrfs."
+#define XATTR_BTRFS_PREFIX_LEN (sizeof(XATTR_BTRFS_PREFIX) - 1)
+
+
 static int prop_read_only(enum prop_object_type type,
 			  const char *object,
 			  const char *name,
@@ -102,10 +108,82 @@ static int prop_label(enum prop_object_type type,
 	return ret;
 }
 
+static int prop_compression(enum prop_object_type type,
+			    const char *object,
+			    const char *name,
+			    const char *value)
+{
+	int ret;
+	ssize_t sret;
+	int fd = -1;
+	DIR *dirstream = NULL;
+	char *buf = NULL;
+	char *xattr_name = NULL;
+
+	fd = open_file_or_dir(object, &dirstream);
+	if (fd == -1) {
+		ret = -errno;
+		fprintf(stderr, "ERROR: open %s failed. %s\n",
+			object, strerror(-ret));
+		goto out;
+	}
+
+	xattr_name = malloc(XATTR_BTRFS_PREFIX_LEN + strlen(name));
+	if (!xattr_name) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	memcpy(xattr_name, XATTR_BTRFS_PREFIX, XATTR_BTRFS_PREFIX_LEN);
+	memcpy(xattr_name + XATTR_BTRFS_PREFIX_LEN, name, strlen(name));
+
+	if (value)
+		sret = fsetxattr(fd, xattr_name, value, strlen(value), 0);
+	else
+		sret = fgetxattr(fd, xattr_name, NULL, 0);
+	if (sret < 0) {
+		ret = -errno;
+		if (ret != -ENODATA)
+			fprintf(stderr,
+				"ERROR: failed to %s compression for %s. %s\n",
+				value ? "set" : "get", object, strerror(-ret));
+		goto out;
+	}
+	if (!value) {
+		size_t len = sret;
+
+		buf = malloc(len);
+		if (!buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		sret = fgetxattr(fd, xattr_name, buf, len);
+		if (sret < 0) {
+			ret = -errno;
+			fprintf(stderr,
+				"ERROR: failed to get compression for %s. %s\n",
+				object, strerror(-ret));
+			goto out;
+		}
+		fprintf(stdout, "compression=%.*s\n", (int)len, buf);
+	}
+
+	ret = 0;
+out:
+	free(xattr_name);
+	free(buf);
+	if (fd >= 0)
+		close_file_or_dir(fd, dirstream);
+
+	return ret;
+}
+
+
 const struct prop_handler prop_handlers[] = {
 	{"ro", "Set/get read-only flag of subvolume.", 0, prop_object_subvol,
 	 prop_read_only},
 	{"label", "Set/get label of device.", 0,
 	 prop_object_dev | prop_object_root, prop_label},
+	{"compression", "Set/get compression for a file or directory", 0,
+	 prop_object_inode, prop_compression},
 	{0, 0, 0, 0, 0}
 };
-- 
1.7.9.5


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

* Re: [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it
  2013-11-12 13:41 ` [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it Filipe David Borba Manana
@ 2013-11-13 15:43   ` David Sterba
  0 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2013-11-13 15:43 UTC (permalink / raw)
  To: Filipe David Borba Manana; +Cc: linux-btrfs

On Tue, Nov 12, 2013 at 01:41:42PM +0000, Filipe David Borba Manana wrote:
> get_label prints the label at the moment. Change this so that
> the label is returned and printing is done by the caller.
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>

FYI, I'll add this patch to integration now because it's independent on
the rest of the series.

david

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

* Re: [PATCH 0/5] Add support for object properties
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
                   ` (5 preceding siblings ...)
  2013-11-13  1:22 ` [PATCH 5/5 V2] " Filipe David Borba Manana
@ 2013-11-13 16:15 ` David Sterba
  2013-11-23  0:52 ` David Sterba
  7 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2013-11-13 16:15 UTC (permalink / raw)
  To: Filipe David Borba Manana; +Cc: linux-btrfs

On Tue, Nov 12, 2013 at 01:41:41PM +0000, Filipe David Borba Manana wrote:
> This is a revised version of the original proposal/work from Alexander Block
> to introduce a generic framework to set properties on btrfs filesystem objects
> (inodes, subvolumes, filesystems, devices).

Thank you very much for working on that. I've added patches 2-5 to the
unstable part of integration branch. More comments later.

david

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

* Re: [PATCH 0/5] Add support for object properties
  2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
                   ` (6 preceding siblings ...)
  2013-11-13 16:15 ` [PATCH 0/5] Add support for object properties David Sterba
@ 2013-11-23  0:52 ` David Sterba
  2013-11-23 10:45   ` Goffredo Baroncelli
  2014-01-07 11:51   ` Filipe David Manana
  7 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2013-11-23  0:52 UTC (permalink / raw)
  To: Filipe David Borba Manana; +Cc: linux-btrfs

On Tue, Nov 12, 2013 at 01:41:41PM +0000, Filipe David Borba Manana wrote:
> This is a revised version of the original proposal/work from Alexander Block
> to introduce a generic framework to set properties on btrfs filesystem objects
> (inodes, subvolumes, filesystems, devices).

>         Currently the command group looks like this:
>         btrfs prop set [-t <type>] /path/to/object <name> <value>
>         btrfs prop get [-t <type>] /path/to/object [<name>] (omitting name dumps all)
>         btrfs prop list [-t <type>] /path/to/object (lists properties with description)
> 
>         The type is used to explicitly specify what type of object you mean. This is 
>         necessary in case the object+property combination is ambiguous.  For example
>         '/path/to/fs/root' could mean the root subvolume, the directory inode or the 
>         filesystem itself. Normally, btrfs-progs will try to detect the type 
>         automatically.

The generic commandline UI still looks ok to me, storing properties as xattr’s
is also ok (provided that we can capture the “btrfs.” xattr namespace).

I’ll dump my thoughts and questions about the rest.

1) Where are stored properties that are not directly attached to an inode? Ie.
   whole-filesystem and device. How can I access props for a subvolume that is
   not currently reachable in the directory tree?

A fs or device props must be accessible any time, eg. no matter which subvolume
is currently mounted. This should be probably a special case where the property
can be queried from any inode but will be internally routed to eg. toplevel
subvolume that will store the respective xattrs.


2) if a property’s object can be ambiguous, how is that distinguished in the
   xattrs?

We don’t have a list of props yet, so I’m trying to use one that hopefully
makes some sense. The example here can be persistent-mount-options that are
attached to fs and a subvolume. The fs-wide props will apply when a different
subvolume is explicitly mounted.

Assuming that the xattrs are stored with the toplevel subvolume, the fs-wide
and per-subvolume property must have a differnt xattr name (another option is
to store fs-wide elsewhere). So the question is, if we should encode the
property object into the xattr name directly. Eg.:

  btrfs.fs.persistent_mount
  btrfs.subvol.persistent_mount

or if the fs-wide uses a reserved naming scheme that would appear as xattr
named

  btrfs.persistent_mount

but the value would differ if queried with ‘-t fs or ‘-t subvolume’.


3) property precedence, interaction with mount options

The precedence should follow the rule of the least surprise, ie. if I set eg. a
compression of a file to zlib, set the subvolume compression type to ‘none’ and
have fs-wide mount the filesystem with compress=lzo, I’m expecting that the
file will use ‘zlib’.

The generic rule says that mount options (that have a corresponding property)
take precedence. There may be exceptions.

What if there are no specific mount options, but the subvolume has eg.
compression set? Should new files inherit that automatically or only if
explicitly marked as such?

The background concern is how far do I need to look and whether this could be a
potential performance problem. The lookup chain can be long: inode -> dir ->
subvol -> fs, plus mount options along the path where applicable.

If the xattr values are cached in structs that are accessed anyway (containing
subvol, fs_info) and updated when property xattr changes, this could work.


4) behaviour of tools that see the btrfs-specific props

What would cp or rsync do when copying the xattrs to a different filesystem?
They may simply ignore it and just try to copy the user. namespace, I haven’t
done any research here.


5) properties to consider, naming, xattr name namespaces, inheritable props

Here’s a list of properties that I’ve collected so far:

filesystem-wide
- persistent mount options
- default raid allocation profile
- default compression parameters
- label
- scrub ioprio

subvolume
- persistent mount options
- default raid allocation profile
- default compression parameters
- writable

device
- writable/ro/dead
- hot-spare
- preferred for metadata
- preferred data reads
- preferred data writes
- no new allocations if possible
- speed profile, io priorities
- allocation group - from project idea "Chunk allocation groups"
- "ssd cache", L2ARC
- scrub ioprio

file, directory
- compression parameters
- raid allocation profile

details on compression parameters:
- compression algo
- compression level
- desired compression ratio target
- file compressibility hint - from project idea “Compression updates”
- compression container type - dtto

The props should be basically self explanatory. This is not a complete or
finalized list, feel free to suggest what you think should/not be here.

The number of compression parameters suggest that a namespace would be
appropriate. This could establish a common practice for any set of properties
that are not attached to any filesystem object but are grouped logically.

Most of the props are expected to be read-write, I don’t have a counterexample
for a property that makes sense, is persistent and must be read-only. Such one
can be exported through the sysfs, so s/Most of/All/ in the previous sentence.

I don’t have a proposal how to mark inheritable props yet.


david

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

* Re: [PATCH 0/5] Add support for object properties
  2013-11-23  0:52 ` David Sterba
@ 2013-11-23 10:45   ` Goffredo Baroncelli
  2014-01-07 11:51   ` Filipe David Manana
  1 sibling, 0 replies; 13+ messages in thread
From: Goffredo Baroncelli @ 2013-11-23 10:45 UTC (permalink / raw)
  To: dsterba, Filipe David Borba Manana, linux-btrfs

Hi David,

On 2013-11-23 01:52, David Sterba wrote:
> On Tue, Nov 12, 2013 at 01:41:41PM +0000, Filipe David Borba Manana wrote:
>> This is a revised version of the original proposal/work from Alexander Block
>> to introduce a generic framework to set properties on btrfs filesystem objects
>> (inodes, subvolumes, filesystems, devices).
> 
>>         Currently the command group looks like this:
>>         btrfs prop set [-t <type>] /path/to/object <name> <value>
>>         btrfs prop get [-t <type>] /path/to/object [<name>] (omitting name dumps all)
>>         btrfs prop list [-t <type>] /path/to/object (lists properties with description)
>>
>>         The type is used to explicitly specify what type of object you mean. This is 
>>         necessary in case the object+property combination is ambiguous.  For example
>>         '/path/to/fs/root' could mean the root subvolume, the directory inode or the 
>>         filesystem itself. Normally, btrfs-progs will try to detect the type 
>>         automatically.
> 
> The generic commandline UI still looks ok to me, storing properties as xattr’s
> is also ok (provided that we can capture the “btrfs.” xattr namespace).
> 
> I’ll dump my thoughts and questions about the rest.
> 
> 1) Where are stored properties that are not directly attached to an inode? Ie.
>    whole-filesystem and device. How can I access props for a subvolume that is
>    not currently reachable in the directory tree?
> 
> A fs or device props must be accessible any time, eg. no matter which subvolume
> is currently mounted. This should be probably a special case where the property
> can be queried from any inode but will be internally routed to eg. toplevel
> subvolume that will store the respective xattrs.


I think that we should divided "how access the properties" and "where
the properties are stored".

- Storing the *inode* properties in xattrs makes sense: it is a clean
interface, and the infrastructure is capable to hold all the informations.
It could be discussed also if we need to use 1 property <-> 1 xattr or
use one xattr to hold more properties (this could alleviate some
performance problem)

- Storing the *subvolume* and the *filesystem* properties in an xattr
associated to subvolue or '/' inode could be done. When it is accessed
this subvolume or '/' inode (during the mount and/or inode traversing)
all the information could be read from the xattr and stored in the
btrfs_fs_info struct and  btrfs_root struct (which are easily accessible
from the inode, avoiding performance issues)

- For the *device* properties, we could think to use other trees like
the device tree to store the information but still using the *xattr
interfaces to access them.

> 
> 
> 2) if a property’s object can be ambiguous, how is that distinguished in the
>    xattrs?
> 
> We don’t have a list of props yet, so I’m trying to use one that hopefully
> makes some sense. The example here can be persistent-mount-options that are
> attached to fs and a subvolume. The fs-wide props will apply when a different
> subvolume is explicitly mounted.
> 
> Assuming that the xattrs are stored with the toplevel subvolume, the fs-wide
> and per-subvolume property must have a differnt xattr name (another option is
> to store fs-wide elsewhere). So the question is, if we should encode the
> property object into the xattr name directly. Eg.:
> 
>   btrfs.fs.persistent_mount
>   btrfs.subvol.persistent_mount
> 
> or if the fs-wide uses a reserved naming scheme that would appear as xattr
> named
> 
>   btrfs.persistent_mount
> 
> but the value would differ if queried with ‘-t fs or ‘-t subvolume’.
> 
> 
> 3) property precedence, interaction with mount options
> 
> The precedence should follow the rule of the least surprise, ie. if I set eg. a
> compression of a file to zlib, set the subvolume compression type to ‘none’ and
> have fs-wide mount the filesystem with compress=lzo, I’m expecting that the
> file will use ‘zlib’.

> 
> The generic rule says that mount options (that have a corresponding property)
> take precedence. There may be exceptions.


As general rule I suggest the following priority list:

fs props, subvol props, dir props, inode props, mount opts

It should be possible to "override" via mount option all options (to
force some behaviour).

However I have some doubts about:

1) in case of nested subvolumes, should the inner subvolumes inherit the
properties of the outer subvolumes? I think no, because it is not so
obvious the hierarchy when a subvolume is mounted (or moved).

2) there are properties that are "inode" related. Other no. For example
does make sense to have inode-setting about compression, raid profile,
datasum/cow ... when the data is shared between different
inode/subvolume which could have different setup? Which should be the
"least surprise" behaviour ? Or we should intend these properties only
as hints for the new file/data/chunk ? Anyway what it would do a balance
in these case ?
I am inclined to think that the "storage policy" (raid profile,
compression, cow...) should be  "per filesystem" and not per inode:
having different "storage policy" doesn't mix well with the sharing of
data (like the snapshot).

> What if there are no specific mount options, but the subvolume has eg.
> compression set? Should new files inherit that automatically or only if
> explicitly marked as such?
> 
> The background concern is how far do I need to look and whether this could be a
> potential performance problem. The lookup chain can be long: inode -> dir ->
> subvol -> fs, plus mount options along the path where applicable.

Looking at the acl, exists the concept of the "default" acl (the one
inherited).

> If the xattr values are cached in structs that are accessed anyway (containing
> subvol, fs_info) and updated when property xattr changes, this could work.

I reached the same conclusion
> 
> 
> 4) behaviour of tools that see the btrfs-specific props
> 
> What would cp or rsync do when copying the xattrs to a different filesystem?
> They may simply ignore it and just try to copy the user. namespace, I haven’t
> done any research here.


>From rsync man page

	This  option  causes  rsync  to  update the destination extended
        attributes to be the same as the source ones.

        For systems that support extended-attribute namespaces,  a  copy
        being  done  by  a  super-user copies all namespaces except sys‐
        tem.*.

> 
> 
> 5) properties to consider, naming, xattr name namespaces, inheritable props
> 
> Here’s a list of properties that I’ve collected so far:
> 
> filesystem-wide
> - persistent mount options
> - default raid allocation profile
> - default compression parameters
> - label
> - scrub ioprio
> 
> subvolume
> - persistent mount options
> - default raid allocation profile
> - default compression parameters
> - writable
> 
> device
> - writable/ro/dead
> - hot-spare
> - preferred for metadata
> - preferred data reads
> - preferred data writes
> - no new allocations if possible
> - speed profile, io priorities
> - allocation group - from project idea "Chunk allocation groups"
> - "ssd cache", L2ARC
> - scrub ioprio
> 
> file, directory
> - compression parameters
> - raid allocation profile
> 
> details on compression parameters:
> - compression algo
> - compression level
> - desired compression ratio target
> - file compressibility hint - from project idea “Compression updates”
> - compression container type - dtto
> 
> The props should be basically self explanatory. This is not a complete or
> finalized list, feel free to suggest what you think should/not be here.
> 
> The number of compression parameters suggest that a namespace would be
> appropriate. This could establish a common practice for any set of properties
> that are not attached to any filesystem object but are grouped logically.
> 
> Most of the props are expected to be read-write, I don’t have a counterexample
> for a property that makes sense, is persistent and must be read-only. Such one
> can be exported through the sysfs, so s/Most of/All/ in the previous sentence.

Some properties which could be exported as readonly:
- the uuid of the subvolume
- the "generation" number (but in effect exists and ioctl for that)

Anyway sysfs is good for filesystem properties; it could handle also
subvolume properties. Definitely it is not adapted to "per inode"
properties.

> 
> I don’t have a proposal how to mark inheritable props yet.
> 
> 
> david
> --
> 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
> 


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

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

* Re: [PATCH 0/5] Add support for object properties
  2013-11-23  0:52 ` David Sterba
  2013-11-23 10:45   ` Goffredo Baroncelli
@ 2014-01-07 11:51   ` Filipe David Manana
  2014-01-23 17:47     ` David Sterba
  1 sibling, 1 reply; 13+ messages in thread
From: Filipe David Manana @ 2014-01-07 11:51 UTC (permalink / raw)
  To: dsterba, Filipe David Borba Manana, linux-btrfs

On Sat, Nov 23, 2013 at 12:52 AM, David Sterba <dsterba@suse.cz> wrote:
> On Tue, Nov 12, 2013 at 01:41:41PM +0000, Filipe David Borba Manana wrote:
>> This is a revised version of the original proposal/work from Alexander Block
>> to introduce a generic framework to set properties on btrfs filesystem objects
>> (inodes, subvolumes, filesystems, devices).
>
>>         Currently the command group looks like this:
>>         btrfs prop set [-t <type>] /path/to/object <name> <value>
>>         btrfs prop get [-t <type>] /path/to/object [<name>] (omitting name dumps all)
>>         btrfs prop list [-t <type>] /path/to/object (lists properties with description)
>>
>>         The type is used to explicitly specify what type of object you mean. This is
>>         necessary in case the object+property combination is ambiguous.  For example
>>         '/path/to/fs/root' could mean the root subvolume, the directory inode or the
>>         filesystem itself. Normally, btrfs-progs will try to detect the type
>>         automatically.
>
> The generic commandline UI still looks ok to me, storing properties as xattr’s
> is also ok (provided that we can capture the “btrfs.” xattr namespace).
>
> I’ll dump my thoughts and questions about the rest.
>
> 1) Where are stored properties that are not directly attached to an inode? Ie.
>    whole-filesystem and device. How can I access props for a subvolume that is
>    not currently reachable in the directory tree?
>
> A fs or device props must be accessible any time, eg. no matter which subvolume
> is currently mounted. This should be probably a special case where the property
> can be queried from any inode but will be internally routed to eg. toplevel
> subvolume that will store the respective xattrs.
>
>
> 2) if a property’s object can be ambiguous, how is that distinguished in the
>    xattrs?
>
> We don’t have a list of props yet, so I’m trying to use one that hopefully
> makes some sense. The example here can be persistent-mount-options that are
> attached to fs and a subvolume. The fs-wide props will apply when a different
> subvolume is explicitly mounted.
>
> Assuming that the xattrs are stored with the toplevel subvolume, the fs-wide
> and per-subvolume property must have a differnt xattr name (another option is
> to store fs-wide elsewhere). So the question is, if we should encode the
> property object into the xattr name directly. Eg.:
>
>   btrfs.fs.persistent_mount
>   btrfs.subvol.persistent_mount
>
> or if the fs-wide uses a reserved naming scheme that would appear as xattr
> named
>
>   btrfs.persistent_mount
>
> but the value would differ if queried with ‘-t fs or ‘-t subvolume’.
>
>
> 3) property precedence, interaction with mount options
>
> The precedence should follow the rule of the least surprise, ie. if I set eg. a
> compression of a file to zlib, set the subvolume compression type to ‘none’ and
> have fs-wide mount the filesystem with compress=lzo, I’m expecting that the
> file will use ‘zlib’.
>
> The generic rule says that mount options (that have a corresponding property)
> take precedence. There may be exceptions.
>
> What if there are no specific mount options, but the subvolume has eg.
> compression set? Should new files inherit that automatically or only if
> explicitly marked as such?
>
> The background concern is how far do I need to look and whether this could be a
> potential performance problem. The lookup chain can be long: inode -> dir ->
> subvol -> fs, plus mount options along the path where applicable.
>
> If the xattr values are cached in structs that are accessed anyway (containing
> subvol, fs_info) and updated when property xattr changes, this could work.
>
>
> 4) behaviour of tools that see the btrfs-specific props
>
> What would cp or rsync do when copying the xattrs to a different filesystem?
> They may simply ignore it and just try to copy the user. namespace, I haven’t
> done any research here.
>
>
> 5) properties to consider, naming, xattr name namespaces, inheritable props
>
> Here’s a list of properties that I’ve collected so far:
>
> filesystem-wide
> - persistent mount options
> - default raid allocation profile
> - default compression parameters
> - label
> - scrub ioprio
>
> subvolume
> - persistent mount options
> - default raid allocation profile
> - default compression parameters
> - writable
>
> device
> - writable/ro/dead
> - hot-spare
> - preferred for metadata
> - preferred data reads
> - preferred data writes
> - no new allocations if possible
> - speed profile, io priorities
> - allocation group - from project idea "Chunk allocation groups"
> - "ssd cache", L2ARC
> - scrub ioprio
>
> file, directory
> - compression parameters
> - raid allocation profile
>
> details on compression parameters:
> - compression algo
> - compression level
> - desired compression ratio target
> - file compressibility hint - from project idea “Compression updates”
> - compression container type - dtto
>
> The props should be basically self explanatory. This is not a complete or
> finalized list, feel free to suggest what you think should/not be here.
>
> The number of compression parameters suggest that a namespace would be
> appropriate. This could establish a common practice for any set of properties
> that are not attached to any filesystem object but are grouped logically.
>
> Most of the props are expected to be read-write, I don’t have a counterexample
> for a property that makes sense, is persistent and must be read-only. Such one
> can be exported through the sysfs, so s/Most of/All/ in the previous sentence.
>
> I don’t have a proposal how to mark inheritable props yet.

Thanks for sharing your thoughts on this David.

There seems to be a lot of details for this feature, and many other
projects (such as those you point on the wiki) would benefit from
properties.
It might be hard to get all requirements at once, so I think it might
be better to start simple, but in a way that doesn't prevent adding
the more advanced requirements later.

As for the xattrs namespace: yes, we can capture the "btrfs."
namespace. hfsplus does this, it captures the "osx." namespace.
See my reply to Hugo about this on this thread:
http://comments.gmane.org/gmane.comp.file-systems.btrfs/29972

My work so far, for the kernel side, was all about inode (and
subvolume) properties. Haven't considered properties for devices so
far.
I think the inheritable detail can depend on the specific property.
What I did so far, adds generic support  for both inheritable and
non-inheritable ones.

And yes, so far it has some impact on performance. I added some
results (and concerns) in the last rfc patch (v4) I just sent minutes
ago.
By using xattrs, while simplifying the storage and not
touching/extending the on-disk format, it will always have some impact
on performance - they take leaf space, hence btrees will be larger
than before.

I think we need to clearly define what are the initial requirements,
start with a simple approach, always thinking on how to extend/refine
it later without the need for breaking backwards compatibility later
of course.

Thanks David.


>
>
> david



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

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

* Re: [PATCH 0/5] Add support for object properties
  2014-01-07 11:51   ` Filipe David Manana
@ 2014-01-23 17:47     ` David Sterba
  0 siblings, 0 replies; 13+ messages in thread
From: David Sterba @ 2014-01-23 17:47 UTC (permalink / raw)
  To: Filipe David Manana; +Cc: linux-btrfs

On Tue, Jan 07, 2014 at 11:51:08AM +0000, Filipe David Manana wrote:
> There seems to be a lot of details for this feature, and many other
> projects (such as those you point on the wiki) would benefit from
> properties.
> It might be hard to get all requirements at once, so I think it might
> be better to start simple, but in a way that doesn't prevent adding
> the more advanced requirements later.

Absolutely, that's why I'm trying to gather all potential future
requirements and usecases now so we don't become surpristed later.
Although it's detailed, there are groups that are covered by saying
eg.  "this is per-device, stored here, accessed trhough this xattr".
If the proposal is able to give such answers, then it's what I'm
expecting at this phase.

> As for the xattrs namespace: yes, we can capture the "btrfs."
> namespace. hfsplus does this, it captures the "osx." namespace.
> See my reply to Hugo about this on this thread:
> http://comments.gmane.org/gmane.comp.file-systems.btrfs/29972

JFYI, there's a discussion on fsdevel about replacing the existing file
attributes ioctl with something else, xattr's are considered as the
UI/storage, so it would need some coordination to be consistent with
whatever gets decided.

> My work so far, for the kernel side, was all about inode (and
> subvolume) properties. Haven't considered properties for devices so
> far.

I hope it's clear that I'm not asking about implementing all the
properties from the beginning. I's fine that you're focused on inode
props, get the things working so we have something tangible to play
with. If the device props are not forgottent to add to some high-level
xattr namespace, I'm satisfied.

> By using xattrs, while simplifying the storage and not
> touching/extending the on-disk format, it will always have some impact
> on performance - they take leaf space, hence btrees will be larger
> than before.

Not all of the props need to end up as a xattr, we can still use the
existing inode->flags field that's used for the file attributes, there
are 11 bits occupied, plenty of room to add more where it would make
sense.  The rest shall be in xattr items.

> I think we need to clearly define what are the initial requirements,
> start with a simple approach, always thinking on how to extend/refine
> it later without the need for breaking backwards compatibility later
> of course.

Well, I think once the xattr naming scheme is set it won't be changed or
refined. I'm expecting only specific properties appended. Eg. if you
propose btrfs.compression as an inode property, then it's a no-go to
move it to btrfs.fs.compression once a whole-filesystem gains persistent
option, and refine again when somebody like me comes and says that any
*.compression should really be a napespace that groups all
compression-related options.

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

end of thread, other threads:[~2014-01-23 17:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-12 13:41 [PATCH 0/5] Add support for object properties Filipe David Borba Manana
2013-11-12 13:41 ` [PATCH 1/5] Btrfs-progs: let get_label return the label instead of printing it Filipe David Borba Manana
2013-11-13 15:43   ` David Sterba
2013-11-12 13:41 ` [PATCH 2/5] Btrfs-progs: introduce btrfs property subgroup Filipe David Borba Manana
2013-11-12 13:41 ` [PATCH 3/5] Btrfs-progs: fix detection of root objects in cmds-property.c Filipe David Borba Manana
2013-11-12 13:41 ` [PATCH 4/5] Btrfs-progs: add type root to label property Filipe David Borba Manana
2013-11-12 13:41 ` [PATCH 5/5] Btrfs-progs: add support for the compression property Filipe David Borba Manana
2013-11-13  1:22 ` [PATCH 5/5 V2] " Filipe David Borba Manana
2013-11-13 16:15 ` [PATCH 0/5] Add support for object properties David Sterba
2013-11-23  0:52 ` David Sterba
2013-11-23 10:45   ` Goffredo Baroncelli
2014-01-07 11:51   ` Filipe David Manana
2014-01-23 17:47     ` David Sterba

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.